nixfmt icon indicating copy to clipboard operation
nixfmt copied to clipboard

[WIP] Sort assignments in sets and "let"

Open KAction opened this issue 4 years ago • 1 comments

Sort alphabetically identifier keys in sets and let abstraction. More complex ways to specify keys, like "inherit" statement or interpolated sorts after plain identifier keys, but without any meaningful order between themself.

With this change expression "{ b = 10; a = 15; }" transformed into "{ a = 15; b = 10; }", as indented. Things are getting trickier with comments.

Nixfmt preserves comments by attaching them to tokens before comment. It may result to unexpected behaviour in following quite natural case:

{
  b = 12; # talk about "b"

  # This is comment about "a"
  a = 10;
}

This expression gets transformed into following, since from Nixfmt point of view both comments are attached to "b = 12;" assignment (semicolon, to be more precise).

{
  a = 10;
  b = 12; # talk about "b"

  # This is comment about "a"
}

DISCUSSION: What should be done about this unexpected behaviour with comments?

First option is just make this feature opt-in. Second one, probably harder, it to use some heuristics on whether comment should be attached to token before or after comment. This is the most complex case I managed to invent:

FunctionFoo { # comment on set. Dont't move!
  # comment on b, move with b.
  b = 42; # this is also comment on b.

  # comment on a.
  a = 14;

  c = [ # comment on array
    1
    2
    3
  ]; # comment on c;
}

Seems that rule "comment that takes whole line is attached to token after it, otherwise it is attached to token before it" is good approximation of human perception.

Dear maintainer, what do you suggest and implementation of what approach you are willing to accept?

KAction avatar Feb 19 '20 00:02 KAction

Thank you for the suggestion and for helping out! I have the following thoughts:

First of all, sorting can never be default in a formatter because it is not always what people want. There are two acceptable ways to turn it on, either with a flag that turns it on for everything or with a comment like # nixfmt: sort attached to the specific structure one wants to sort. This shouldn't be much trouble to implement though.

Second, you've correctly identified the issue with comments. As you said, ideally a trailing comment is attached to the token before it on the same line and all comments on their own line are attached to the token that follows them. I'd like to this while parsing already but I couldn't make that work. I forgot the details, but I think it affected how parsers failed and therefor whether the next alternative would be tried or not, so it would accept a different language.

Since I couldn't figure out how to solve it during parsing, a transformation pass that moves all the comments to the correct token would be good. Either way, the formatting of many things needs to be fixed after that. To complicate things, the formatter sometimes moves comments to different tokens, e.g. past a comma. Those moves should also become part of the same transformation. Issue #32 tracks this, but I haven't spent time on it yet. I'll see if I can make some time for it soon.

In short, the bulk of the hard work is in fixing #32, after that, this shouldn't be a problem to implement.

Lucus16 avatar Feb 19 '20 07:02 Lucus16

(discussed in the formatting team meeting today)

While we think sorting attribute sets could be a good opt-in feature, we'd first need to figure out an opt-in mechanism. Let's close this PR for now since it's a bit outdated.

infinisil avatar Apr 02 '24 18:04 infinisil

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

https://discourse.nixos.org/t/formatting-team-meeting-2024-04-02/42658/1

nixos-discourse avatar Apr 02 '24 19:04 nixos-discourse