stil4m / elm-syntax

Elm syntax in Elm
MIT License
93 stars 28 forks source link

v8: don't split `Integer` and `Hex` #255

Open lue-bird opened 1 month ago

lue-bird commented 1 month ago

just like string literals which can be constructed using " or """, whole number literals can be constructed using hexadecimal or decimal. v8 introduces an extra variant parameter on the string variant to differentiate the two instead of introducing a new variant. This makes sense since the way these are constructed is usually irrelevant when matching for strings. Not having separate variants makes this easy and prevents the mistake to forget e.g. the """ case.

The same argument applies to hexadecimal vs decimal whole numbers, so I suggest merging the two as

| Integer IntegerBase Int

type IntegerBase
    = Hexadecimal -- alternatives: Base16, Hex
    | Decimal -- alternatives: Base10, Dec
jfmengels commented 1 month ago

I agree with the idea of merging Integer and Hex.

There is also the proposal in https://github.com/stil4m/elm-syntax/issues/108, where we store the raw string, like Integer 1 "00001", which opens up for a few things. What I don't know yet is whether we need the IntegerBase to figure out whether a value was written in hexadecimal, given that one could then check the string and see whether it starts with 0x or not. I'm thinking yes?

lue-bird commented 1 month ago

My current thinking is that "having exactly enough information to produce elm-formatted code" is a great line for what to include in the syntax tree. That means that it should be impossible to for example recover whether the original String in a string expression is "\''" or "'\'" since elm-format would it convert it to "''" anyway. And that's how we also do it currently. Why would whole numbers be a special case? Large numbers or initial 0s get corrected by elm-format anyway.