Skip to content

handle trailing comma in import list properly#3035

Merged
mergify[bot] merged 11 commits into
masterfrom
kokobd/fix-import-2958
Jul 16, 2022
Merged

handle trailing comma in import list properly#3035
mergify[bot] merged 11 commits into
masterfrom
kokobd/fix-import-2958

Conversation

@kokobd

@kokobd kokobd commented Jul 11, 2022

Copy link
Copy Markdown
Collaborator

fix #2958

@kokobd
kokobd force-pushed the kokobd/fix-import-2958 branch from 3fc1062 to 3393450 Compare July 12, 2022 03:34
@kokobd
kokobd force-pushed the kokobd/fix-import-2958 branch from 3535077 to 20e73a4 Compare July 12, 2022 04:47
@kokobd

kokobd commented Jul 12, 2022

Copy link
Copy Markdown
Collaborator Author

Strange. I can not reproduce this test failure locally. Both running tests and manually trying this feature give successful results.

extend multi line import with trailing comma:                                                     FAIL (1.67s)
          test/exe/Main.hs:[208](https://github.com/haskell/haskell-language-server/runs/7295318339?check_suite_focus=true#step:8:209)3:
          expected: "module ModuleB where\nimport ModuleA (stuffB, stuffA,\n               )\nmain = print (stuffA, stuffB)\n"
           but got: "module ModuleB where\nimport ModuleA (stuffB, stuffA\n               )\nmain = print (stuffA, stuffB)\n"
          Use -p '/without checkAll.extend multi line import with trailing comma/' to rerun this test only.

@kokobd
kokobd marked this pull request as ready for review July 12, 2022 22:56
@kokobd
kokobd requested a review from pepeiborra as a code owner July 12, 2022 22:56
@kokobd

kokobd commented Jul 12, 2022

Copy link
Copy Markdown
Collaborator Author

I can not reproduce this test failure locally.

Okay, that's because ghc-exactprint behaves differently in GHC 9.0 (not 9.2!) and GHC 8.10.7, where the API is the same. But in GHC 9.0, an extra comma is attached to the "list", besides the one attached to the "list item". According to the docs, that behavior might be wrong.

Anyway, I finally got all compatibility issues handled.

@kokobd
kokobd requested review from georgefst, michaelpj and wz1000 July 13, 2022 03:36
@georgefst

Copy link
Copy Markdown
Collaborator

Works for me on GHC 9.2.3, thanks! (but I won't approve this because I haven't looked at the code)

@kokobd

kokobd commented Jul 13, 2022

Copy link
Copy Markdown
Collaborator Author

Works for me on GHC 9.2.3

@georgefst Thanks for your testing!

@kokobd
kokobd requested review from fendor and guibou July 14, 2022 11:58
kokobd added 2 commits July 14, 2022 13:21
These cache directories are small, but not preserving them requires
HLS to compile all modules in local project on workspace restarts.

@fendor fendor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +383 to +384
let isAnnComma (G AnnComma, _) = True
isAnnComma _ = False

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nitpick: can we align this or remove the superfuous space?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Aha, my mistake. btw, stylish-haskell refuses to run on this file, because there are too many CPP, making it fail.

@kokobd kokobd added the merge me Label to trigger pull request merge label Jul 16, 2022
@mergify
mergify Bot merged commit 2f886bf into master Jul 16, 2022
@kokobd
kokobd deleted the kokobd/fix-import-2958 branch July 16, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge me Label to trigger pull request merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parse error when adding def to trailing-comma import list Adding imports adds an extra comma

3 participants