[RFC 0101] Nix formatting #101
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| --- | ||
| feature: nix_formatting | ||
| start-date: 2021-08-17 | ||
| author: Raphael Megzari | ||
| co-authors: (find a buddy later to help out with the RFC) | ||
| shepherd-team: Timothy DeHerrera (nrdxp), 0x4A6F | ||
| shepherd-leader: Jonas Chevalier (zimbatm) | ||
| related-issues: (will contain links to implementation PRs) | ||
| --- | ||
|
|
||
| # Summary | ||
|
|
||
| [summary]: #summary | ||
|
|
||
| Decide on a recommended automated formatter for nix files in nixpkgs. | ||
|
|
||
| # Motivation | ||
|
|
||
| [motivation]: #motivation | ||
|
|
||
| Prevent debate around how things should be formatted. | ||
|
|
||
| # Detailed design | ||
|
|
||
| [design]: #detailed-design | ||
|
|
||
| The implementation of this RFC should include several parts | ||
|
|
||
| - Agree on an automated formatter. [nixpkgs-fmt](https://github.com/nix-community/nixpkgs-fmt) is proposed here. | ||
| - If adopted, agree on a schedule to format nixpkgs codebase. Should it be done at once, or on a per package basis. | ||
|
There was a problem hiding this comment. One good point made by Domen in NixOS/nixpkgs#121490 is that it should be done once, on a treewide basis, just before branch-off of a stable release: Otherwise backporting changes from unstable will almost always run into merge conflicts which have to be manually resolved. There was a problem hiding this comment. If nixpkgs-fmt changes the content of values, I would consider this as a bug. Unfortunately due to how the internal data structures that rnix-parser is using, deal with multi-line strings and comments has been particularly challenging. If somebody could extract the repro and post an issue to nixpkgs-fmt that would be great. |
||
| - Potentially agree on a hook to enforce formatting | ||
|
There was a problem hiding this comment. This could be implemented as a github action pretty easily. |
||
|
|
||
|
There was a problem hiding this comment. I think a few formatting choices still need to be discussed. One of the largest issues is probably where the trailing commands in attrsets should be nix-community/nixpkgs-fmt#248. The main arguments have probably already been said in that issue - I don't know if we should continue the discussion here or there. |
||
| # Examples and Interactions | ||
|
|
||
| [examples-and-interactions]: #examples-and-interactions | ||
|
|
||
| This section is not needed for this RFC. | ||
|
There was a problem hiding this comment. I think if adding an exact specification of how files are formatted is too much work a few representative samples should at least be added here. I think that would be a fair compromise between effort and at least having some reference. Other than that the formatter at a specified commit is probably the ground truth. |
||
|
|
||
| # Drawbacks | ||
|
|
||
| [drawbacks]: #drawbacks | ||
|
|
||
| - Having a commit that changes the formatting, can make git blame harder to use. It will need `--ignore-rev` flag. | ||
|
There was a problem hiding this comment. Ignore-rev can be set up convenient-ish-ly by contributors by running one to two There was a problem hiding this comment. Made a pr for a According to a github employee they're currently working on the web blame
There was a problem hiding this comment. This feature is now live on GitHub: https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view |
||
| - Every formatter will have bugs. An example of a bug for nixpkgs-fmt can be found [here](https://github.com/NixOS/nixpkgs/pull/129392) | ||
|
There was a problem hiding this comment. Another drawback is that, as it stands This implies that the “ideal” solution I've outlined in my other comment may not actually be doable. There was a problem hiding this comment. Alejandra gets multi-line strings done perfectly, plus only 94 out of 34079 derivations would need rebuilding (and for reasons that can be happily ignored, see footnote on the readme) |
||
|
|
||
| # Alternatives | ||
|
|
||
| [alternatives]: #alternatives | ||
|
|
||
| - Alternative formatter [nixfmt](https://github.com/serokell/nixfmt) | ||
|
There was a problem hiding this comment. Enforces short line lengths which is not ideal with src in fetchers. There was a problem hiding this comment. If single string literal is too long, it is kept, so I see little problem with it. Can you provide example when it generates outright ugly results? |
||
| - Keep the status quo of not having an official formatter. The danger is that this creates discord within the nix community. On top of fragmented the community it can generate lengthy discussions (which do not advance the eco-system). | ||
|
There was a problem hiding this comment. There is also an alternative of explicitly discouraging any comments on formatting not directly based on coding style recommendations spelled out in the Nixpkgs manual. There was a problem hiding this comment. I'd say that it would be step in wrong direction. As far as I remember, nothing in nixpkgs manual says that your expression should contain newlines at all. There was a problem hiding this comment. Well, discouraging is not the same as strictly forbidding, if you in good faith consider something an exceptionally unfortunate formatting decision, you could comment about that… with a link to a PR to the manual, obviously. There was a problem hiding this comment. Where would you draw the line? It should be explicit, otherwise one reviewer thinks one part is exceptionally unfortunate and the next thinks another part is exceptionally unfortunate. |
||
|
|
||
| # Unresolved questions | ||
|
|
||
| [unresolved]: #unresolved-questions | ||
|
|
||
| - Not sure how much work there is left on nixpkgs-fmt before most people would consider it ok to use. Not even sure how much it is actually used. | ||
| - Are there situation where automated formatting is worse than manual formatting? Do we want to have exceptions to automated formatting? | ||
|
|
||
| # Future work | ||
|
|
||
| [future]: #future-work | ||
|
|
||
| What future work, if any, would be implied or impacted by this feature | ||
| without being directly part of the work? | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That formatter would need to become a channel blocker for the
nixpkgs-*channels (like e. g.nixpkgs-reviewis).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to propose Alejandra to be taken into consideration as well.