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

Inline comment after last data constructor disapearing fixed #427

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Lev135
Copy link
Contributor

@Lev135 Lev135 commented Nov 6, 2022

Resolves #425. I don't think this solution to be nice enough. It works however, as far as I can see (all tests have been passed). I solved this randomly by tracing the structure and selecting functions from GHC lib, so at least, my change should be checked by someone more experienced. These are two lines, though

Here is some (maybe useful) information about the bug in my comment to issue:
#425 (comment)

@Lev135 Lev135 changed the title Inline comment after last record field Inline comment after last data constructor disapearing fixed Nov 9, 2022
@Lev135
Copy link
Contributor Author

Lev135 commented Jan 4, 2023

There is another way to solve this problem with haddock: to add an Opt_Haddock at the parser stage: in this case all haddock comments will be attached to the correct places and also we'll have an option to reformat comment (if we want). This seems nice for me. However, there are some problems at this way: I've tried to enable this option in my fork and many test cases from Data and Import step failed. For Data I fixed them, but for Import there are some problems. However, nothing seems to be impossible and I can try to resolve all isues here, if we'll chose this road.

Also, if we'll chose the "right" way for haddock processing, issue for common comments still remains. So we'll have to provide two different ways for dealing with comments: one for haddock and another for other comments. It doesn't seem to be pretty enough. Nevertheless, work with haddock should be more easy this way

@Lev135
Copy link
Contributor Author

Lev135 commented Jun 29, 2023

I found a much worse issue in this PR, than the original one. If we have a prefix comment after the data declaration like this:

data A = A

-- | foo
foo = undefined

Now stylish attaches it to the data:

data A = A
  -- | foo

-- | foo
foo = undefined

That's indeed awful. However, I don't see any good way for working around it without enabling Opt_Haddock flag. The reason is that in current parser's behavior -- | foo comment appears at the same place, where post-comment to the data constructor is. Of course, we could filter only comments starting with -- ^, but this doesn't seems to be a good and clean idea.

That's why I think we should use Opt_Haddock flag and to work haddock as ghc expects us to work with them. This will require some work, though. I'll try to carry it out. It would be helpful to know, why this hasn't been done already? Were there any problems in this way?

Anyway, it would be great to take your opinion about this problem, @jaspervdj. If my description is not clear enough, I can try to clarify it

@Lev135 Lev135 marked this pull request as draft June 29, 2023 22:30
@jaspervdj
Copy link
Member

Yes, I'm happy to adopt Opt_Haddock -- I suspect this option wasn't around when the majority of the comment-handling code was written...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inline comment to the last constructor disapears
2 participants