Keep trailing comment in { and [

[piegamesde]

This is a partial revert of 5bb0639

The change introduced there is now redundant because we catch that specific corner case more precisely in the parser nowadays, so it's reasonable to revert

Fixes #323

CC @wolfgangwalther

[github-actions]

Nixpkgs diff

[MattSturgeon]

Concept SGTM

[MattSturgeon]

A lot of these test cases have two spaces

Suggested change

	{ # switch
	{ # switch

[wolfgangwalther]

I was wondering about that, too - shouldn't nixfmt remove this extra whitespace?

[piegamesde]

It is introduced deliberately here in order to prevent idempotency issues in some very rare special cases. The test suite may not make it look like it but the entirety of Nixpkgs hits that edge case only once. Take a look at the input here:

nativePrefix = { # switch
  i686-solaris = "/usr/gnu";
  x86_64-solaris = "/opt/local/gcc47";
}.${system} or "/usr";

Naively, it would be formatted like this:

nativePrefix =
  { # switch
    i686-solaris = "/usr/gnu";
    x86_64-solaris = "/opt/local/gcc47";
  }
  .${system} or "/usr";

Because the comment and i686-solaris are vertically aligned, the comment will be associated with the i686-solaris and not with the {. This kind of respecting vertical alignment is necessary because too many people write code like:

foo = # comment
      and now the code

and without respecting that vertical alignment the comment would not be associated with its correct code. Anyways, because of this, a second formatting would wrongly associate the switch comment with solaris (which was not the intent of the original code), and thus produce

nativePrefix =
  {
    # switch
    i686-solaris = "/usr/gnu";
    x86_64-solaris = "/opt/local/gcc47";
  }
  .${system} or "/usr";

which is different from the output after one formatting, and thus an idempotency issue. The "fix" is to insert one single space before comments that should not be aligned with the line that follows, so that they will be parsed correctly:

nativePrefix =
  {  # switch
    i686-solaris = "/usr/gnu";
    x86_64-solaris = "/opt/local/gcc47";
  }
  .${system} or "/usr";

[MattSturgeon]

Knowing it is a pre-existing behaviour makes me feel better. Although I'm unsure how I feel about the ugly hack now being more exposed by this PR.

I guess, although it is covered a lot in our test suite, in reality it should be fairly rare to run into comments that need to be explicitly mis-aligned with the following value?

[wolfgangwalther]

Naively, it would be formatted like this:

nativePrefix =
  { # switch
    i686-solaris = "/usr/gnu";
    x86_64-solaris = "/opt/local/gcc47";
  }
  .${system} or "/usr";

Because the comment and i686-solaris are vertically aligned, the comment will be associated with the i686-solaris and not with the {. This kind of respecting vertical alignment is necessary because too many people write code like:

foo = # comment
      and now the code

and without respecting that vertical alignment the comment would not be associated with its correct code.

I argue that in both cases the comment belongs to the whole block and should be formatted like this:

nativePrefix =
  # switch
  {
    i686-solaris = "/usr/gnu";
    x86_64-solaris = "/opt/local/gcc47";
  }
  .${system} or "/usr";

and respectively:

# comment
foo =
  and now the code

Aka: The rule "an end-of-line comment belongs to this line" should have priority over the "aligned comments" rule. Aligned comments should only be "only a comment on this line, nothing else" comments.

[MattSturgeon]

I argue that in both cases the comment belongs to the whole block and should be formatted like this:

Personally, I agree. However if I understand @piegamesde correctly, this may not be realistically achievable without significant work.

Apparently, the code responsible doesn't have any knowledge of the variable binding itself, or what lines surround the comment tokens.

[nixos-discourse]

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

https://discourse.nixos.org/t/formatting-team-meeting-2025-08-05/67630/1