lue-bird / elm-review-missing-record-field-lens

elm-review: helper generation
https://package.elm-lang.org/packages/lue-bird/elm-review-missing-record-field-lens/latest/
MIT License
2 stars 1 forks source link

Generate lens for `type` with 1 variant #8

Closed lue-bird closed 2 years ago

lue-bird commented 2 years ago

Currently, single-variant types are just ignored.

lue-bird commented 2 years ago

more or less done and tested

erlandsona commented 2 years ago

Currently, single-variant types are just ignored.

We could leverage the Prism mechanics for injecting stuff inline underneath the definitions cause these are all isomorphic. We just can't use the extensible record mechanics for the single variant types. EG: type Record = Record String Int ~ (String, Int) ~ type alias Record = { name : String, age : Int } .

Only difference between these representations is how the data is accessed EG:

type Record = 
   Record String Int

--  Raw Elm
someFn (Record name age) = -- Get
    Record (name++"!") (age+1) -- Set

-- Accessors
someFn = over Record.fst  (\name -> name ++ "!") >> over Record.snd ((+) 1)
-- (String, Int)
someFn = over fst (\name -> name ++ "!") >> over snd ((+) 1)
type alias Record = { name : String, age : Int }

someFn = over Lens.name (\name -> name ++ "!") >> over Lens.age ((+) 1)
lue-bird commented 2 years ago

If I'm understanding correctly, the suggestion is to additionally support generating lenses for variants that aren't exposed for module-internal use. Please open a new issue as this one is unrelated and resolved (though a few more tests for generation wouldn't hurt).

To your example; is it just supposed to show differences between single-variant type and record type alias?

For single-variant types, I generally use typed-value to avoid boilerplate. Overall though, I think it's a great idea to add generated lenses for internal use only.

erlandsona commented 2 years ago

To your example; is it just supposed to show differences between single-variant type and record type alias?

Not the difference but that they're actually "the same". They're isomorphic to one another in that you can go between them with not loss of information.

It's related as you mentioned currently the Prism codegen "ignores this case" except that's by design because a single variant custom type is just a record type with positional arguments... It doesn't become a "Sum type" or an actual Variant until you add a | OtherCase to it at which point you can no longer just wrap and unwrap it at will.

And this particular case can't be codegened with extensible records so the lens has to be generated inline like the prisms only using a lens generator.

lue-bird commented 2 years ago

Seems I again wasn't clear enough: Lenses are already generated for 1-variant types. This issue is closed as completed. For example with

build =
    VariantLens.GenerateUsed.accessors
        { valuesRepresent = VariantLens.GenerateUsed.valuesRecord }
name =
    VariantLens.GenerateUsed.prismNameVariant
type Three a
    = Three String Int a
three : Lens (Three a) transformed { value0 : String, value1 : Int, value2 : a } wrap
three =
    makeOneToOne_
        "Data.Three"
        (\(Three value0 value1 value2) -> { value0 = value0, value1 = value1, value2 = value2 })
        (\valuesAlter (Three value0 value1 value2) ->
            let
                altered =
                    { value0 = value0, value1 = value1, value2 = value2 } |> valuesAlter
            in
            Three altered.value0 altered.value1 altered.value2
        )

Is implementing it like this what you suggested?

erlandsona commented 2 years ago

So that's the basic idea... personally I'd probably go with a Tuple since keynames with magic numbers indicating the positional nature of where the arguments came from seems a bit unintuitive to me?

three : Lens (Three a) transformed ( String, Int, a ) wrap
three =
    makeOneToOne_
        "Data.Three"
        (\(Three value0 value1 value2) -> (value0, value1, value2))
        (\fn (Three v0 v1 v2) ->
            let
                (value0, value1, value2) = fn (v0, v1, v2)
            in
            Three value0 value1 value2
        )

and the nested tuple thing I wrote can be leveraged to handle "more than 3" situations.

lue-bird commented 2 years ago

"value-combining" is a required configuration that doesn't default to valuesRecord. You're required to choose what ever you want:

-

  build =
      VariantLens.GenerateUsed.accessors
          { valuesRepresent = VariantLens.GenerateUsed.valuesRecord }

vs

-

  build =
      VariantLens.GenerateUsed.accessors
          { valuesRepresent = VariantLens.GenerateUsed.tupleNest }

vs

What difference do you see between record vs tuple positional names apart from the latter one being a lot less ergonomic (Lens.value3 vs Accessors.Tuple5.part3/three/third lens)? I can think of

erlandsona commented 2 years ago

Ah I hadn't seen VariantLens.GenerateUsed.tupleNest and requiring the user to choose one makes sense 👌

Tuples can (mu,(s,t)) be destructured exhaustively while records can't (honestly the biggest pain point in elm)

Not entirely sure how you mean

case tup of
   (firstPart, _) -> -- do stuff... and rest can be ignored

case rec of
    { firstPart } -> 

Both of these are "Exhaustive" in that they are ignoring the rest of the structure...

apart from the latter one being a lot less ergonomic (Lens.value3 vs Accessors.Tuple5.part3/three/third lens)?

Actually at first I wasn't sure why you felt like the tuples are less ergonomic (except for the general dogma which I also subscribe too 😅 )... BUT I just realized, if you're serializing the arguments into an "anonymous record" giving names to the positions of the arguments then you can just use the regular extensible record based Lenses! And yeh that'd be WAY more ergonomic.

lue-bird commented 2 years ago

Maybe I'm using "exhaustive" wrong, but what I meant is:

case maybe of
    Just content -> ...
-- error, you forgot `Nothing`
errorToString = \error ->
    let { expectation } = error
    in
    "Error! I expected " ++ expectation
    -- no error, even though you forgot `error.context` and `error.location`