Implement single-deriving-parens#386
Conversation
|
👋 @pbrisbin Reviewer: Please verify the following things have been done, if applicable.
|
|
|
I'm a little skeptical of the |
|
@pbrisbin if you read the README, it should explain 🙂 Basically, every release, we move everything in changelog.d into CHANGELOG.md. The point of the directory is to avoid merge conflicts when multiple contributors append to the changelog in different PRs |
|
Thanks, added. |
brandonchinn178
left a comment
There was a problem hiding this comment.
Thanks for putting together such a well-organized PR! I just have a question about why the logic is in Parser and not Printer
|
OK, this is ready for another look. I left the history messy, but can (would prefer to) rebase that before merge, once you let me know the change is good. I also included some more HLint-related changes that were directly related to what I was touching. I'm happy to leave that out if you prefer. |
brandonchinn178
left a comment
There was a problem hiding this comment.
💯 Much better! Please revert the HLint changes, then feel free to rebase and push
492e0c7 to
6cc84b6
Compare
The current implementation of this function relies on the fact that we never output un-parenthesised deriving classes from formatting. This means the right-hand argument is never a `DctSingle`. If it ever were, the function would incorrectly return a `Difference` no matter what, even if it were the exact same `DctSingle` as the left-hand argument. Since we're about to start making those parenthesis conditional, this won't work. Instead, we now use `considerEqualVia` and pattern match on a comparison of single-to-singleton or singleton-to-single. I find this a conceptually simpler approach anyway, and is more lenient in the way we need.
6cc84b6 to
2c66260
Compare
brandonchinn178
left a comment
There was a problem hiding this comment.
Looks awesome, thanks! 🚀
|
Definitely looks like my fault on the website build failure -- not quite sure what I'm missing. |
|
@brandonchinn178 everything looks green now :) Would you like me to clean up the history before merge? |
|
Yes please! |
Analogous to `single-constraint-parens`, this option decides how
deriving clauses should parenthesis single-element lists.
The implementation logic is as follow:
- If the user typed `Show` and we are configured for "always", use
parenthesis. Otherwise ("never" or "auto"), don't.
- If the user typed `(Show)` and we are configured for "never", don't
use parenthesis. Otherwise ("always" or "auto"), do.
611f9dc to
68a4dc3
Compare
|
Done 😄 |
As discussed in #385.
Make derivedTyClsParensEq more rebust
33c7a8c
The current implementation of this function relies on the fact that we
never output un-parenthesised deriving classes from formatting. This
means the right-hand argument is never a
DctSingle. If it ever were,the function would incorrectly return a
Differenceno matter what,even if it were the exact same
DctSingleas the left-hand argument.Since we're about to start making those parenthesis conditional, this
won't work. Instead, we now use
considerEqualViaand pattern match ona comparison of single-to-singleton or singleton-to-single. I find this
a conceptually simpler approach anyway, and is more lenient in the way
we need.
Implement single-deriving-parens
2c8645b
Analogous to
single-constraint-parens, this option decides howderiving clauses should parenthesis single-element lists.
The implementation logic is as follow:
If the user typed
Showand we are configured for "always", useparenthesis. Otherwise ("never" or "auto"), don't.
If the user typed
(Show)and we are configured for "never", don'tuse parenthesis. Otherwise ("always" or "auto"), do.
Add changelog snippet for single-deriving-parens
2c66260