Add configuration options for sorting deriving clauses and types by michaelpj · Pull Request #434 · fourmolu/fourmolu

michaelpj · 2024-10-06T14:24:37Z

This adds two configuration options for sorting a) types in deriving clauses, and b) deriving clauses themselves.

github-actions · 2024-10-06T14:24:50Z

👋 @michaelpj
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/
Docs have been updated in web/site/pages/config/
Tests have been added
Diff files in compat-tests/ either:
1. Have NOT been changed, or
2. Modifies fourmolu.yaml with an appropriate option to keep the same formatting, or
3. Modifies Haskell files with new formatting
  - ONLY as a last resort - see Breaking changes policy

michaelpj · 2024-10-06T14:25:30Z

+  let sortedDeriving = if sortDeriving then sortOn (derivingStrategyKey . fmap unLoc . deriv_clause_strategy . unLoc) dd_derivs else dd_derivs
+  inci $ sep newline (located' p_hsDerivingClause) sortedDeriving
+  where
+    derivingStrategyKey Nothing = ClauseNoStrategy


I considered just sorting on showOutputable again, but that gives an order that's a bit different to what I've most often seen in the wild, which is what I've replicated here.

brandonchinn178 · 2024-10-07T03:01:24Z

Also some CI failures

michaelpj · 2024-10-07T16:28:16Z

CI fixed. I think mostly just up to you to make a call on what you'd like to do wrt defaults etc. Or I'm happy for you to just change it post-merge!

I think I'm not super keen on the "sort unless there are comments" idea, what do you think?

brandonchinn178

Update changelogs + finish the last stuff around defaults, and this should be good to go!

Changing the name to sort-derived-types is optional

georgefst

Unfortunately I feel quite strongly that both should be off by default.

michaelpj · 2024-10-13T14:30:55Z

Back to off by default. I added some tests with comments. I was able to get some weird behaviour with sort-deriving-clauses but actually sort-derived-classes mostly seems to do the right thing. I'm not sure of it, and upstream seems wary, so I've left in the cautious wording for now.

michaelpj · 2024-10-13T19:37:04Z

I also fixed some more issues in the AST comparison. I can now run this on the whole work codebase without issues.

brandonchinn178

Looks good to me! @georgefst?

brandonchinn178 · 2024-10-14T14:31:47Z

@michaelpj Can you fix lints and also rebase? I fixed CI on main

michaelpj · 2024-10-16T08:25:53Z

Fixed and rebased.

michaelpj · 2024-10-16T12:28:30Z

Not sure what's causing this failure.

michaelpj · 2024-11-02T16:01:16Z

Are you both happy with this?

brandonchinn178 · 2024-11-02T16:07:51Z

@michaelpj Yeah, good to go. Sorry, can you try rebasing one more time? That CI failure should be fixed

michaelpj · 2024-11-03T19:25:21Z

Rebased on main but it still seems broken?

brandonchinn178 · 2024-11-04T04:25:02Z

Ok sorry, can you try one last time? 🤞

This adds two configuration options for sorting a) types in deriving clauses, and b) deriving clauses themselves. Fixes tweag/ormolu#968

michaelpj · 2024-11-04T09:34:44Z

That worked!

## Fourmolu 0.17.0.0 * Add new `import-grouping` option to group imports with grouping rules specified in configuration ([#403](fourmolu/fourmolu#403)) * Add new `sort-constraints` option to sort constraints alphabetically ([#433](fourmolu/fourmolu#433)) * Add new `sort-derived-classes` option to sort classes in deriving clauses ([#434](fourmolu/fourmolu#434)) * Add new `sort-derived-clauses` option to sort classes deriving clauses ([#434](fourmolu/fourmolu#434)) * Add new `trailing-section-operators` option to disable trailing "section" operators (those that are `infixr 0`, such as `$`) ([#444](fourmolu/fourmolu#444)) * Fix issue where `single-constraint-parens: never` would drop parentheses around implicit parameters ([#446](fourmolu/fourmolu#446)) * Fix indentation for parenthesized expressions that start off the indentation column ([#428](fourmolu/fourmolu#428)) * Allow multiline comments in indented contexts ([#65](fourmolu/fourmolu#65)) ## Fourmolu 0.16.2.0 ### Upstream changes: #### Ormolu 0.7.7.0 * Use single-line layout for parens around single-line content. [Issue 1120](tweag/ormolu#1120). * Allow function arguments to be on the same line even if the full type (with constraints and foralls) are on multiple lines. [PR 1125](tweag/ormolu#1125). ## Fourmolu 0.16.1.0 ### Upstream changes: #### Ormolu 0.7.6.0 * Fix Haddock comments on infix constructors [Issue 758](tweag/ormolu#758). * Don't require a trailing newline in `.ormolu` files. [Issue 1122](tweag/ormolu#1122). * Remove unnecessary indentation from list comprehensions. [Issue 966](tweag/ormolu#966). ## Fourmolu 0.16.0.0 * Allow specifying path to configuration file with `--config` ([#396](fourmolu/fourmolu#396)) ### Upstream changes: #### Ormolu 0.7.5.0 * Switched to `ghc-lib-parser-9.10`, with the following new syntactic features/behaviors: * GHC proposal [#575](https://github.com/ghc-proposals/ghc-proposals/blob/10290a668608d608c3f6c6010be265cf7a02e1fc/proposals/0575-deprecated-instances.rst): deprecated instances. * GHC proposal [#281](https://github.com/ghc-proposals/ghc-proposals/blob/10290a668608d608c3f6c6010be265cf7a02e1fc/proposals/0281-visible-forall.rst): visible forall in types of terms. Enabled by `RequiredTypeArguments` (enabled by default). * `LinearTypes`: `let` and `where` bindings can now be linear, in particular have multiplicity annotations. * Using `forall` as an identifier is now a parse error. * GHC proposal [#65](https://github.com/ghc-proposals/ghc-proposals/blob/10290a668608d608c3f6c6010be265cf7a02e1fc/proposals/0065-type-infix.rst): namespacing fixity declarations for type names and WARNING/DEPRECATED pragmas. * `TypeAbstractions` now supports `@`-binders in lambdas and function equations. * Support for the `GHC2024` language. * Updated to `Cabal-syntax-3.12`.

michaelpj commented Oct 6, 2024

View reviewed changes

brandonchinn178 reviewed Oct 6, 2024

View reviewed changes

Comment thread src/Ormolu/Printer/Meat/Declaration/Data.hs Outdated

Comment thread src/Ormolu/Printer/Meat/Declaration/Data.hs Outdated

Comment thread config/FourmoluConfig/ConfigData.hs

michaelpj force-pushed the mpj/sort-deriving branch from 8cf97fb to df5d033 Compare October 6, 2024 21:14

brandonchinn178 approved these changes Oct 7, 2024

View reviewed changes

Comment thread web/site/pages/config/sort-deriving-clauses.md

Comment thread web/site/pages/config/sort-derived.md Outdated

michaelpj force-pushed the mpj/sort-deriving branch from df5d033 to ed3f554 Compare October 7, 2024 16:21

brandonchinn178 approved these changes Oct 9, 2024

View reviewed changes

Comment thread changelog.d/sort-derived.md Outdated

Comment thread web/site/pages/config/sort-derived.md Outdated

Comment thread web/site/pages/config/sort-deriving-clauses.md

georgefst requested changes Oct 10, 2024

View reviewed changes

Comment thread config/FourmoluConfig/ConfigData.hs

Comment thread web/site/pages/config/sort-deriving-clauses.md

michaelpj force-pushed the mpj/sort-deriving branch from ed3f554 to 9133d2f Compare October 13, 2024 14:29

michaelpj force-pushed the mpj/sort-deriving branch from 9133d2f to 5761e74 Compare October 13, 2024 19:36

brandonchinn178 approved these changes Oct 13, 2024

View reviewed changes

Comment thread src/Ormolu/Diff/ParseResult.hs

Comment thread src/Ormolu/Diff/ParseResult.hs Outdated

michaelpj force-pushed the mpj/sort-deriving branch from 5761e74 to 3b42667 Compare October 14, 2024 09:01

georgefst self-requested a review October 14, 2024 09:22

georgefst approved these changes Oct 14, 2024

View reviewed changes

michaelpj force-pushed the mpj/sort-deriving branch from 3b42667 to 38bcfe0 Compare October 16, 2024 08:25

michaelpj force-pushed the mpj/sort-deriving branch from 38bcfe0 to c369a69 Compare October 16, 2024 08:27

michaelpj force-pushed the mpj/sort-deriving branch from c369a69 to 4d82f83 Compare November 2, 2024 16:09

Add configuration options for sorting deriving clauses and types

799beb8

This adds two configuration options for sorting a) types in deriving clauses, and b) deriving clauses themselves. Fixes tweag/ormolu#968

michaelpj force-pushed the mpj/sort-deriving branch from 4d82f83 to 799beb8 Compare November 4, 2024 08:58

brandonchinn178 merged commit 7805631 into fourmolu:main Nov 4, 2024

BrewTestBot mentioned this pull request Jan 15, 2025

fourmolu 0.17.0.0 Homebrew/homebrew-core#204316

Merged

Conversation

michaelpj commented Oct 6, 2024

Uh oh!

github-actions Bot commented Oct 6, 2024 • edited by brandonchinn178 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelpj Oct 6, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brandonchinn178 commented Oct 7, 2024

Uh oh!

michaelpj commented Oct 7, 2024

Uh oh!

brandonchinn178 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

georgefst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

michaelpj commented Oct 13, 2024

Uh oh!

michaelpj commented Oct 13, 2024

Uh oh!

brandonchinn178 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

brandonchinn178 commented Oct 14, 2024

Uh oh!

michaelpj commented Oct 16, 2024

Uh oh!

michaelpj commented Oct 16, 2024

Uh oh!

michaelpj commented Nov 2, 2024

Uh oh!

brandonchinn178 commented Nov 2, 2024

Uh oh!

michaelpj commented Nov 3, 2024

Uh oh!

brandonchinn178 commented Nov 4, 2024

Uh oh!

michaelpj commented Nov 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Oct 6, 2024 •

edited by brandonchinn178

Loading