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

Remove check about immediately invoked lambdas with variable name #147

Merged
merged 1 commit into from
Sep 16, 2023

Conversation

jfmengels
Copy link
Owner

@jfmengels jfmengels commented Sep 15, 2023

Removed the check introduced in #124

This simplification is pretty nice, but has a few problems:

  • It stands out as a simplification because it does not have an autofix (because there are multiple solutions for it, relating to code style)
  • There are some edge cases where this simplification is not necessarily appropriate
    • Record updates f a |> (\v -> { v | a = 1 })
    • Extracting through destructuring f a |> (\(Thing thing) -> thing)

From feedback, people seem to think the idea is good and that it improves the readability of the code (with the exceptions that probably need some refinement), but I think it stands out too much (especially the non-fixing part) and should therefore be moved to a separate rule, probably in jfmengels/elm-review-code-style, where autofix could be supported by configuring the rule (PRs welcome 😄 )

@jfmengels jfmengels merged commit 5d3a403 into main Sep 16, 2023
4 checks passed
@jfmengels jfmengels deleted the remove-direct-lambda-calls branch September 16, 2023 14:40
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.

1 participant