bfad42f · ormolu · git.iko.soy

git.iko.soy/ormolu

Drop the NOTE marks for more uniform comments

[skip ci] Before we sort of had two kinds of comments: with and without NOTEs. This is confusing because there is really no difference between the two. I originally added the words just because they are nicely highlighted in Emacs, which, as I understand now, was not a good idea.

commitbfad42f4e16d64b86b998b4d928b0ae548e07cb8 authormrkkrp <mark.karpov@tweag.io> dateTue 3 Dec 2019, 11:49 (7 years ago) parent0d0c94f

10 files changed+41−42

compare with master

unifiedsplit

src/Ormolu.hs+4−5 src/Ormolu/Diff.hs+1−1 src/Ormolu/Parser.hs+1−1 src/Ormolu/Printer/Comments.hs+2−2 src/Ormolu/Printer/Internal.hs+1−1 src/Ormolu/Printer/Meat/Common.hs+7−7 src/Ormolu/Printer/Meat/Declaration/Class.hs+4−4 src/Ormolu/Printer/Meat/Declaration/Rule.hs+3−3 src/Ormolu/Printer/Meat/Declaration/Value.hs+15−15 src/Ormolu/Printer/Meat/Module.hs+3−3

src/Ormolu.hs+4−5

@@ -55,11 +55,10 @@ ormolu cfg path str = do

5555 traceM "warnings:\n"

5656 traceM (concatMap showWarn warnings)

5757 traceM (prettyPrintParseResult result0)

58− -- NOTE We're forcing 'txt' here because otherwise errors (such as

59− -- messages about not-yet-supported functionality) will be thrown later

60− -- when we try to parse the rendered code back, inside of GHC monad

61− -- wrapper which will lead to error messages presenting the exceptions as

62− -- GHC bugs.

58+ -- We're forcing 'txt' here because otherwise errors (such as messages

59+ -- about not-yet-supported functionality) will be thrown later when we try

60+ -- to parse the rendered code back, inside of GHC monad wrapper which will

61+ -- lead to error messages presenting the exceptions as GHC bugs.

6362 let !txt = printModule result0

6463 when (not (cfgUnsafe cfg) || cfgCheckIdempotency cfg) $ do

6564 let pathRendered = path ++ "<rendered>"

src/Ormolu/Diff.hs+1−1

@@ -55,7 +55,7 @@ matchIgnoringSrcSpans = genericQuery

5555 where

5656 genericQuery :: GenericQ (GenericQ Diff)

5757 genericQuery x y

58− -- NOTE 'ByteString' implements 'Data' instance manually and does not

58+ -- 'ByteString' implements 'Data' instance manually and does not

5959 -- implement 'toConstr', so we have to deal with it in a special way.

6060 | Just x' <- cast x,

6161 Just y' <- cast y =

src/Ormolu/Parser.hs+1−1

@@ -50,7 +50,7 @@ parseModule ::

5050 )

5151parseModule Config {..} path input' = liftIO $ do

5252 let (input, extraComments) = extractCommentsFromLines path input'

53− -- NOTE It's important that 'setDefaultExts' is done before

53+ -- It's important that 'setDefaultExts' is done before

5454 -- 'parsePragmasIntoDynFlags', because otherwise we might enable an

5555 -- extension that was explicitly disabled in the file.

5656 let baseFlags =

src/Ormolu/Printer/Comments.hs+2−2

@@ -81,8 +81,8 @@ spitPrecedingComment (L ref a) mlastSpn = do

8181 withPoppedComment p $ \l comment -> do

8282 dirtyLine <-

8383 case mlastSpn of

84− -- NOTE When the current line is dirty it means that something that

85− -- can have comments attached to it is already on the line. To avoid

84+ -- When the current line is dirty it means that something that can

85+ -- have comments attached to it is already on the line. To avoid

8686 -- problems with idempotence we cannot output the first comment

8787 -- immediately because it'll be attached to the previous element (on

8888 -- the same line) on the next run, so we play safe here and output

src/Ormolu/Printer/Internal.hs+1−1

@@ -234,7 +234,7 @@ spit dirty printingComments txt' = do

234234 scDirtyLine = scDirtyLine sc || dirty,

235235 scRequestedDelimiter = RequestedNothing,

236236 scLastCommentSpan =

237− -- NOTE If there are pending comments, do not reset last comment

237+ -- If there are pending comments, do not reset last comment

238238 -- location.

239239 if printingComments || (not . null . scPendingComments) sc

240240 then scLastCommentSpan sc

src/Ormolu/Printer/Meat/Common.hs+7−7

@@ -76,10 +76,10 @@ p_rdrName l@(L spn _) = located l $ \x -> do

7676 Qual mname occName ->

7777 p_qualName mname occName

7878 Orig _ occName ->

79− -- NOTE This is used when GHC generates code that will be fed

80− -- into the renamer (e.g. from deriving clauses), but where we

81− -- want to say that something comes from given module which is

82− -- not specified in the source code, e.g. @Prelude.map@.

79+ -- This is used when GHC generates code that will be fed into

80+ -- the renamer (e.g. from deriving clauses), but where we want

81+ -- to say that something comes from given module which is not

82+ -- specified in the source code, e.g. @Prelude.map@.

8383 --

8484 -- My current understanding is that the provided module name

8585 -- serves no purpose for us and can be safely ignored.

@@ -99,9 +99,9 @@ doesNotNeedExtraParens :: RdrName -> Bool

9999doesNotNeedExtraParens = \case

100100 Exact name ->

101101 let s = nameStableString name

102− in -- NOTE I'm not sure this "stable string" is stable enough, but it looks

103− -- like this is the most robust way to tell if we're looking at exactly

104− -- this piece of built-in syntax.

102+ in -- I'm not sure this "stable string" is stable enough, but it looks

103+ -- like this is the most robust way to tell if we're looking at

104+ -- exactly this piece of built-in syntax.

105105 ("$ghc-prim$GHC.Tuple$" `isPrefixOf` s)

106106 || ("$ghc-prim$GHC.Types$[]" `isPrefixOf` s)

107107 _ -> False

src/Ormolu/Printer/Meat/Declaration/Class.hs+4−4

@@ -50,10 +50,10 @@ p_classDecl ctx name HsQTvs {..} fixity fdeps csigs cdefs cats catdefs cdocs = d

5050 (p_rdrName name)

5151 (located' p_hsTyVarBndr <$> hsq_explicit)

5252 inci (p_classFundeps fdeps)

53− -- NOTE GHC's AST does not necessarily store each kind of element in

54− -- source location order. This happens because different declarations are

55− -- stored in different lists. Consequently, to get all the declarations in

56− -- proper order, they need to be manually sorted.

53+ -- GHC's AST does not necessarily store each kind of element in source

54+ -- location order. This happens because different declarations are stored

55+ -- in different lists. Consequently, to get all the declarations in proper

56+ -- order, they need to be manually sorted.

5757 let sigs = (getLoc &&& fmap (SigD NoExt)) <$> csigs

5858 vals = (getLoc &&& fmap (ValD NoExt)) <$> toList cdefs

5959 tyFams = (getLoc &&& fmap (TyClD NoExt . FamDecl NoExt)) <$> cats

src/Ormolu/Printer/Meat/Declaration/Rule.hs+3−3

@@ -35,9 +35,9 @@ p_ruleDecl = \case

3535 Just xs -> do

3636 p_forallBndrs p_hsTyVarBndr xs

3737 space

38− -- NOTE It appears that there is no way to tell if there was an empty

39− -- forall in the input or no forall at all. We do not want to add

40− -- redundant foralls, so let's just skip the empty ones.

38+ -- It appears that there is no way to tell if there was an empty forall

39+ -- in the input or no forall at all. We do not want to add redundant

40+ -- foralls, so let's just skip the empty ones.

4141 unless (null ruleBndrs) $

4242 p_forallBndrs p_ruleBndr ruleBndrs

4343 breakpoint

src/Ormolu/Printer/Meat/Declaration/Value.hs+15−15

@@ -99,7 +99,7 @@ p_matchGroup' placer render style MG {..} = do

9999 Case -> id

100100 LambdaCase -> id

101101 _ -> dontUseBraces

102− -- NOTE since we are forcing braces on 'sepSemi' based on 'ob', we have to

102+ -- Since we are forcing braces on 'sepSemi' based on 'ob', we have to

103103 -- restore the brace state inside the sepsemi.

104104 ub <- bool dontUseBraces useBraces <$> canUseBraces

105105 ob $ sepSemi (located' (ub . p_Match)) (unLoc mg_alts)

@@ -168,8 +168,8 @@ p_match' ::

168168 GRHSs GhcPs (Located body) ->

169169 R ()

170170p_match' placer render style isInfix strictness m_pats GRHSs {..} = do

171− -- NOTE Normally, since patterns may be placed in a multi-line layout, it

172− -- is necessary to bump indentation for the pattern group so it's more

171+ -- Normally, since patterns may be placed in a multi-line layout, it is

172+ -- necessary to bump indentation for the pattern group so it's more

173173 -- indented than function name. This in turn means that indentation for

174174 -- the body should also be bumped. Normally this would mean that bodies

175175 -- would start with two indentation steps applied, which is ugly, so we

@@ -391,14 +391,14 @@ p_stmt' placer render = \case

391391 space

392392 sitcc $ located binds p_hsLocalBinds

393393 ParStmt {} ->

394− -- NOTE 'ParStmt' should always be eliminated in 'gatherStmt' already,

395− -- such that it never occurs in 'p_stmt''. Consequently, handling it

396− -- here would be redundant.

394+ -- 'ParStmt' should always be eliminated in 'gatherStmt' already, such

395+ -- that it never occurs in 'p_stmt''. Consequently, handling it here

396+ -- would be redundant.

397397 notImplemented "ParStmt"

398398 TransStmt {..} ->

399− -- NOTE 'TransStmt' only needs to account for render printing itself,

400− -- since pretty printing of relevant statements (e.g., in 'trS_stmts')

401− -- is handled through 'gatherStmt'.

399+ -- 'TransStmt' only needs to account for render printing itself, since

400+ -- pretty printing of relevant statements (e.g., in 'trS_stmts') is

401+ -- handled through 'gatherStmt'.

402402 case (trS_form, trS_by) of

403403 (ThenForm, Nothing) -> do

404404 txt "then"

@@ -458,7 +458,7 @@ p_hsLocalBinds = \case

458458 markInit [] = []

459459 markInit [x] = [(False, x)]

460460 markInit (x : xs) = (True, x) : markInit xs

461− -- NOTE When in a single-line layout, there is a chance that the inner

461+ -- When in a single-line layout, there is a chance that the inner

462462 -- elements will also contain semicolons and they will confuse the

463463 -- parser. so we request braces around every element except the last.

464464 br <- layoutToBraces <$> getLayout

@@ -1016,8 +1016,8 @@ p_hsSplice = \case

10161016 txt "["

10171017 p_rdrName (L srcSpan quoterName)

10181018 txt "|"

1019− -- NOTE QuasiQuoters often rely on precise custom strings. We cannot do

1020− -- any formatting here without potentially breaking someone's code.

1019+ -- QuasiQuoters often rely on precise custom strings. We cannot do any

1020+ -- formatting here without potentially breaking someone's code.

10211021 atom str

10221022 txt "|]"

10231023 HsSpliced {} -> notImplemented "HsSpliced"

@@ -1252,9 +1252,9 @@ p_exprOpTree ::

12521252 R ()

12531253p_exprOpTree _ s (OpNode x) = located x (p_hsExpr' s)

12541254p_exprOpTree isDollarSpecial s (OpBranch x op y) = do

1255− -- NOTE If the beginning of the first argument and the second argument

1256− -- are on the same line, and the second argument has a hanging form, use

1257− -- hanging placement.

1255+ -- If the beginning of the first argument and the second argument are on

1256+ -- the same line, and the second argument has a hanging form, use hanging

1257+ -- placement.

12581258 let placement =

12591259 if isOneLineSpan

12601260 (mkSrcSpan (srcSpanStart (opTreeLoc x)) (srcSpanStart (opTreeLoc y)))

src/Ormolu/Printer/Meat/Module.hs+3−3

@@ -30,9 +30,9 @@ p_hsModule ::

3030 ParsedSource ->

3131 R ()

3232p_hsModule shebangs pragmas (L moduleSpan HsModule {..}) = do

33− -- NOTE If span of exports in multiline, the whole thing is multiline.

34− -- This is especially important because span of module itself always seems

35− -- to have length zero, so it's not reliable for layout selection.

33+ -- If span of exports in multiline, the whole thing is multiline. This is

34+ -- especially important because span of module itself always seems to have

35+ -- length zero, so it's not reliable for layout selection.

3636 let exportSpans = maybe [] (\(L s _) -> [s]) hsmodExports

3737 deprecSpan = maybe [] (\(L s _) -> [s]) hsmodDeprecMessage

3838 spans' = exportSpans ++ deprecSpan ++ [moduleSpan]