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

Lambda enclosed in aligned parens to support Elm language server parsing #13

Merged

Conversation

JSuder-xx
Copy link

@JSuder-xx JSuder-xx commented Mar 31, 2022

As part of this item I have need to generate some Elm API endpoints from Rostering.

The current generation breaks the parsing of the Elm Language server that is used by VS Code (at least, probably other setups as well) and this leads to a really bad developer experience. Not only are errors reported in the generated files but also any Elm files that depend on those generated artifacts (ex. Curriculum).

This PR attempts to fix it by simply emitting lambdas wrapped in column aligned parenthesis.

It is a really small change. I updated 1 code file and 11 expectations.

Prior to the Fix

image

After the Fix

image

@JSuder-xx JSuder-xx requested a review from zwilias March 31, 2022 16:06
Copy link
Member

@zwilias zwilias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to let the tests do their thing (assuming CI works), but honestly, this looks good! (At least, the updated "golden" tests look good!)

In the meantime, you can also use this locally by running niv update servant-elm -b kraken/emit-expect-compatible-with-elm-language-server within the tooling/ directory (and running firstaide-update right after).

That should at least tell you whether changes look good (enough), or whether further tweaks are needed.

Feel free to poke me if you want a hand with that!

@JSuder-xx
Copy link
Author

It worked as intended! See this PR.

@zwilias
Copy link
Member

zwilias commented Apr 1, 2022

Sweet! I'll go ahead and merge this and put in a PR to update the monorepo 👍

@zwilias zwilias merged commit 6cda93e into master Apr 1, 2022
@zwilias zwilias deleted the kraken/emit-expect-compatible-with-elm-language-server branch April 1, 2022 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants