elm-syntax
elm-syntax copied to clipboard
V8: Use NonEmptyList
In the V8 branch (https://github.com/stil4m/elm-syntax/pull/188), a number of fields and custom type variant data have switched to create a guarantee that these things are never an empty list (ex: a record update without any field assignments).
The way it is done is inconsistent though:
-
Expression.RecordUpdateExpression
uses two variant items:RecordUpdateExpression (Node String) (Node RecordSetter) (List (Node RecordSetter))
-
Exposing.Explicit
uses two variant itemsExplicit (Node TopLevelExpose) (List (Node TopLevelExpose))
-
Expression.Lambda
uses two record fields{ ..., firstArg : Node DestructurePattern, restOfArgs : List (Node DestructurePattern) }
- Custom type declarations uses two record fields
firstConstructor
andrestOfConstructors
The idea of making sure we can't have an empty list is great, but this should be consistent.
Proposal
My proposal is to use a NonEmptyList
defined as type alias NonEmpty a = ( a, List a )
. I'm not entirely sure where to define it, probably in a separate moduleElm.Syntax.NonEmptyList
that only contains that definition. In practice people can then use a definition from other places if they like to such as turboMaCk/non-empty-list-alias
. This would have several benefits.
- If we apply this everywhere, things would be more consistent.
- The breaking change would be smaller. We'd have
Container (List X)
switch toContainer (NonEmptyList X)
andContainer { things : List X }
switch toContainer { things : NonEmptyList X }
, which is not a large change in terms of API. Maybe it's a bit of a moot point but still. - We would be grouping the first element and the second in a value that can be passed around easily. I imagine that we will often have functions that take the whole list of arguments for instance. With the current v8 API for Lambda, we'd probably often recreate the nonempty structure in the function call to keep the guarantee:
Lambda { firstArg, restOfArgs } -> fn ( firstArg, restOfArgs )
. With this approach, we could keep the call just like today, we'd only need to have the function take a NonEmptyList instead. - There'd be less of a need to guess the name of the different fields. Is it
firstArg
orfirstArgument
? - Users could use https://package.elm-lang.org/packages/turboMaCk/non-empty-list-alias/latest/ and other functions that work with tuples as non empty list, without having this package depend on a specific implementation of
NonEmptyList
(so less needed dependencies)
What do you think? Should we migrate all of our variants to use this NonEmptyList
? Can you think of counter arguments?
I guess this is a follow up and/or reproposal of https://github.com/stil4m/elm-syntax/issues/43
This seems reasonable to me. Though, at the risk of bike-shedding, I prefer the name Nonempty
over NonEmpty
or NonEmptyList
. It's short while being still clear in its meaning and doesn't require me to press the shift key in the middle of typing.
I'm willing to be convinced, and I agree with the pain of needing to press shift. But I wonder what the "grammatically" correct name is, and the library I linked to uses NonEmpty
as the convention, so I'm not sure :/
For Nonempty
vs NonemptyList
, I guess it makes sense, since we're unlikely to have a NonemptySet
in this repo.
If turboMaCk/non-empty-list-alias didn't already use NonEmpty
for the same type, I'd prefer FilledList
or ListFilled
Filled in my mind means that it's full, that there's no more space to add something. "Non empty" fits better in my opinion, and it's been used by a bunch of other people so it's less surprising.
An argument in favor of NonEmptyList
is that it might not be immediately obvious in the docs what it is, if you see NonEmpty Case
for instance. It doesn't help that the packages website won't turn NonEmptyList
into a link to the type alias (it only does so for custom types if I'm not mistaken).
Okay this is too much bikeshedding for me. I'm fine with NonEmptyList!
Strong approve of this. Tupley nonempty lists are also better for pattern matching!
Expression.Case {...}
can only be pattern matches shallowly, Expression.Case (Node Expression) (NonEmptyList (Node Expression))
can be pattern matched deeply