Bump Fourmolu to 0.4 by georgefst · Pull Request #2254 · haskell/haskell-language-server

georgefst · 2021-10-04T09:29:21Z

Addresses the relevant checklist item in #297.

georgefst · 2021-10-04T09:31:04Z

+import qualified Data.Text                       as T
+import           Development.IDE                 hiding (pluginHandlers)
+import           Development.IDE.GHC.Compat      as Compat hiding (Cpp)
 import qualified Development.IDE.GHC.Compat.Util as S


I don't know how this file was passing CI before? Seeing as this line causes stylish-haskell to reformat all the other imports (I personally detest this behaviour, but that's an argument for another day).

georgefst · 2021-10-04T09:33:11Z

    , base            >=4.12 && <5
    , filepath
-    , fourmolu        ^>=0.3
+    , fourmolu        ^>=0.3 || ^>=0.4


Ideally, I'd remove 0.3 entirely, as 0.4 includes some important bug fixes. But this isn't possible while stylish-haskell is tied to older versions of GHC and Cabal.

(Same applies to Ormolu here - right now, the latest version will never be chosen by the dependency solver)

the latest version will never be chosen by the dependency solver

Actually, looks like this only applies to GHC < 9. The stylish-haskell plugin is disabled for 9.0.1.

And if CI passes here for GHC 9.0.1, we know that Fourmolu 0.4 is being used, since it's the first version to support it.

Maybe adding a comment about, to remove 0.3 some day?

Maybe adding a comment about, to remove 0.3 some day?

Actually, I think there's no need. I'll probably end up at least trying to remove it every time I bump the upper bound, and eventually it'll be possible.

jneira · 2021-10-04T09:37:48Z

@georgefst many thanks to spend time on fourmolu/fourmolu#104 and this, getting support for ghc-9

jneira · 2021-10-04T09:41:47Z

I hope you dont mind i enabled fourmolu for ghc-9 to give it a shot in ci

georgefst · 2021-10-04T09:43:09Z

@georgefst many thanks to spend time on fourmolu/fourmolu#104 and this, getting support for ghc-9

Thanks, but @brandonchinn178 has actually done most of the hard work recently.

And in fact, as for GHC 9 support, the work was done upstream in Ormolu.

jneira

lgtm i hope fourmolu will continue rocking in the formatter side

jneira · 2021-10-04T10:30:01Z

There is a build error for ghc-9:

 src/Ide/Plugin/Fourmolu.hs:20:1: error:
Error:     Could not find module ‘GhcPlugins’
    Perhaps you meant GHC.Plugins (from ghc-9.0.1)
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
   |
20 | import           GhcPlugins                      (HscEnv (hsc_dflags))
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

georgefst · 2021-10-04T10:53:54Z

There is a build error for ghc-9:

 src/Ide/Plugin/Fourmolu.hs:20:1: error:
Error:     Could not find module ‘GhcPlugins’
    Perhaps you meant GHC.Plugins (from ghc-9.0.1)
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
   |
20 | import           GhcPlugins                      (HscEnv (hsc_dflags))
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

That import should have been removed in ec53fcb. Fixed in 46d3264.

This would have been caught by -Wall, but for some reason that isn't enabled for many of the plugins?

jneira · 2021-10-04T10:56:59Z

This would have been caught by -Wall, but for some reason that isn't enabled for many of the plugins?

Jumm they should afaiu, maybe is cargo culting and otoh it is enabled for hls itself, maybe it is time to give a try at least here

jneira

what about ghc-options: -Wall -Wredundant-constraints, like hls itself?

(without -Wno-name-shadowing -Wno-unticked-promoted-constructors if this does not need it)

georgefst · 2021-10-04T11:14:45Z

-Wredundant-constraints

Meh, I think there are legitimate use cases for redundant constraints, though it's very unlikely to come up in this plugin anyway.

-Wno-name-shadowing

I prefer to be warned.

-Wno-unticked-promoted-constructors

No strong opinion, but again unlikely to come up.

On the whole, I'd go for keeping things simple with just -Wall. But I don't feel that strongly about any of it.

jneira · 2021-10-04T11:18:17Z

-Wredundant-constraints

Meh, I think there are legitimate use cases for redundant constraints, though it's very unlikely to come up in this plugin anyway.

-Wno-name-shadowing

I prefer to be warned.

-Wno-unticked-promoted-constructors

No strong opinion, but again unlikely to come up.

On the whole, I'd go for keeping things simple with just -Wall. But I don't feel that strongly about any of it.

Agree with you, i forgot to mention -Wincomplete-uni-patterns (it is in ghcide but no hls, we have to unify all flags for sure 😓)

georgefst · 2021-10-04T11:21:29Z

it is in ghcide but no hls, we have to unify all flags for sure 😓

Yeah, the fact that I know of no good way to do this in a multi-cabal-package project (dhall-to-cabal with flags etc. defined in a single file? probably a totally uncharted path and almost certainly more effort than it's worth right now) is why I suggest keeping it simple.

georgefst · 2021-10-04T12:06:02Z

CI test failure looks unrelated to Fourmolu...

jneira · 2021-10-04T12:07:12Z

CI test failure looks unrelated to Fourmolu...

Yeah, all pr's are affected by #2253 😢

georgefst · 2021-10-04T12:07:40Z

CI test failure looks unrelated to Fourmolu...

Probably due to dependency changes? This PR does bump index-state considerably.

georgefst · 2021-10-04T12:08:06Z

CI test failure looks unrelated to Fourmolu...

Yeah, all pr's are affected by #2253 😢

Ah, ok.

jneira · 2021-10-05T06:37:20Z

CI test failure looks unrelated to Fourmolu...

Yeah, all pr's are affected by #2253 cry

Ah, ok.

I was wrong, not all pr's are affected only those which are updating the cabal.project index state so maybe a new dep version or the cache invalidation is triggering it

Some of these have genuine bugs, as discussed in #2254 (comment). We also get to drop a load of CPP introduced in #2944. Users can still use older versions via CLI mode. This wasn't possible until recently because `stylish-haskell`'s `ghc-lib-parser` bounds were holding us back.

georgefst added 3 commits October 4, 2021 10:13

Allow Fourmolu 0.4

8342138

Reformat record construction to Fourmolu style

bc56d49

Run stylish-haskell on imports

c96314d

georgefst commented Oct 4, 2021

View reviewed changes

jneira added 2 commits October 4, 2021 11:39

Enable fourmolu for ghc-9

ed50c8b

Enable fourmolu tests for ghc-9

cd016ec

jneira approved these changes Oct 4, 2021

View reviewed changes

jneira added 2 commits October 4, 2021 12:07

Enable fourmolu for ghc-9 and stack

c540450

Correct stack-9.0.1.yaml changes

b16a028

jneira mentioned this pull request Oct 4, 2021

Support for GHC-9.0 #297

Closed

35 tasks

georgefst added 2 commits October 4, 2021 11:51

Use explicit import to avoid shadowing

ddfe020

Remove unused import

46d3264

georgefst added 2 commits October 4, 2021 12:02

Bump plugin version

3f45fac

Enable -Wall

7c42612

jneira reviewed Oct 4, 2021

View reviewed changes

jneira added the merge me Label to trigger pull request merge label Oct 4, 2021

jneira mentioned this pull request Oct 5, 2021

hls-eval-plugin test error in ubuntu related with interactive linking #2253

Closed

jneira added 2 commits October 5, 2021 13:20

Merge branch 'master' into fourmolu-0.4

7288e42

Merge branch 'master' into fourmolu-0.4

0d5c3ef

mergify Bot merged commit ee4b496 into haskell:master Oct 5, 2021

georgefst mentioned this pull request Mar 4, 2022

Document corresponding versions for standalone tools (e.g., fourmolu) for binary releases #2649

Closed

georgefst mentioned this pull request Jun 8, 2022

Support fourmolu ^>= 0.7 #2944

Merged

Uh oh!

Uh oh!

Conversation

georgefst commented Oct 4, 2021 • edited by gitpod-io Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georgefst Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

georgefst Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

georgefst Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jneira Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

georgefst Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

jneira commented Oct 4, 2021

Uh oh!

jneira commented Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georgefst commented Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jneira left a comment

Choose a reason for hiding this comment

Uh oh!

jneira commented Oct 4, 2021

Uh oh!

georgefst commented Oct 4, 2021

Uh oh!

jneira commented Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jneira left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

georgefst commented Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jneira commented Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georgefst commented Oct 4, 2021

Uh oh!

georgefst commented Oct 4, 2021

Uh oh!

jneira commented Oct 4, 2021

Uh oh!

georgefst commented Oct 4, 2021

Uh oh!

georgefst commented Oct 4, 2021

Uh oh!

jneira commented Oct 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

georgefst commented Oct 4, 2021 •

edited by gitpod-io Bot

Loading

georgefst Oct 4, 2021 •

edited

Loading

jneira commented Oct 4, 2021 •

edited

Loading

georgefst commented Oct 4, 2021 •

edited

Loading

jneira commented Oct 4, 2021 •

edited

Loading

jneira left a comment •

edited

Loading

georgefst commented Oct 4, 2021 •

edited

Loading

jneira commented Oct 4, 2021 •

edited

Loading