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

Add leading-args option for function-arrows #248

Merged
merged 1 commit into from Nov 28, 2022

Conversation

brandonchinn178
Copy link
Collaborator

Blocked by #247
Resolves #233

@github-actions
Copy link

github-actions bot commented Nov 1, 2022

👋 @brandonchinn178
Thank you for raising your pull request.
Please make sure you have followed our contributing guidelines in DEVELOPER.md. We will review it as soon as possible!

Reviewer: Please verify the following things have been done, if applicable.

  • A file has been added to changelog.d/
  • "Configuration > Available options" section in README.md has been updated
  • "Configuration > Specifying configuration" section in README.md has been updated
  • fourmolu.yaml updated to stay in sync with config in README.md
  • Tests have been added

@brandonchinn178 brandonchinn178 force-pushed the function-arrows-leading-args branch 2 times, most recently from 719484e to 83ed553 Compare November 8, 2022 00:42
Comment on lines +9 to +12
bar ::
String
-> String
-> a
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The original issue suggested that this first argument would be indented like

   String
-> String
-> a

but it wasn't trivial to implement that exactly, and I don't see it as a big deal. If a first arg becomes the second arg, you have a diff either way:

-String
+Int
+-> String
 -> String
 -> a

-   String
+   Int
+-> String
 -> String
 -> a

@brandonchinn178 brandonchinn178 marked this pull request as ready for review November 8, 2022 00:47
@brandonchinn178
Copy link
Collaborator Author

cc @tchoutri would love to get your feedback here, given that you commented on the original issue.
cc @georgefst would also be interested in your thoughts

@brandonchinn178 brandonchinn178 force-pushed the function-arrows-leading-args branch 2 times, most recently from 9e3b6db to 20755b8 Compare November 10, 2022 22:23
Copy link
Collaborator

@georgefst georgefst left a comment

Choose a reason for hiding this comment

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

I don't ever intend to use this, so I won't comment on output. But the implementation looks fine.

@brandonchinn178
Copy link
Collaborator Author

Merging now, we can improve upon this later. (Specifically, would like to improve #269)

@brandonchinn178 brandonchinn178 merged commit 5ff37f1 into main Nov 28, 2022
@brandonchinn178 brandonchinn178 deleted the function-arrows-leading-args branch November 28, 2022 18:11
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.

Add 'only-args' option to 'function-arrows'
2 participants