Skip to content

Add let-style configuration#229

Merged
brandonchinn178 merged 9 commits intomainfrom
let-style
Sep 28, 2022
Merged

Add let-style configuration#229
brandonchinn178 merged 9 commits intomainfrom
let-style

Conversation

@brandonchinn178
Copy link
Copy Markdown
Collaborator

@brandonchinn178 brandonchinn178 commented Aug 14, 2022

Resolves #202
Supercedes #77

Still definitely some kinks that could be worked out, especially around empty let blocks (ref: tweag/ormolu#917), let-blocks with multiple bindings on one line (e.g. let a = 1; b = 2), etc. But I think this is a good initial implementation.

Examples of each style can be found in the test directory. As mentioned in the issue (and in the changelog entry), I made auto-newline the default, which changes default formatting. IMO auto-newline has the best compromise between Fourmolu's goal of being diff-friendly and compactness. Happy to hear suggestions for another default (I don't think we should use inline-ormolu, though, because people are usually confused why in starts off-indentation when initially using Fourmolu, e.g. #152).

@github-actions
Copy link
Copy Markdown

👋 @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 let-style branch 4 times, most recently from cb825ee to 305033d Compare August 14, 2022 22:45
@brandonchinn178 brandonchinn178 changed the title [WIP] Add let-style configuration Add let-style configuration Aug 14, 2022
@brandonchinn178 brandonchinn178 marked this pull request as ready for review August 14, 2022 22:49
@georgefst
Copy link
Copy Markdown
Collaborator

georgefst commented Aug 16, 2022

I don't think we should use inline-ormolu, though, because people are usually confused

I'm gonna go out on a limb and guess that there are just as many people who like the current style, and haven't had reason to complain until now because it's the default (full disclosure, I'm one of them).

More generally, I think we should start being pretty conservative about making changes to default formatting. There are a lot of people using Fourmolu these days and these small changes create a lot of churn. Anecdotally, I know my colleagues would be grateful to hear this.

I am still happy to change defaults for things that are pretty inarguably currently far from ideal e.g. some of the ways in which commented-out code gets moved around, or haddocks with data constructor declarations.

(This also means we should maybe change the option names (sorry!). It seems odd for "ormolu" to be the default.)

@brandonchinn178 brandonchinn178 force-pushed the let-style branch 2 times, most recently from 109d51a to dad3d08 Compare August 24, 2022 06:44
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.

Configuration for styling let

2 participants