-
Notifications
You must be signed in to change notification settings - Fork 59
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 a single-context-parens option (#288) #304
Conversation
👋 @sgillespie Reviewer: Please verify the following things have been done, if applicable.
|
OK, I just realized I didn't add the documentation above. Working on that now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Some minor comments, but should be good to go!
changelog.d/single-context-parens.md
Outdated
@@ -0,0 +1,3 @@ | |||
* Add a `single-constraint-parens` for controlling parenthesis around constraints in type | |||
signatures. See [issue 288](https://github.com/fourmolu/fourmolu/pull/304) | |||
* https://github.com/fourmolu/fourmolu/pull/304 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This is already linked in the first bullet, no need to add a second
README.md
Outdated
| `unicode` | `always`, `detect`, **`never`** | Output Unicode syntax. With `detect` we output Unicode syntax exactly when the extension is seen to be enabled. When using `always`, make sure to have the `UnicodeSyntax` extension enabled, or Fourmolu will throw errors. | ||
| `respectful` | **`true`**, `false` | Be less aggressive in reformatting input, e.g. keep empty lines in import list | ||
| `fixities` | list of strings (**`[]`**) | See the "Language extensions, dependencies, and fixities" section below | ||
| `single-constraint-parens`|**`always`**, `never`, `auto` | How to style optional parentheses around single constraints (`auto` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an unclosed parenthesis here. Also, can you add spaces around this pipe 🙃
| `single-constraint-parens`|**`always`**, `never`, `auto` | How to style optional parentheses around single constraints (`auto` | |
| `single-constraint-parens` | **`always`**, `never`, `auto` | How to style optional parentheses around single constraints (`auto` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback!
I've updated the changelog.d/ file and updated the formatting in the README. I did not add an explanation of all the options as I felt they were fairly straightforward, but please let me know if you disagree.
I'm not sure how you guys like to structure commits, so let me know if you want me to squash them into one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super small nit. If you don't get to it, I'll just merge and can fix at release.
Feel free to squash, or I can just squash when merging the PR. Thanks!
README.md
Outdated
| `unicode` | `always`, `detect`, **`never`** | Output Unicode syntax. With `detect` we output Unicode syntax exactly when the extension is seen to be enabled. When using `always`, make sure to have the `UnicodeSyntax` extension enabled, or Fourmolu will throw errors. | ||
| `respectful` | **`true`**, `false` | Be less aggressive in reformatting input, e.g. keep empty lines in import list | ||
| `fixities` | list of strings (**`[]`**) | See the "Language extensions, dependencies, and fixities" section below | ||
| `single-constraint-parens` | **`always`**, `never`, `auto` | How to style optional parentheses around single constraints |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think this would sound much better as "Whether to style optional parentheses ..."? "How" is a bit ambiguous
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, updated, and squashed
This option controls parenthesis around constraints in type signatures when there is only one:
becomes
This addresses #288. Note that always is the default, even though the issue suggests auto (ignore) be the default. I did this because Ormolu always adds parenthesis, and it would break some other tests.