Preserve more trailing comments

[jfly]

Today, we sometimes allow trailing comments, and sometimes move them to the next line. For example:

Input:

foo = { # this should be an SSID
}

Current output:

foo = {
  # this should be an SSID
}

Requested output:

# this should be an SSID
foo = {
}

We discussed this at today's team meeting, and agreed that we'd like to preserve more of these.

See this commit for context: https://github.com/NixOS/nixpkgs/pull/427437/commits/0f3f710c86878c769e3929692aab58f39cfbc09c

Originally reported below:

Maybe there is a strong argument for assuming the comment should usually go above instead of below?

I'd argue:

• A comment at the end of the line is always about that line.
• Comments on separate lines should usually go before the thing that they concern.

There are probably edge-cases where just moving the comment to the next line is the right thing... but my guess is that the majority of cases is better handled by moving them to the previous line instead.

Originally posted by @wolfgangwalther in https://github.com/NixOS/nixpkgs/issues/427460#issuecomment-3105894929

[MattSturgeon]

I don't want to speak on behalf of @piegamesde, but it sounds like moving trailing comments to the line before is technically challenging.

In #324 they've removed comment-moving altogether, which is simpler and should (hopefully) also be an acceptable solution to this issue.

[wolfgangwalther]

In #324 he's removed comment-moving altogether, which is simpler and should (hopefully) also be an acceptable solution to this issue.

tbh, I think the revert is worse than the status quo.

[piegamesde]

tbh, I think the revert is worse than the status quo.

The diff on Nixpkgs is empty, FWIW. Even within the test suite, if you ignore the synthetic tests that no programmer would otherwise write it contains only two genuine instances of that edge case (of which one was deliberately introduced into the test set as a reproducer for this specific issue)

[jfly]

The diff on Nixpkgs is empty, FWIW

While true, this isn't a very meaningful statement, right? It's expected because we've already formatted all of nixpkgs with versions of nixfmt that didn't allow for these kinds of trailing comments.

[wolfgangwalther]

While true, this isn't a very meaningful statement, right? It's expected because we've already formatted all of nixpkgs with versions of nixfmt that didn't allow for these kinds of trailing comments.

Exactly. The nixpkgs diff would have to be run on nixpkgs before nixfmt was introduced at all.

[MattSturgeon]

The nixpkgs diff would have to be run on nixpkgs before nixfmt was introduced at all.

It may be interesting to see that diff.

---

Apparently one of the original reasons we started moving these comments in the first place, is because people used to format comments like:

{
  foo =
  { # Bar is an example thing,
    # that has a comment attached to it...
    bar = null;
  };
}

(This probably happened more often with inline attrsets applied to functions, rather than variable bindings.)

I believe examples like this are why we move the comment down into the attrset, instead of up to the binding:

{
  foo = {
    # Bar is an example thing,
    # that has a comment attached to it...
    bar = null;
  };
}

I'd personally like to avoid encouraging the re-introduction of this formatting style for comments, so I'm hesitant to approve #324.

---

During the initial treewide reformat, I think the current impl was the right decision, as it handles this case well.

Now that most code is well-formatted, I think we could assume that comments like foo = { # some comment relate to foo itself. So I approve of any effort to try moving such comments to before the variable binding instead of into the value.

However unless @piegamesde has a change of heart, I believe this may be too much effort for too little reward; so the formatting team won't take on this work ourselves. I'll bring this up again at the next meeting to confirm (2025-08-12).

[wolfgangwalther]

I'd personally like to avoid encouraging the re-introduction of this formatting style for comments, so I'm hesitant to approve #324.

Agreed.

However unless @piegamesde has a change of heart, I believe this may be too much effort for too little reward; so the formatting team won't take on this work ourselves.

Just to clarify: That's an entirely fine outcome for me, I don't think this is important enough to spend too many cycles on, if it's too complicated to do.