-
-
Notifications
You must be signed in to change notification settings - Fork 389
Extract fourmolu plugin into a standalone package #1823
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
Conversation
4431b07
to
bd689a8
Compare
2787573
to
5576997
Compare
@gustavoavena nothing should be moved and |
One thing would be I'm not very familiar with the code, but maybe refactoring the |
The purpose of Moreover, now there are some test suites in HLS referencing plugins' test data, e.g. haskell-language-server/test/functional/Progress.hs Lines 32 to 53 in bb99905
Which is a bit hacky but works. Unfortunately, I don't have a good idea so far. @gustavoavena Could you try changing the path |
075e3c5
to
580de2a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
580de2a
to
64e3c7f
Compare
64e3c7f
to
4fb92e1
Compare
4fb92e1
to
3ee15cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
Nobody likes `PackageImports`. And it's unnecessary since haskell#1823.
What
Extract fourmolu plugin to a standalone package.
Similar to what was done for stylish-haskell (#1604) and brittany (#1422) plugins.
RFC: Test case in
Progress.hs
TL;DR: is it ok to leave the progress tests case from fourmolu plugin where it is?
I'm basing this PR on the ones that did the same for stylish-haskell (#1604) and brittany (#1422). I moved the functional tests from the
Format
test suite the same way, but unlike stylish-haskell and brittany, fourmolu has a test case inwindow/workDoneProgress
test suite (intest/functional/Progress.hs
).My initial plan was to extract it to the fourmolu test suite, but it depends on some things from utility modules that are not visible to the new fourmolu test suite (e.g.
hlsCommand
fromTest.Hls.Command
).It seems that to extract it, I would need to move some things around to avoid adding a dependency from the new test suite to the functional test suite. Since I wanted to avoid making bigger changes on my first PR, I wanted to double check if extracting that one test case is necessary, if it's passing as is.