elm-format icon indicating copy to clipboard operation
elm-format copied to clipboard

Transform `case .. then` into `case .. of` (lenient-parser) 🛠

Open kutyel opened this issue 2 years ago • 4 comments

Fixes #766


Sorry about the git rewrite @avh4 🙈 I just changed my old work email (which no longer exists) to my gmail personal account so that previous contributions to the project are still counted 🙏🏻 (make sure to squash and merge this if you accept it 😳).

kutyel avatar Apr 26 '22 09:04 kutyel

I think you messed up your merge/rebase :sweat_smile:

jfmengels avatar Apr 26 '22 10:04 jfmengels

I think you messed up your merge/rebase 😅

Yup, definitely 🙈🤣. As I explained in the first comment on the PR, the only thing that changes is the meta-information in the commits to replace my older work-mail, by my current personal email address. Wish there was a better way to do this within git 🤦🏻

kutyel avatar Apr 26 '22 11:04 kutyel

You can maybe start a new branch on top of master, then cherry-pick the commits, and rename your branch to the name of the one behind this PR.

git checkout -b tmpbranchname
git cherry-pick 943cc8833f886a667a7b899a0e919b0e00b18662 192ea4c80f1fcac34361678d5d3d294459bd534d 9c49caaf2363cb3d888b8f5117352ad20e04935c
git branch -M fix/lenient-parser-case-then
# Then force-push the branch

jfmengels avatar Apr 26 '22 12:04 jfmengels

You can maybe start a new branch on top of master, then cherry-pick the commits, and rename your branch to the name of the one behind this PR.

git checkout -b tmpbranchname
git cherry-pick 943cc8833f886a667a7b899a0e919b0e00b18662 192ea4c80f1fcac34361678d5d3d294459bd534d 9c49caaf2363cb3d888b8f5117352ad20e04935c
git branch -M fix/lenient-parser-case-then
# Then force-push the branch

Yes but if I do that my previous contributions to elm-format will remain lost (as they are now), so I'm letting @avh4 decide wether he lets me correct the previous commit's author (mine) or not 😉

kutyel avatar Apr 26 '22 12:04 kutyel

To to self: :+1: confirmed that current elm-compiler does prohibit use of "of" as an expression: https://github.com/elm/compiler/blob/770071accf791e8171440709effe71e78a9ab37c/compiler/src/Parse/Expression.hs#L253-L258 -> https://github.com/elm/compiler/blob/770071accf791e8171440709effe71e78a9ab37c/compiler/src/Parse/Expression.hs#L35 -> https://github.com/elm/compiler/blob/770071accf791e8171440709effe71e78a9ab37c/compiler/src/Parse/Expression.hs#L77 -> https://github.com/elm/compiler/blob/770071accf791e8171440709effe71e78a9ab37c/compiler/src/Parse/Variable.hs#L193-L194 -> https://github.com/elm/compiler/blob/770071accf791e8171440709effe71e78a9ab37c/compiler/src/Parse/Variable.hs#L73

Just a side-note that if a future Elm version ever removes "then" from the list of reserved words, then this feature would need to be removed.

avh4 avatar Mar 11 '23 23:03 avh4

Ah, okay, so I think I'm understanding the git history here:

@kutyel these will need to be fixed before I can merge:

  • (not sure if this applies or not) I may have told you a different branch to base off of in the past, but if I did, it's one that I didn't end up merging. So this needs to be based off of main if it's not currently
  • the messy rebase is from trying to fix the author of your previous commits from Oct-Dec 2020, right? I think we need to remove that from this PR. Even then, I don't think I'll accept rewriting or duplicating all of history, but if you want to try making a separate PR that also only affects your old commits, then I'd be up for merging that.
    • idea for that separate PR: I think if you take those 2020 commits and just rewrite them on the same base commit that they're already on, then make a merge commit between that and current main that ends up with the same file contents as main, then that should end up making new copies of your old commits with your new author name, and also avoid rewriting anyone else's commits. If you want to make that as a new PR, I can merge that.

avh4 avatar Mar 11 '23 23:03 avh4

@avh4 Ok, thanks for the feedback, will create a new PR without all the git noise 👌🏻

kutyel avatar Mar 13 '23 10:03 kutyel