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

Doctest comment parsing using module annotations in Eval Plugin #1232

Merged
merged 60 commits into from
Jan 31, 2021
Merged

Doctest comment parsing using module annotations in Eval Plugin #1232

merged 60 commits into from
Jan 31, 2021

Conversation

konn
Copy link
Collaborator

@konn konn commented Jan 19, 2021

This PR aims at rewriting existing custom comment parser in Eval plugin with getParsedModuleWithComments, which is introduced recently.
This should address #1214, #1231 and #1258.

closes #1214, closes #1258

@konn konn changed the title WIP: Doctest comment parsing using module annotations in Eval Plugin Draft: Doctest comment parsing using module annotations in Eval Plugin Jan 19, 2021
@konn konn marked this pull request as draft January 19, 2021 14:08
@konn
Copy link
Collaborator Author

konn commented Jan 21, 2021

I think it's done for line comments (not blocks yet), and it seems that the infinite loop occurring when one toggles block comments, as described in #1214, seems gone with the current change.

@konn
Copy link
Collaborator Author

konn commented Jan 22, 2021

Hmm, also plugins/hls-eval-plugin/README.rm states that Multiline expressions are supported, there seem no implementation and test cases for it. I decided just to ignore this feature in this PR.

@konn
Copy link
Collaborator Author

konn commented Jan 22, 2021

Fmm, I assume that Opt_Haddock is disabled in getParsedModuleWithComments, but it seems it is.
Then, we have to decide the style (block or line) of comments corresponding to AnnDocCommentNext, AnnDocCommentPrev and AnnDocCommentNamed.
I couldn't find the information to decide their style, does anyone has knowledge of them, or we have to decide it by just scanning the corresponding initial segments of range from editor buffer and see whether it start with -- or not?

@jneira
Copy link
Member

jneira commented Jan 22, 2021

and what about disable haddock for getParsedModuleWithComents?

@konn
Copy link
Collaborator Author

konn commented Jan 22, 2021

and what about disable haddock for getParsedModuleWithComents?

Yes, it must be the easiest way to try!
I'm not sure if other plugins using getParsedModuleWithComments (e.g. HLint) work as previous. Let me try disabling Opt_Haddock and see how CI result changes.

@jneira
Copy link
Member

jneira commented Jan 22, 2021

In fact when i wrote the rule i was about to disable haddock, but as hlint worked without doing it i decided to let is as is now

@konn
Copy link
Collaborator Author

konn commented Jan 24, 2021

To comply with the current Eval Plugin behaviour, I decided to limit the indentation level to be processed as Eval command so that:

  • In ordinary .hs: indent level must be 0
  • Literate Haskell: Indent Level must be 1

This doesn't cover modules with the base indentation level strictly greater than those specified above, though it might rarely occur.
Since the objective of this PR is just to replace the existing comment parser, such improvements deserve another PR.

@jneira jneira linked an issue Jan 30, 2021 that may be closed by this pull request
goldenTest mdl
#if __GLASGOW_HASKELL__ >= 808
, testCase "CPP support" $ goldenTest "TCPP.hs"
,
#if mingw32_HOST_OS
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Anyway i will try to take a look in my local windows env

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

Looks good to me, and tests are the evidence it improves the actual situation.
I would like to have some other approve before merge though.

Copy link
Member

@Ailrun Ailrun left a comment

Choose a reason for hiding this comment

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

I think it's in a good shape.

@konn konn added the merge me Label to trigger pull request merge label Jan 31, 2021
@mergify mergify bot merged commit 6b6c405 into haskell:master Jan 31, 2021
@konn konn deleted the eval-comments-from-parsed-module branch January 31, 2021 07:52
@konn konn mentioned this pull request Feb 2, 2021
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.

Token parse error on {-# UNPACK #-} HLS version 0.8.0 frequently stuck when saving files
3 participants