search

stil4m / elm-syntax

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

Incremental changes for the switch to a Pratt parser #212

Closed jiegillet closed 2 weeks ago

jiegillet commented 2 weeks ago

I was working on this for a while, it's quite a complex refactor, and I hit a number of dead ends. Instead of pushing myself until I got everything right, I thought I would show the changes I'm most confident about, of course I'm happy to discuss them. I also realized that it's been a while since the original PR was open, so I don't expect a quick answer.

jfmengels commented 2 weeks ago

Thank you so much! I feel like you made some amazing progress! :tada:

Instead of pushing myself until I got everything right, ...

That's absolutely fine. Feel free to make plenty of small PRs, whatever makes you feel comfortable :+1:

I have a few questions above, but I'm happy to merge this as it is.

Contrary to what I said in the other PR, I ended up rebasing the pratt-parser branch on top of the main branch, and I rebased this branch on top of pratt-parser, while applying your (awesome) rule to every commit of the branch. Some changes in the tests are much easier to grok, which is a nice plus.

I could push the branch to your fork of the repo, but I didn' want to disturb your flow too much. So I have pushed a branch named pratt-parser-minus-jeroen. Feel free to update your branch to mine. git fetch ; git reset --hard origin/pratt-parser-minus-jeroen should do the trick, though maybe you need to replace origin by something else. After that, a git push --force-with-lease will be necessary.

I don't know if I'm over- or under-explaining Git to you, I'm trying to be as helpful as possible :sweat_smile: Let me know if you need more information though.

jiegillet commented 2 weeks ago

OK I added a few more things, and the number of failing tests is shrinking, but I think I'll leave this PR as it is, the rebase was a bit complex :D

I don't know if I'm over- or under-explaining Git to you, I'm trying to be as helpful as possible 😅

I'm quite familiar with git (I'm a professional dev after all) but the branch you made was very helpful indeed. Thank you.

jfmengels commented 2 weeks ago

Thank you so much for this @jiegillet, this is a big leap forward!