stil4m / elm-syntax

Elm syntax in Elm
MIT License
92 stars 26 forks source link

Proposal: Remove records in variants #193

Open jfmengels opened 10 months ago

jfmengels commented 10 months ago

For most variants of the different types defined in this package, we use separate elements. For instance, for if we have Expression.If (Node Expression) (Node Expression) (Node Expression), and I think this works quite well even if they're not named.

But for a few of our variants, we use records hidden behind aliases: (I'll use the Nonempty notation from #192)

Expression.LambdaExpression Lambda, where Lambda = { arguments : NonEmpty (Node DestructurePattern), expression : Node Expression }. We could instead have Expression.Lambda (NonEmpty (Node DestructurePattern)) (Node Expression). Removing the definition for Lambda will also allow to rename LambdaExpression to Lambda.

Expression.Case CaseBlock, where CaseBlock = { expression : Node Expression, cases : NonEmpty Case }. We could instead have Expression.Case (Node Expression) (NonEmpty Case). Case is defined as ( Node Pattern, Node Expression ), and I would actually like to remove it too as I find it more confusing than helpful. That would lead us to Expression.Case (Node Expression) (NonEmpty ( Node Pattern, Node Expression ))

Expression.Let LetBlock, where LetBlock = { declarations : NonEmpty (Node LetDeclaration), expression : Node Expression }. We could instead have `Expression.Let (NonEmpty (Node LetDeclaration)) (Node Expression). (TODO this one hasn't been turned into a non empty list yet, create an issue)

If we do flatten everything, then it might make sense to inline RecordSetter for Record and RecordUpdate.

I don't know if we should apply this logic to other types, I haven't looked at those yet.

miniBill commented 10 months ago

YES please. Those are so much better for pattern matching