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.

Sign up for GitHub

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

Jump to bottom

Remove empty haddocks and remove empty comments. #1459

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

Remove empty haddocks and remove empty comments. #1459

wants to merge 19 commits into from
+768 −20

Conversation

philderbeast
Copy link
Contributor

Can we suggest removing empty comments and removing empty haddocks? I've seen examples of these littering code, like the following:

-- |
foo = 1

I had a crack at doing this.

  • If I run cabal run hlint -- --refactor tests/empty-comment-test.hs the comments are removed but single line comments are replaced by an empty line. I'd really like to remove the whole line. Is there a way to do that? I tried advanceSrcLoc (realSrcSpanEnd realSpan) '\n' for the refactor span but this didn't work as I'd hoped.
  • I'm not quite sure what is the best way to convert tests/empty-comment-test.hs into tests.
$ cabal run hlint -- --refactor --refactor-options=inplace tests/empty-comment-test.hs
$ git diff

diff --git a/tests/empty-comment-test.hs b/tests/empty-comment-test.hs
index 33b38157..84811b24 100644
--- a/tests/empty-comment-test.hs
+++ b/tests/empty-comment-test.hs
@@ -1,11 +1,11 @@
--- |
+
 haddockAboveSingle = 1
 
 haddockBelowSingle = 1
--- ^
 
--- |
---
+
+
+
 haddockAboveSingles = 1
 
 -- |
@@ -24,51 +24,45 @@ haddockBelowIntoNonEmpty = 1
 --
 -- foo
 
-{--}
+
 commentMultiEmpty = 1
 
-{- -}
+
 commentMultiEmptySpace = 1
 
-{-
 
-    -}
 commentMultiEmptyLines = 1
 
-{- |-}
+
 haddockAboveMultiEmpty = 1
 
-{- | -}
+
 haddockAboveMultiEmptySpace = 1
 
-{- |
 
-    -}
 haddockAboveMultiEmptyLines = 1
 
 haddockBelowMultiEmpty = 1
-{- ^-}
+
 
 haddockBelowMultiEmptySpace = 1
-{- ^-}
+
 
 haddockBelowMultiEmptyLines = 1
-{- ^
 
-    -}
 
 data HaddockSingle = HaddockSingle
   { haddockFieldEmptyBelow :: Int
-  -- ^
-  -- |
+  
+  
   , haddockFieldAboveEmpty :: Int
-  , haddockFieldEmptyAfter :: Int -- ^
+  , haddockFieldEmptyAfter :: Int 
   }
 
 data HaddockMulti = HaddockMulti
   { haddockFieldEmptyBelow :: Int
-  {- ^-}
-  {- |-}
+  
+  
   , haddockFieldAboveEmpty :: Int
-  , haddockFieldEmptyAfter :: Int {- ^-}
-  }
\ No newline at end of file
+  , haddockFieldEmptyAfter :: Int 
+  }

@philderbeast philderbeast changed the title Remove/empty haddock empty comment Remove empty haddocks and remove empty comments. Feb 14, 2023
@ndmitchell
Copy link
Owner

No idea on the refactor thing - @zliu41 is the expert there.

Can you turn your test script into something that runs via the .test files in the tests directory?

@philderbeast philderbeast force-pushed the remove/empty-haddock-empty-comment branch 2 times, most recently from 330fa93 to a6dcef7 Compare July 26, 2024 18:42
@philderbeast
Copy link
Contributor Author

Can you turn your test script into something that runs via the .test files in the tests directory?

Thanks for the suggestion @ndmitchell, I've done that now.

@philderbeast philderbeast force-pushed the remove/empty-haddock-empty-comment branch from 6c6e49e to 0026c5a Compare July 26, 2024 19:53
@philderbeast philderbeast mentioned this pull request Jul 27, 2024
Merged
@philderbeast philderbeast marked this pull request as draft July 27, 2024 13:04
@philderbeast
Detect empty comments in isolation.
8adccf8 
- Only those not followed by something.
- Add haddocks.
- Use GHC's isPointRealSpan
- Satisfy -Wprepositive-qualified-module
- Use isOneLineSpan
@philderbeast
Add empty comment test file
a6f3b09 
- Convert .hs into a .test
- Split empty line comment tests
- Add test for spaced single line comments
@philderbeast
Empty line tests; leading, trailing & double
25ce756
@philderbeast
Add checks for single line >>> doctest comment
ab2a9f0
@philderbeast
Update test expectations
5d59c9e
@philderbeast
Use helper in construction of Idea
dd54168 
- Rename comment to idea
- Add EmptyComment type
@philderbeast
Add isCommentPragma
44fe67c
@philderbeast
Add isCommentHaddock
e0eb345
@philderbeast
Very much WIP
89f527e
@philderbeast
Add TODO comment
17352af
@philderbeast
Need to sort comments by line
c863c27
@philderbeast
Add isHaddockLeader
fdd100e
@philderbeast
Add classify comments
65a87b1 
- Partition pragmas
- Partition haddocks
@philderbeast philderbeast force-pushed the remove/empty-haddock-empty-comment branch from 7c419c5 to cf7d1ee Compare August 3, 2024 14:36
@philderbeast philderbeast force-pushed the remove/empty-haddock-empty-comment branch from cf7d1ee to d5ffa47 Compare August 4, 2024 16:20
@ndmitchell
Copy link
Owner

I note this is still in draft - is there something else you are hoping to do here before I take a look?

@philderbeast
Copy link
Contributor Author

Thanks @ndmitchell but this is not ready for review (and apologies for reworking it on this PR branch and not on another branch).

I had an initial implementation that detected simple empty comments, like these;

  • the empty multi-line haddock comment.
  • the empty single line haddock comment or run of comments.

As I did more testing I found more types of empty comment that I'd like to include, doctest comments in particular;

  • the empty doctest leader >>>
  • the empty multi-line doctest :{ .. :}

If an otherwise empty haddock comment includes a empty doctest comment then which new HLint suggestion should take precedence or should I report both?

-- |
-- >>>
--

I'm also considering reporting leading or trailing blank lines in haddocks. For instance, in the Agda source there are many trailing blank lines in its haddocks.

-- | A symbolic library name.
--
type LibName = String

Should there be separate HLint suggestions for trimming leading or trailing blank lines? Should the same go for trimming a run of blank lines down to single blank line in otherwise non-empty comments?

I'm in the process of redoing the implementation to first partition comment types and then do the detection as before.

In the earlier implementation I processed single line comments one at a time but I think I could provide more nuanced empty comment detection if I treated a run of comments (with a haddock leader) like a multi-line haddock comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndmitchell ndmitchell ndmitchell left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@philderbeast @ndmitchell