Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
KasMA1990 opened this issue Sep 7, 2023 · 3 comments

Comments

@KasMA1990
Copy link

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:

  • I think I would only want this for Msg types and the update function, and not in general for all custom types everywhere.
  • I get that this may not be the right decision for everyone, so it might be worth having as something you opt into.

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
Copy link
Owner

avh4 commented Sep 7, 2023

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.

@avh4 avh4 added the discussion label Sep 7, 2023
@avh4 avh4 changed the title Ordering of Msg constructors Sort case branches to match Msg constructor ordering (only in update functions?) Sep 7, 2023
@jfmengels
Copy link

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants