/* comments */ should not be reformatted

[toastal]

Description

/* */ style comments should not be reformatted. They have additional use cases such as tree-sitter picking up the the syntax to highlight before scripts inside multiline strings.

Small example input

/* lua */ ''
  print("Hello, world!")
''

Expected output

/* lua */ ''
  print("Hello, world!")
''

(now tree-sitter in my editor highlights this string to Lua which really helps me read/write code in this block now that it’s not just syntax highlighted as a string)

Actual output

# lua
''
  print("Hello, world!")
''

[toastal]

In the wild, you can see this all over Nixpkgs test such as https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/switch-test.nix#L8

[dasJ]

Fwiw, nvim-treesitter now supports that: nvim-treesitter/nvim-treesitter#6418 (comment)

[SuperSandro2000]

They have additional use cases such as tree-sitter picking up the the syntax to highlight before scripts inside multiline strings.

or fenced vim hints.

nvim-treesitter now supports that

That's not released, yet.

[infinisil]

We discussed this in the team meeting today:

• Sounds okay, but a bit ad-hoc, would be good to have a standard for such annotations
  • @tomberek: variations: /*! /*nofmt /*^ /*lang:lua
• @tomberek: Can't upstream treesitter also support the # <lang> style comments?
  • @infinisil: Should be possible!
  • @das_j: Is now possible for at least nvim-treesitter: Nix cleanups nvim-treesitter/nvim-treesitter#6418 (review), but might not be used by downstream projects
  • @infinisil: Seems like it's an easy fix, a similar patch can be used for other treesitter Nix injection definitions
  • @tomberek: Related: https://github.com/nix-community/tree-sitter-nix/blob/b3cda619248e7dd0f216088bd152f59ce0bbe488/queries/injections.scm#L2-L4
• Original motivations:
  • Worked best with the implementation, hard to change
  • Didn't know a use case
  • What to do if there's code after a /* */ single-line comment?
    • We can insert a newline, e.g. from

      /* long comment */ foo + 10
      

      to

      /* long comment */
      foo + 10
      

  • Previously they were often used for vertical alignment

Conclusion: We generally think that it would be fine to preserve /*-style comments more. While we don't intend to fix this ourselves, anybody is free to PR this. If you do, make sure to update this line in the standard.

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2024-04-16/43533/1

[MattSturgeon]

Looks like this is the relevant rendering code:

nixfmt/src/Nixfmt/Pretty.hs

Lines 77 to 91 in c67a7b6

	instance Pretty Trivium where
	pretty EmptyLine = emptyline
	pretty (LineComment c) = comment ("#" <> c) <> hardline
	pretty (BlockComment isDoc c) =
	comment (if isDoc then "/**" else "/*")
	<> hardline
	-- Indent the comment using offset instead of nest
	<> offset 2 (hcat $ map prettyCommentLine c)
	<> comment "*/"
	<> hardline
	where
	prettyCommentLine :: Text -> Doc
	prettyCommentLine l
	| Text.null l = emptyline
	| otherwise = comment l <> hardline

And here is the corresponding parsing code:

nixfmt/src/Nixfmt/Lexer.hs

Lines 83 to 88 in c67a7b6

	-- Try to parse /** before /*, but don't parse /**/ (i.e. the empty comment)
	isDoc <- try ((True <$ char '*') <* notFollowedBy (char '/')) <|> pure False
	chars <- manyTill anySingle $ chunk "*/"
	return $ PTBlockComment isDoc $ dropWhile Text.null $ fixIndent pos' $ removeStars pos' $ splitLines $ pack chars
	where

Maybe there should be an additional isInline parameter passed to BlockComment, true when chars contains no linebreaks?
I guess it should also check if there is additional content after the end of the comment, but on the same line?
When isInline is true, the rendering code would avoid inserting newlines unless otherwise necessary due to line-length.

Having never touched haskell, I doubt I'm up to the task. Hopefully pointing to the relevant code will inspire someone more knowledgeable to submit a PR.

[piegamesde]

The issue is less with the parsing, that's a bit fiddly but no more than a chore. The big open question is, what to do in the renderer with such comments when they are followed by significant amounts of code which overflow the line length limit.

The rendering engine currently ignores all line length calculations involving comments, however this strongly depends on the assumption that comment tokens are always strictly followed by a line break.

[MattSturgeon]

Thanks for clarifying!

The big open question is, what to do in the renderer with such comments when they are followed by significant amounts of code which overflow the line length limit.

Ideally, inserting a line break before the comment would be enough to not overflow, but that's the best case scenario...

I think it's fair to say, that if someone has written an inline block comment on the same line as a long string (for example), it was intentional. Maybe in that scenario it is just accepted that the line will overflow?
Alternatively, perhaps a '' indented raw string could be used, with the string content starting after a "''\n"" sequence? Though this seems overly complex at first glance.

If the long content to the right of the inline block comment is not a string, then I think it is ok to reformat that as usual, such that it is wrapped over multiple lines.

In summary, yes - you're right; this is more complex that it first appears!

The rendering engine currently ignores all line length calculations involving comments, however this strongly depends on the assumption that comment tokens are always strictly followed by a line break.

That sounds like a bigger issue. Perhaps "inline" comments can be a distinct type from other comments and therefore included in line length calculations?

[piegamesde]

So one thing that one could do would be to special case these comments in the parser but only if they are followed by '' and then a line break. I think that should cover all reasonable use cases without breaking everything else.

[MattSturgeon]

So one thing that one could do would be to special case these comments in the parser but only if they are followed by '' and then a line break. I think that should cover all reasonable use cases without breaking everything else.

While it'd be nice to have support for normal strings too, I agree that is a reasonable approach that'd work for most usage.

I do sometimes do things like this in my config.

{
  # This snippet is so short it feels strange to use an indented string:
  someLuaOption = /* lua */ "function() print('hello, world!') end";
}

But some support is better than no support, and it can always be iterated on in the future.

[ian-h-chamberlain]

Just to propose one additional use case for /* */ comment (non-)formatting: it's possible to toggle a block comment with a single # using something like the following:

{
  /* <-- Toggling a # line comment at the beginning of this line toggles the whole block
    services.foo = {
      enable = true;
      bar = "baz";
    };
  # */
}

For this kind of case I think it would be valuable to avoid splitting the final # */ onto two lines (and maybe also the first line, but that's less important). Otherwise this "quick toggle" results in invalid syntax, e.g.

{
  # /*
    <-- Toggling a # line comment at the beginning of this line toggles the whole block
      services.foo = {
        enable = true;
        bar = "baz";
      };
    #
  */
}

I use this in my configs to quickly toggle on/off large sections of the config. I'm not sure how popular a practice it is in general, but it seems to be moderately common from a cursory search:
https://github.com/search?q=lang%3Anix+%2F%28%23+%5C*%5C%2F%7C%23+%5C%2F%5C*%29%2F&type=code

[MattSturgeon]

Just to propose one additional use case for /* */ comment (non-)formatting

I don't think this issue is proposing that block comments not be formatted; instead this issue is about allowing short and/or inline /* */ comments.

Given nixfmt is deterministic, it'd probably have to be based on how long the comment content is and whether or not there is trailing code after the comment?

Your issue about block comments being formatted messing up your workflow should probably be tracked in a separate ticket and may be best implemented as a configurable cli flag.

[piegamesde]

@ian-h-chamberlain commenting out code blocks with /* has the advantage that it is low on diff noise, however it has the big disadvantage that it breaks as soon as that block contains another nested /* comment.

[ian-h-chamberlain]

I opened #225 to track separately, maybe it would also make sense to update the title of this issue to be more specifically about short/inline /* */ comments just to clarify?
Any more discussion about whether / how to handle this case can continue on that issue, I suppose.

[infinisil]

Btw here's the code that handles the conversion from /* foo */ to # foo:

nixfmt/src/Nixfmt/Lexer.hs

Lines 131 to 169 in e819b2d

	convertTrailing :: [ParseTrivium] -> Maybe TrailingComment
	convertTrailing = toMaybe . join . map toText
	where
	toText (PTLineComment c _) = strip c
	toText (PTBlockComment False [c]) = strip c
	toText _ = ""
	join = Text.unwords . filter (/= "")
	toMaybe "" = Nothing
	toMaybe c = Just $ TrailingComment c
	convertLeading :: [ParseTrivium] -> Trivia
	convertLeading =
	concatMap
	( \case
	PTNewlines 1 -> []
	PTNewlines _ -> [EmptyLine]
	PTLineComment c _ -> [LineComment c]
	PTBlockComment _ [] -> []
	PTBlockComment False [c] -> [LineComment $ " " <> strip c]
	PTBlockComment isDoc cs -> [BlockComment isDoc cs]
	)
	isTrailing :: ParseTrivium -> Bool
	isTrailing (PTLineComment _ _) = True
	isTrailing (PTBlockComment False []) = True
	isTrailing (PTBlockComment False [_]) = True
	isTrailing _ = False
	convertTrivia :: [ParseTrivium] -> Pos -> (Maybe TrailingComment, Trivia)
	convertTrivia pts nextCol =
	let (trailing, leading) = span isTrailing pts
	in case (trailing, leading) of
	-- Special case: if the trailing comment visually forms a block with the start of the following line,
	-- then treat it like part of those comments instead of a distinct trailing comment.
	-- This happens especially often after `{` or `[` tokens, where the comment of the first item
	-- starts on the same line ase the opening token.
	([PTLineComment _ pos], (PTNewlines 1) : (PTLineComment _ pos') : _) | pos == pos' -> (Nothing, convertLeading pts)
	([PTLineComment _ pos], [PTNewlines 1]) | pos == nextCol -> (Nothing, convertLeading pts)
	_ -> (convertTrailing trailing, convertLeading leading)

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/enforcing-nix-formatting-in-nixpkgs/49506/8

[infinisil]

For reference, we had a discussion about this on Matrix

[omkumar312]

I want to work on this issues please assign me this issue.

[SuperSandro2000]

Btw here's the code that handles the conversion from /* foo */ to # foo:

Does that mean we just need to drop that code and we are good to go?