Add formatting plugin for cabal files which uses cabal-fmt by VeryMilkyJoe · Pull Request #2047 · haskell/haskell-language-server

VeryMilkyJoe · 2021-07-28T14:52:23Z

This allows formatting of cabal files with hls.

~~Currently waiting for merges on cabal-fmt and stylish haskell.~~ We gave up on this, also it makes maintenance harder as cabal-fmt supports only one Cabal version at a time.

Closes #183 and part of #2964.

Builds on top of #2945

jneira · 2021-07-28T17:06:04Z

thanks you much for this nice plugin
I think you will have to add the missing deps in the different stack.yaml's to make the stack builds work

plugins/hls-cabal-fmt-plugin/hls-cabal-fmt-plugin.cabal

pepeiborra · 2021-07-29T18:08:14Z

I think that you need to run the test suite explicitly in the test GitHub action:

haskell-language-server/.github/workflows/test.yml

Lines 200 to 206 in c2bd211

    
           - if: ${{ needs.pre_job.outputs.should_skip != 'true' && matrix.test && matrix.ghc != '9.0.1' }} 
        
             name: Test hls-refine-imports-plugin test suite 
        
             run: cabal test hls-refine-imports-plugin --test-options="-j1 --rerun-update" || cabal test hls-refine-imports-plugin --test-options="-j1 --rerun" || LSP_TEST_LOG_COLOR=0 LSP_TEST_LOG_MESSAGES=true LSP_TEST_LOG_STDERR=true cabal test hls-refine-imports-plugin --test-options="-j1 --rerun" 
        
           - if: ${{ needs.pre_job.outputs.should_skip != 'true' && matrix.test}} 
        
             name: Test hls-call-hierarchy-plugin test suite 
        
             run: cabal test hls-call-hierarchy-plugin --test-options="-j1 --rerun-update" || cabal test hls-call-hierarchy-plugin --test-options="-j1 --rerun" || LSP_TEST_LOG_COLOR=0 LSP_TEST_LOG_MESSAGES=true LSP_TEST_LOG_STDERR=true cabal test hls-call-hierarchy-plugin --test-options="-j1 --rerun"

ghcide/src/Development/IDE/LSP/Notifications.hs

plugins/hls-cabal-fmt-plugin/hls-cabal-fmt-plugin.cabal

VeryMilkyJoe · 2021-08-04T12:52:45Z

I had to remove stack support as cabal-fmt requires Cabal 3.4 but it seems impossible to make stack use Cabal 3.4 due to stack's design philosophy.

Ailrun

Could you elaborate the stack issue more? I believe stack works well with extra-deps?

hls-plugin-api/src/Ide/Plugin/Config.hs

hls-test-utils/src/Test/Hls.hs

plugins/hls-cabal-fmt-plugin/hls-cabal-fmt-plugin.cabal

fendor · 2021-08-04T20:53:30Z

Great work! I love the idea to have cabal formatting available in HLS!

VeryMilkyJoe · 2021-08-05T09:59:55Z

Could you elaborate the stack issue more? I believe stack works well with extra-deps?

You might be right, I'm going to look into it some more.

ghcide/src/Development/IDE/LSP/Notifications.hs

ghcide/src/Development/IDE/LSP/HoverDefinition.hs

VeryMilkyJoe · 2021-08-06T10:24:35Z

You now get a bunch of error messages when opening a cabal file since the client requests certain actions for which we have no plugins for cabal files.

[Error - 12:07:11] Request textDocument/codeLens failed.
  Message: No plugin enabled for STextDocumentCodeLens, available: []
  Code: -32600 
[Error - 12:07:51] Request textDocument/codeLens failed.
  Message: No plugin enabled for STextDocumentCodeLens, available: []
  Code: -32600 
[Error - 12:09:07] Request textDocument/codeLens failed.
  Message: No plugin enabled for STextDocumentCodeLens, available: []
  Code: -32600 
[Error - 12:09:08] Request textDocument/codeAction failed.
  Message: No plugin enabled for STextDocumentCodeAction, available: []
  Code: -32600 
[Error - 12:09:08] Request textDocument/hover failed.
  Message: No plugin enabled for STextDocumentHover, available: []

ghcide/src/Development/IDE/LSP/HoverDefinition.hs

jneira · 2021-09-01T08:53:00Z

@VeryMilkyJoe hi! i am afraid that the pr has to be rebased on master

jneira · 2021-09-16T06:02:16Z

@VeryMilkyJoe i did a rebase, i hope it is correct, do you have plans to continue working on this? thanks!

michaelpj · 2022-11-04T16:42:56Z

plugins/hls-cabal-fmt-plugin/src/Ide/Plugin/CabalFmt.hs

+      ++ ["failed with standard error:" <+> pretty stdErrorOut | not (null stdErrorOut)]
+    LogInvalidInvocationInfo -> "Invocation of cabal-fmt with range was called but is not supported."
+
+descriptor :: Recorder (WithPriority Log) -> PluginId -> PluginDescriptor IdeState


Should we provide a configuration option to let users specify the path of cabal-fmt?

Maybe that's overkill, probably cabal-fmt on path is going to be the thing.

I think that's a good addition!

michaelpj · 2022-11-04T17:06:54Z

plugins/hls-cabal-fmt-plugin/src/Ide/Plugin/CabalFmt.hs

+provider :: Recorder (WithPriority Log) -> FormattingHandler IdeState
+provider recorder _ (FormatRange _) _ _ _ = do
+  logWith recorder Info LogInvalidInvocationInfo
+  pure $ Left (ResponseError InvalidRequest "You cannot format a text-range using cabal-fmt." Nothing)


Hmm, do we not have a better way to have a formatting provider that only supports whole buffer formatting? I think some of our others have this limitation too 🤔

I checked all formatter plugins (stylish-haskell, ormolu, fourmolu, brittany, floskell), everyone seems to handle ranges :/

I think maybe we could do something clever with dynamic registration to say we only support it in certain circumstances... but that seems hard. This seems okay I guess. Worth a comment, though.

What kind of comment are you thinking here?

michaelpj · 2022-11-04T17:07:43Z

plugins/hls-cabal-fmt-plugin/src/Ide/Plugin/CabalFmt.hs

+          let fmtDiff = makeDiffTextEdit contents (T.pack out)
+          pure $ Right fmtDiff
+    Nothing -> do
+      pure $ Left (ResponseError InvalidRequest "No installation of cabal-fmt could be found. Please install it into your global environment." Nothing)


@drsooch did we do this?

.github/workflows/test.yml

michaelpj · 2022-11-05T14:33:42Z

I'd be happy to merge this as is and do improvements in follow ups, WDYT @fendor ?

fendor · 2022-11-05T15:06:06Z

2-3 improvements are still missing, @VeryMilkyJoe is very responsive and will incorporate the requested changes until tomorrow.

michaelpj · 2022-11-05T15:16:17Z

There's not any hurry, I just thought you both might like to have it in :)

fendor

LGTM, let's get this going :)

For CI, we want to run the tests with a specific cabal-fmt version, installed automatically by cabal. However, locally, we might want to test with a locally installed cabal-fmt version. This flag allows developers to either let cabal install the build-tool-depends or install a fitting version locally.

obsolete

This comment has been minimized.

Sign in to view

Ailrun reviewed Jul 28, 2021

View reviewed changes

plugins/hls-cabal-fmt-plugin/hls-cabal-fmt-plugin.cabal Show resolved Hide resolved

VeryMilkyJoe force-pushed the cabal-fmt branch from 72e94c3 to 7802d6d Compare July 29, 2021 12:07

VeryMilkyJoe mentioned this pull request Jul 29, 2021

Add tracking of cabal files haskell/vscode-haskell#417

Merged

jneira mentioned this pull request Jul 30, 2021

Activate extension for cabal files only if the used hls version supports it haskell/vscode-haskell#418

Closed

VeryMilkyJoe force-pushed the cabal-fmt branch from 7802d6d to 3c08adc Compare July 31, 2021 09:42

Ailrun reviewed Aug 1, 2021

View reviewed changes

ghcide/src/Development/IDE/LSP/Notifications.hs Outdated Show resolved Hide resolved

plugins/hls-cabal-fmt-plugin/hls-cabal-fmt-plugin.cabal Outdated Show resolved Hide resolved

VeryMilkyJoe force-pushed the cabal-fmt branch 3 times, most recently from e5ed4d8 to 8e06b38 Compare August 3, 2021 16:48

VeryMilkyJoe force-pushed the cabal-fmt branch from 8e06b38 to ef9bc5e Compare August 4, 2021 12:54

Ailrun reviewed Aug 4, 2021

View reviewed changes

hls-plugin-api/src/Ide/Plugin/Config.hs Show resolved Hide resolved

hls-test-utils/src/Test/Hls.hs Show resolved Hide resolved

fendor requested changes Aug 4, 2021

View reviewed changes

plugins/hls-cabal-fmt-plugin/hls-cabal-fmt-plugin.cabal Show resolved Hide resolved

VeryMilkyJoe force-pushed the cabal-fmt branch from ef9bc5e to 1770989 Compare August 6, 2021 07:21

VeryMilkyJoe commented Aug 6, 2021

View reviewed changes

ghcide/src/Development/IDE/LSP/Notifications.hs Outdated Show resolved Hide resolved

VeryMilkyJoe force-pushed the cabal-fmt branch from 4d53eb7 to eebf3b0 Compare August 6, 2021 10:15

VeryMilkyJoe commented Aug 6, 2021

View reviewed changes

ghcide/src/Development/IDE/LSP/HoverDefinition.hs Outdated Show resolved Hide resolved

pepeiborra previously requested changes Aug 6, 2021

View reviewed changes

ghcide/src/Development/IDE/LSP/HoverDefinition.hs Outdated Show resolved Hide resolved

wz1000 mentioned this pull request Aug 15, 2021

Can not go to definition of non local libraries (i.e. dependencies) #708

Open

jneira mentioned this pull request Aug 29, 2021

Migrate from the .cabal format to a widely supported format haskell/cabal#7548

Open

jneira added the pr: needs rebase label Sep 1, 2021

fendor force-pushed the cabal-fmt branch 2 times, most recently from 1c38e53 to 721047e Compare July 9, 2022 08:52

fendor mentioned this pull request Jul 10, 2022

Explicitly use Posix's splitDirectories phadej/cabal-fmt#47

Closed

fendor force-pushed the cabal-fmt branch 3 times, most recently from e25fcf4 to 2ae9f02 Compare October 11, 2022 12:24

fendor force-pushed the cabal-fmt branch 2 times, most recently from 9dbd1bf to 2790cfa Compare November 2, 2022 14:29

fendor self-requested a review November 2, 2022 14:30

fendor force-pushed the cabal-fmt branch 2 times, most recently from cde554f to 631a006 Compare November 4, 2022 11:51

fendor requested review from July541, drsooch and michaelpj November 4, 2022 15:20

michaelpj approved these changes Nov 4, 2022

View reviewed changes

VeryMilkyJoe force-pushed the cabal-fmt branch from 631a006 to d6b6d89 Compare November 9, 2022 18:05

fendor force-pushed the cabal-fmt branch from d6b6d89 to 1d2d7d2 Compare November 10, 2022 13:16

fendor approved these changes Nov 10, 2022

View reviewed changes

Jana Chadt and others added 4 commits November 10, 2022 14:57

Add plugin for formatting cabal files using cabal-fmt

59e502f

Ignore failing cabal-fmt test on windows

97aefa4

Add log message for missing cabal-fmt executable

cd91837

fendor force-pushed the cabal-fmt branch from 1d2d7d2 to cd91837 Compare November 10, 2022 13:57

michaelpj approved these changes Nov 11, 2022

View reviewed changes

michaelpj merged commit 7020532 into haskell:master Nov 11, 2022

openingnow mentioned this pull request Dec 29, 2022

Cannot format .cabal files with HLS 1.9 haskell/vscode-haskell#791

Closed

Uh oh!

Conversation

VeryMilkyJoe commented Jul 28, 2021 • edited by fendor Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

jneira commented Jul 28, 2021

Uh oh!

Uh oh!

pepeiborra commented Jul 29, 2021

Uh oh!

Uh oh!

Uh oh!

VeryMilkyJoe commented Aug 4, 2021

Uh oh!

Ailrun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fendor commented Aug 4, 2021

Uh oh!

VeryMilkyJoe commented Aug 5, 2021

Uh oh!

Uh oh!

Uh oh!

VeryMilkyJoe commented Aug 6, 2021

Uh oh!

Uh oh!

jneira commented Sep 1, 2021

Uh oh!

jneira commented Sep 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michaelpj commented Nov 5, 2022

Uh oh!

fendor commented Nov 5, 2022

Uh oh!

michaelpj commented Nov 5, 2022

Uh oh!

fendor left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

VeryMilkyJoe commented Jul 28, 2021 •

edited by fendor

Loading