Skip to content

Conversation

@jfly
Copy link
Contributor

@jfly jfly commented Apr 13, 2025

Ping @NixOS/nix-formatting, since I can't see to add the team as a reviewer on this PR.

We now have some decent tooling around this, but it's not very discoverable for people new to nix. We (the formatting team) believe nix.dev is a good place for this documentation to live.

I copied this text (with slight modifications) from nix eval --raw nixpkgs#nixfmt-tree.meta.longDescription.

Copy link

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this!

formatter.''${system} = nixpkgs.legacyPackages.''${system}.nixfmt-tree;
```

- The same can be done more efficiently with the `treefmt` command,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please show exactly one way of doing things, and since official documentation is committed to stable features (and you already point out that it's more efficient) let's use the one where you add the package to your shell environment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please show exactly one way of doing things

I'm not sure I agree we should limit ourselves to only documenting a single approach to things.

nix fmt and nix-shell --run treefmt both have their own pros and cons, I'd expect users would want to be aware of each.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realty is, we have to balance cognitive load against factual completeness. This place is about managing cognitive load, reference documentation is for completeness. I'd appreciate discussion in reference documentation and issue threads on the costs and benefits of various approaches, but the motto of nix.dev is "getting things done" -- not "agonizing over the options". We're free to agonize in these behind the scenes exchanges, but we must not spill that on beginners.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh I'm not sure if nix fmt has a benefit other than it being a shorter command, but we should recommend direnv anyways, in which case treefmt has the exact same length. I'd be fine with dropping nix fmt, though open about other benefits I'm missing!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed nix fmt. We still do unfortunately document 3 different ways of doing things (nixfmt-tree, treefmt.withConfig, and finally treefmt-nix), but I've tried to convey the reason you would choose the next step.

}
```

Alternatively you can switch to the more fully-featured [treefmt-nix](https://github.com/numtide/treefmt-nix).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than propose ever more alternative ways of doing things, I'd appreciate (and review) a guide to set up a pre-commit hook with treefmt-nix.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While they both relate to formatting, and both produce a configured instance of treefmt, pkgs.nixfmt-tree, pkgs.treefmt.withConfig, and treefmt-nix all exist for different reasons and suit different use cases.

Users who just want to configure formatting nix files in their project shouldn't need to download or configure treefmt-nix; there's a solution built into nixpkgs.

Users who want a more complex formatting setup, where they end up configuring several treefmt options, may benefit from using either treefmt-nix or pkgs.treefmt.withConfig.

Copy link
Collaborator

@fricklerhandwerk fricklerhandwerk Apr 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, and then we'd need to ask what's the purpose of any given article, and whether this information may be better placed elsewhere, such as is Nixpkgs contributor docs.

This sort of problem appears all of the time here, and sometimes one first needs to write an entirely different guide to to write the guide one originally wanted to write. Otherwise it becomes just as unfocused and confusing as we still have an unfortunate reputation for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken another stab at the docs. It's hopefully less of a choose your own adventure, and more of a journey along increasingly complex, but more powerful options, and you can stop anywhere along that journey if you want.

a guide to set up a pre-commit hook with treefmt-nix.

I opted to leave this out, for a couple of reasons:

  1. It would assume you're working in a git repo.
  2. It solves the problem at the wrong layer. IMO the right thing to do is to get people configuring their editors to autoformat for them (which conveniently would not care if you're in a git repo or not).

@infinisil
Copy link
Member

From the formatting team meeting today:

  • Discussed changes to make, there are legitimate reasons for keeping documentation of multiple options, but we can remove some of the options, in particular:
    • nix fmt -> Use a shell with treefmt instead
    • nixfmt-tree.override -> Use treefmt.withConfig instead
    • treefmt --ci -> Use treefmt-nix instead
  • In general briefly mention when one would want to use the next step
  • @jfly will also check about the possibility of (moving treefmt-nix into Nixpkgs](https://discourse.nixos.org/t/formatting-team-meeting-2025-03-18/61868/2) (potentially combining it with treefmt.withConfig), which would remove one more way of doing things
    • But this might hurt the development speed of treefmt-nix considerably

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/formatting-team-meeting-2025-04-15/63097/1

Copy link
Contributor Author

@jfly jfly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated this PR with a second, more streamlined version of the docs.

formatter.''${system} = nixpkgs.legacyPackages.''${system}.nixfmt-tree;
```

- The same can be done more efficiently with the `treefmt` command,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed nix fmt. We still do unfortunately document 3 different ways of doing things (nixfmt-tree, treefmt.withConfig, and finally treefmt-nix), but I've tried to convey the reason you would choose the next step.

}
```

Alternatively you can switch to the more fully-featured [treefmt-nix](https://github.com/numtide/treefmt-nix).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken another stab at the docs. It's hopefully less of a choose your own adventure, and more of a journey along increasingly complex, but more powerful options, and you can stop anywhere along that journey if you want.

a guide to set up a pre-commit hook with treefmt-nix.

I opted to leave this out, for a couple of reasons:

  1. It would assume you're working in a git repo.
  2. It solves the problem at the wrong layer. IMO the right thing to do is to get people configuring their editors to autoformat for them (which conveniently would not care if you're in a git repo or not).

Copy link

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. Minor comments, thoughts & suggestions:


This can get a little tedious.
[treefmt-nix](https://github.com/numtide/treefmt-nix) has a big library of
preconfigured formatters, and provides a `check` derivation you can use in CI.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're mentioning the check derivation, should we include a (simple) example of using it?

If flakes are acceptable in nix.dev guides, maybe adding the drv to a flake's checks output would be a suitable example?

Such an example may be a good opportunity to "sneak in" defining formatter and devshell outputs, although not if that ends up making the example less simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it would be nice to include a simple example of using the check derivation, but that requires either using npins or flakes to pull in treefmt-nix. It gets messy kind of fast, and I'm lightly leaning towards "just send them to treefmt-nix to figure this out", as I've done here.

I'm very ambivalent on this, though. I'm going to leave this unresolved, perhaps someone with strong opinions will show up.

We now have some decent tooling around this, but it's not very
discoverable for people new to nix. We (the formatting team) believe
`nix.dev` is a good place for this documentation to live.
Copy link

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from my perspective 😁

Thanks @jfly

@Mic92 Mic92 merged commit c76bc9b into NixOS:master Apr 29, 2025
4 of 5 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Nix formatting Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants