avh4 / elm-format

elm-format formats Elm source code according to a standard set of rules based on the official Elm Style Guide
BSD 3-Clause "New" or "Revised" License
1.31k stars 148 forks source link

Sort case branches to match Msg constructor ordering (only in update functions?) #823

Open KasMA1990 opened 1 year ago

KasMA1990 commented 1 year ago

In my day-to-day writing Elm code, I can only think of one significant place where I don't have autoformatting today, but wish I had: specifying the order of my Msg constructors in my update functions. That is, if my update function looks like this:

update msg model =
    case msg of
        A -> ...
        B -> ...
        C -> ...

then I know that the A,B,C ordering is something I've defined somewhere.

This usually comes up when I want to add new constructors to an existing Msg type. Often those new constructors relate to some existing messages, so I probably want to group them. The most natural place to do so, I find, is with the type definition itself. So although a type like:

type Msg 
    = A
    | B
    | C

looks fine, and it's easy to keep update using the same ordering, it becomes more annoying to keep them in sync when the type has morphed into

type Msg 
    = A
    -- Initialisation messages 
    | B
    | D
    -- Drag and drop impl messages
    | C
    | E
    | F
...

and it would be really nice if there was a way to just keep update in sync with an ordering like this. Some caveats though:

Of course, those two points can make it tricky to unite this feature with elm-format, because it eschews configuration and special cases.

On the other hand, elm-format already has this feature for a different kind of ordering: formatting exports as specified in module docs. So maybe there is room for something similar for Msg and update? There's lots of room for bikeshedding here, so I won't make concrete proposals for how it should look before testing if this is something anyone besides me would be interested in 😊

avh4 commented 1 year ago

My initial thought is that this is probably a bit beyond the scope of elm-format, as I'm a bit more hesitant to move blocks of code around than I am about the existing features of sorting imports and exposing clauses. Also, as you note, this may not be a good idea for all types and cases -- but then I think having special behavior for very specific code patterns is also not really in line with elm-format's general philosophy. Maybe some users have reason to have types and functions that are very similar to Msg/update but with different names, or maybe reason to have types called Msg or functions called update that are used for a slightly different purpose and they don't want this rule to apply. So I'd also be a bit hesitant about this given that it may be hard to determine a rule that will both satisfy most users, and avoid being confusing to folks who just want to use elm-format and not have to learn much about how to make it do what they expect.

However, I'm open to further discussion.

Also, FYI, there appears to be an existing elm-review rule that implements this https://package.elm-lang.org/packages/SiriusStarr/elm-review-no-unsorted/1.1.4/NoUnsortedCases/ so you could try that out and see if it either already solves your problem, or use it to learn a bit more about the details of how this should best behave.

jfmengels commented 1 year ago

I agree that this is probably not best suited for elm-format, if only for the fact that saving might introduce large changes in an editor when running elm-format.

Limiting this to functions named update (what about derivatives like updateHelp?) is also non-elm-format like. I agree that @SiriusStarr's elm-review rule is probably a better solution (though I have not tried it out before).

SiriusStarr commented 1 year ago

Yep, this is exactly what NoUnsortedCases does, so that should work fine. Please let me know if you have questions or run into any issues with it.