Nixpkgs-fmt beta (0.3.1)

Announcements
1

In What are your goals for 19.09? I said that my plan was to have a release of nix-fmt 1.0 for the next release. We are getting there!

The project has since been renamed nixpkgs-fmt due to naming collisions. More importantly, nixpkgs-fmt is usable today!

I encourage you all to try it out, use it and report formatting issues back to the project so that we can make the best nix code formatter out there.

Installation

Why another code formatter?

If you are confused why we need another nix code formatter, first, it’s very understandable :slight_smile:

The TL;DR is that nixpkgs-fmt is taking a different approach than existing formatters and is specialized to format the nix code in nixpkgs, favourizing mergeability. But it’s great to format any nix code obviously. For a longer read see: GitHub - nix-community/nixpkgs-fmt: Nix code formatter for nixpkgs [maintainer=@zimbatm]

How to contribute

The easiest is to go to the online formatter and submit code samples when you see something not formatted right. Or if you like to write Rust code your contribution is also welcome.

What’s next?

The goal right now is to have as many people as possible test it an be exposed to the format. The hope is to reach consensus and have a majority of happy users. nixpkgs-fmt’s output is quite close to the format used in nixpkgs so hopefully it won’t be too controversial. Reach 1.0 before 19.09.

After the 19.09 release I am hoping to add hooks to nixpkgs to start formatting the code. We need to discuss how to do that but I think this can be quite gradual.

Another thing I would like to explore is how to create rules that can be language-agnostic by emitting and parsing back a JSON representation of the AST tree. This would make it easy for example to rewrite all the URLs to strings with a simple python/ruby/bash script.

Thanks

Thanks a lot to @matklad who implemented most of nixpkgs-fmt. He also invented the rowan lossless parsing library upon which this project is based.
Thanks also to @jD91mZM2 who wrote the rnix library and was responsive to our pull-requests.

15 Likes
2

Looks neat!

I just tried running some code through it, and I don’t like this particular output decision, but I’m also not sure just what the relevant rules should be either.

Input:

      meta = (oldAttrs.meta or {}) // {
        platforms = pkgs.rustc.meta.platforms;
      };

Output:

        meta = (oldAttrs.meta or {})
          // {
               platforms = pkgs.rustc.meta.platforms;
             }
          ;

In this particular case I think my original formatting is best, because it’s structurally similar to just

meta = {
  platforms = pkgs.rustc.meta.platforms;
};

but overlaid on the existing attrset. Given a rule of “operator should be on the new line” I wouldn’t be opposed to an output of

meta = (oldAttrs.meta or {})
  // { platforms = pkgs.rustc.meta.pltforms; };

given that the new attrset is a single field with a relatively short value, though I don’t think this is a good as the input.

Edit: I just noticed this decision too, which strikes me as non-idiomatic based on nixpkgs code I’ve seen.

Input:

    rustc = rustChannel.rust.overrideAttrs (oldAttrs: {
      …
    });

Output:

    rustc = rustChannel.rust.overrideAttrs (
      oldAttrs: {
        …
      }
    );

I find this harder to read, it actually obscures the fact that the parens are to wrap a function.

1 Like
3

Thanks for the feedback! Until we hit 1.0 any parts of the format is open for discussion and change.

The best thing to do is to submit those code samples to the issue tracker so that we can discuss them individually if you don’t mind. And also keep a centralized history of the decision making.

2 Likes
4

Is there a version of nixpkgs formatted with 0.3.1? That would be the easiest way to review. I’d like to check up on some of the expressions I wrote :slight_smile:

5

Since you are all lazy :slight_smile: It took 10 minutes to format all of nixpkgs with one CPU pinned at 100%:

6 Likes
6

Theres some bikesheds to paint of course, but looking pretty good :slight_smile:

If anyone else wants to review the changes to expressions involving them, here is what I used:

cd nixpkgs
git remote add zimbatm git@github.com:zimbatm/nixpkgs
git fetch zimbatm
git co zimbatm/nixpkgs-fmt-0.3.1-demo
git diff HEAD~1 $( rg -l 'timokau' )
2 Likes
7

Thank you @zimbatm. I look forward to this feature! Just a JSON representation to and from nixpkgs-fmt is great for other tools to bootstrap off of the work that nixpkgs-fmt does. I will be writting several nixpkgs transformers once this is available.

8

And if one uses an optimized build instead of a debug build, it takes less than 30 seconds :stuck_out_tongue_winking_eye:

5 Likes
9

Much better :stuck_out_tongue:

[@x1:~/.../NixOS/nixpkgs]$ time nixpkgs-fmt .

real	0m15.774s
user	0m43.099s
sys	0m2.064s
2 Likes
10

I think that the command run git ls-files | grep -e ".nix$" | xargs nixpkgs-fmt could be improved to deal with the files in parallel with GNU parallel or parallel (the Rust one).

Anyway, amazing work ! I will gladly use it ! I will see if I can make myself useful with your codebase.

11

@TheSirC nixpkgs-fmt now does parallel directory traversal itself (using ripgrep’s ignore), so there’s no need for git ls-files anymore.

2 Likes
12

Oh great ! Did not had the time take a look at the code yet and as I saw xargs I thought it was a good opportunity to parallelize the calls to nixpkgs-fmt. But given the timings you have and your comment it was a bit premature, excuse my mindless comment :sweat_smile: