Nix formatting team: Treewide Nixpkgs formatting

Announcements
1

Hi everyone,

After preparing for it throughout the year[1], we as the Nix formatting team are proud to announce the soon to be merged (almost) full reformat of nixpkgs with the following primary PR:

This PR will format all files without recent[2] activity to minimise merge conflicts on active PRs. Once files are formatted, CI will enforce them to stay formatted going forward.

We are coordinating asynchronously on Matrix and will hold a final review and merge party :tada: at Invalid date in Jitsi, feel free to join both!

Once merged, we will monitor for potential issues before going ahead with formatting the remaining files in the new year.

If you have any questions, concerns or problems with the reformatting process, let us know here as a reply or open a nixfmt issue or a Nixpkgs issue while pinging @NixOS/nix-formatting. :slight_smile:

If you are interested in following the development of the tooling, feel free to join the Matrix room and the meetings, which are currently happening every second Tuesday with the next meeting at 2024-12-10T19:00:00Z. The nixfmt repository Readme is also worth mentioning, it contains a lot of instructions for configuring tools and editors (additions welcome!). We also appreciate more help with development.

Note that nixfmt-rfc-style is a not a Nixpkgs specific tool, youā€™re welcome to use it on your own projects.

Finally we would like to thank everybody involved in making this happen, starting from people involved in RFC 101 and the other formatter efforts such as alejandra, nixpkgs-fmt and the classic nixfmt, to everyone giving us feedback in the accepted RFC 166.

  1. Preparation work such as fixing all the critical formatter issues that were brought up, and soon updating the formatter on both master and the release branches and update CI tooling to use the new version. ā†©ļøŽ

  2. Specifically, files that are touched by a PR with activity in the last ~2 months. ā†©ļøŽ

70 Likes
2

The problem described in this issue Appending an optional string / array causes the entire expression to be reformatted Ā· Issue #228 Ā· NixOS/nixfmt Ā· GitHub got already reported multiple times by different people over the last months and keeps coming up as a major bug that should be fixed before this.

4 Likes
3

Thatā€™s not a bug, thatā€™s intended behaviour in accordance with the RFC.

Itā€™s a trade-off between never absorbing anything and diff noise when absorbability changes. Diff noise is the lesser evil here; you do not want non-absorbed expressions everywhere, thatā€™s insanely ugly and wasteful w.r.t. vertical space.

The diff noise isnā€™t that bad either as itā€™s pretty easy to tell what happened and easy to verify nothing else changed at a glance.

To prevent it in situation where you think itā€™s likely that itā€™ll happen, I think weā€™ll see patterns like

foo = 
  [ ]
  ++ [ ... ];

or

foo = 
  #
  [ ... ];

emerge. I donā€™t like this either but, again, this is by far the lesser evil.

5 Likes
4

You can also hide whitespace changes in the github web interface when reviewing; I find indentation changes to be a non-issue.

1 Like
5

Correct me if Iā€™m wrong but in the last meeting we (the team) decided that while this is an issue that has not been addressed, itā€™s not blocking for an initial reformat and can be fixed at a later point.

2 Likes
6

I thought reducing diff noise was a goal and fixing it down the line will cause a lot of noise commits again.

1 Like
7

I may not have been clear enough in my previous reply but this is a trade-off; a compromise between two suboptimal options.

You get to choose between this:

{
  foo =
    [ value ];
  bar = 
    { attr = value; };
  empty = 
    [ ];
  moreData =
    {
      foo = bar;
      baz = quux;
    };
}

and this

{
  foo = [ value ];
  bar = {
    attr = value;
  };
  empty = [ ];
  moreData = {
    foo = bar;
    baz = quux;
  };
}

where the latter has the diff noise issue and the former doesnā€™t.

8

Could I ask why one causes more noise than the other?

Just as I never knew that layout/formatting could increase or decrease ā€œnoiseā€.

Also, given both are sub-optimal options, what would an optimal solution look like?

9

See the issue linked in Nix formatting team: Treewide Nixpkgs formatting - #2 by Sandro.

Of course it can. If the formatter reformats lines that are not part of the actual diff, that adds noise to the diff; itā€™s no longer just the lines that changed but other lines were ā€œchangedā€ that didnā€™t actually meaningfully change and only had to be changed to fit the formatting rules.

There is none.

The best we could theoretically do would be to allow the user to specify whether they want the binding to be absorbed/compact or not so that we as humans can pre-emptively prevent diff noise where we think it is likely to happen.

That is imperfect and has its own additional issues though and I think the workaround patterns I described are preferable, particularly the no-op append.

2 Likes
10

The reason it didnā€™t initially occur to me, was as Iā€™ve never actually used git for anything except storing my nix config.

So as a result, Iā€™ve never touched a formatting tool before - however, I can now see the pros and cons of using one!

Thanks so much for taking the time to explain :pray:

2 Likes