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

Adopt diff-friendly import style. #4173

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

jonathanknowles
Copy link
Member

@jonathanknowles jonathanknowles commented Oct 19, 2023

Summary

This PR adjusts our stylish-haskell configuration to always import each symbol on a separate line. Example:

import Some.Module.A
    ( SomeSymbolA1
    , SomeSymbolA2
    )
import Some.Module.B
    ( SomeSymbolB1
    , SomeSymbolB2
    , SomeSymbolB3
    , SomeSymbolB4
    )

Details

This change has several advantages:

  1. Better cross-version compatibility: this configuration produces almost exactly the same output across versions 0.11.0.3 and 0.14.5.0 of stylish-haskell (current and latest versions respectively).
  2. Better cross-tool compatibility: the output is almost identical to that produced by fourmolu. (See experimental fourmolu branch here.)
  3. Improved diff-friendliness: we virtually eliminate the problem where adding or removing a single import can lead to a multi-line diff.

Adopting this style will make it easier to move to GHC 9.2 or 9.6, both of which no longer work with our current version of stylish-haskell.

Related Issues

Without this change in our configuration, upgrading stylish-haskell from version 0.11.0.3 to 0.14.5.0 exposes us to the following regression:
haskell/stylish-haskell#462

@jonathanknowles jonathanknowles self-assigned this Oct 19, 2023
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/diff-friendly-import-style branch from 9ee843c to 4c69133 Compare October 19, 2023 07:14
@jonathanknowles jonathanknowles changed the title [Experimental] Use diff-friendly import style. Use diff-friendly import style. Oct 19, 2023
@jonathanknowles jonathanknowles marked this pull request as draft October 19, 2023 11:39
@jonathanknowles jonathanknowles changed the title Use diff-friendly import style. Adopt diff-friendly import style. Oct 19, 2023
@@ -4,7 +4,7 @@
# https://github.com/jaspervdj/stylish-haskell/blob/master/data/stylish-haskell.yaml
# for usage.

columns: 80 # Should match .editorconfig
columns: 1 # Force diff-friendly import style (one line per symbol).
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this trick works with both older and more recent versions of stylish-haskell.

( XPub, xpubToBytes )
( XPub
, xpubToBytes
)
Copy link
Member Author

Choose a reason for hiding this comment

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

This style has the final ) on a separate line -- this is the same as fourmolu.

@jonathanknowles jonathanknowles force-pushed the jonathanknowles/diff-friendly-import-style branch from 4c69133 to bf69969 Compare October 20, 2023 01:22
@jonathanknowles jonathanknowles marked this pull request as ready for review October 20, 2023 01:23
This commit changes our `stylish-haskell` configuration so that each
imported symbol is listed on a separate line.

This is identical to the style used by `fourmolu`.

This configuration is respected by older and newer versions of
`stylish-haskell` (specifically, versions `0.11.0.3` and `0.14.5.0`).
Copy link
Member

@Anviking Anviking left a comment

Choose a reason for hiding this comment

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

lgtm

@jonathanknowles jonathanknowles added this pull request to the merge queue Oct 20, 2023
Merged via the queue into master with commit b534f20 Oct 20, 2023
3 checks passed
@jonathanknowles jonathanknowles deleted the jonathanknowles/diff-friendly-import-style branch October 20, 2023 08:40
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.

2 participants