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

Improve haddock comments #3207

Merged
merged 20 commits into from
Sep 26, 2022
Merged

Improve haddock comments #3207

merged 20 commits into from
Sep 26, 2022

Conversation

kokobd
Copy link
Collaborator

@kokobd kokobd commented Sep 21, 2022

Changes

  • Try my best to preserve the original indentation of record fields
  • Add haddock comments to data constructors
  • Add me as a code owner to this plugin. I'll keep this plugin updated.

Limitations

  • This PR doesn't include support for ghc-9.2. It will be added in a later one.

Motivation

Previously, when you execute the "Generate fields comments" action on the following record:

data Record = Record
  { a :: Int
  , b :: String
  }

You will get:

data Record = Record
  {
                -- | 
                a :: Int
  ,
                -- | 
                b :: String
  }

I think this is not satisfactory, and we should preserve the original indentation as much as we can.

@kokobd kokobd requested a review from berberman as a code owner September 21, 2022 05:18
@kokobd kokobd changed the title improve haddock comments Improve haddock comments Sep 21, 2022
@kokobd kokobd force-pushed the kokobd/improve-haddock-comments branch from 9a67492 to 2f425ae Compare September 22, 2022 10:28
Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

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

This looks great, the new code is very nicely commented and structured. Thanks!

Copy link
Collaborator

@michaelpj michaelpj 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 and sensible. It seems like we go to some lengths to recognize Haddock - is this something we could ask upstream to tell us? i.e. identify comments as haddock comments when they are?

#endif

-- | Determines the given node has haddock comments attached to it.
hasHaddock :: Data a => Anns -> Located a -> Maybe Bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this just return False in the Nothing case? Or is it useful to be able to distinguish "don't know"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's better to leave the caller to decide what to do when the given Located a is missing in the Anns. For example, the caller use False as the condition to enable the action for adding new haddock comments, but if it's Nothing, we should disable this action.

@michaelpj michaelpj added the merge me Label to trigger pull request merge label Sep 25, 2022
@mergify mergify bot merged commit 217573f into master Sep 26, 2022
@kokobd
Copy link
Collaborator Author

kokobd commented Sep 26, 2022

Looks good and sensible. It seems like we go to some lengths to recognize Haddock - is this something we could ask upstream to tell us? i.e. identify comments as haddock comments when they are?

@michaelpj Oh, I actually didn't expect this to be merged so quickly ;) I'll check that in another PR, when handling ghc 9.2 support. ghc-exactprint surely doesn't take care of this for us, but maybe haddock has some related library functions

@kokobd kokobd deleted the kokobd/improve-haddock-comments branch October 31, 2022 01:19
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.

3 participants