From 8acc6e21210c64109ffcecdc9eed0d79f1a4960a Mon Sep 17 00:00:00 2001 From: happysalada Date: Tue, 17 Aug 2021 12:43:58 +0900 Subject: [PATCH 1/7] [101] nix formatting --- 0101-nix formatting.md | 65 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 0101-nix formatting.md diff --git a/0101-nix formatting.md b/0101-nix formatting.md new file mode 100644 index 000000000..a06134aca --- /dev/null +++ b/0101-nix formatting.md @@ -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: (names, to be nominated and accepted by RFC steering committee) +shepherd-leader: (name to be appointed by RFC steering committee) +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. +- Potentially agree on a hook to enforce formatting + +# Examples and Interactions + +[examples-and-interactions]: #examples-and-interactions + +This section is not needed for this RFC. + +# Drawbacks + +[drawbacks]: #drawbacks + +- Having a commit that changes the formatting, can make git blame harder to use. It will need `--ignore-rev` flag. +- Every formatter will have bugs. An example of a bug for nixpkgs-fmt can be found [here](https://github.com/NixOS/nixpkgs/pull/129392) + +# Alternatives + +[alternatives]: #alternatives + +- Alternative formatter [nixfmt](https://github.com/serokell/nixfmt) +- 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). + +# 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? From f4c25e75a0069ac4a69f173ba3ea6231441cedd1 Mon Sep 17 00:00:00 2001 From: Raphael Megzari Date: Sat, 21 Aug 2021 15:55:01 +0900 Subject: [PATCH 2/7] Update 0101-nix formatting.md Co-authored-by: Sandro --- 0101-nix formatting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0101-nix formatting.md b/0101-nix formatting.md index a06134aca..bdcca0c6a 100644 --- a/0101-nix formatting.md +++ b/0101-nix formatting.md @@ -55,7 +55,7 @@ This section is not needed for this RFC. [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 ? +- Are there situation where automated formatting is worse than manual formatting? Do we want to have exceptions to automated formatting? # Future work From ac8274bc7b1cac49528105e7b28462f59872ae68 Mon Sep 17 00:00:00 2001 From: happysalada Date: Fri, 24 Sep 2021 00:37:59 +0900 Subject: [PATCH 3/7] rfc101: add shepherd team and leader --- 0101-nix formatting.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/0101-nix formatting.md b/0101-nix formatting.md index bdcca0c6a..fe6e53659 100644 --- a/0101-nix formatting.md +++ b/0101-nix formatting.md @@ -3,8 +3,8 @@ 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: (names, to be nominated and accepted by RFC steering committee) -shepherd-leader: (name to be appointed by RFC steering committee) +shepherd-team: Timothy DeHerrera (nrdxp), 0x4A6F +shepherd-leader: Jonas Chevalier (zimbatm) related-issues: (will contain links to implementation PRs) --- From 564f9e89975af2c725819587402858a3f7f937e0 Mon Sep 17 00:00:00 2001 From: happysalada Date: Wed, 9 Mar 2022 22:29:37 -0500 Subject: [PATCH 4/7] [rfc101]: add infinisil as shepherd --- 0101-nix formatting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0101-nix formatting.md b/0101-nix formatting.md index fe6e53659..7f1555f65 100644 --- a/0101-nix formatting.md +++ b/0101-nix formatting.md @@ -3,7 +3,7 @@ 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-team: Timothy DeHerrera (nrdxp), 0x4A6F, Silvan Mosberger (infinisil) shepherd-leader: Jonas Chevalier (zimbatm) related-issues: (will contain links to implementation PRs) --- From e66eb4c78ab29ab6d0db7971286df9699ae2f917 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 6 Apr 2023 17:05:48 +0200 Subject: [PATCH 5/7] Update from hedgedoc --- 0101-nix formatting.md | 525 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 504 insertions(+), 21 deletions(-) diff --git a/0101-nix formatting.md b/0101-nix formatting.md index 7f1555f65..f6824469c 100644 --- a/0101-nix formatting.md +++ b/0101-nix formatting.md @@ -3,8 +3,8 @@ 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, Silvan Mosberger (infinisil) -shepherd-leader: Jonas Chevalier (zimbatm) +shepherd-team: 0x4A6F, Silvan Mosberger (infinisil), piegames, Thomas Bereknyei (tomberek) +shepherd-leader: Silvan Mosberger (infinisil) related-issues: (will contain links to implementation PRs) --- @@ -12,54 +12,537 @@ related-issues: (will contain links to implementation PRs) [summary]: #summary -Decide on a recommended automated formatter for nix files in nixpkgs. +Decide on basic Nix style guidelines and pick a default code formatter for Nix code. +Format Nixpkgs using that formatter and enforce it using CI. -# Motivation +## Motivation [motivation]: #motivation -Prevent debate around how things should be formatted. +TODO: Currently, there is no authoritative style guide for Nix code in Nixpkgs. +The currenty style has evolved organically over time, leading to inconsistencies both across files and language features. -# Detailed design +- We want to prevent debate around how things should be formatted. +- We want to make it as easy as possible for contributors (especially new ones) to make changes without having to worry about formatting. +- Conversely, reviewers should not be bothered with ill-formatted contributions. +- We want a unified Nix code style that's consistent with itself, easily readable, accessible and results in readable diffs. + +## Detailed design [design]: #detailed-design -The implementation of this RFC should include several parts +There are four main parts to this RFC: +- Establish basic formatting guidelines +- Pick a formatter implementation +- Migrate Nixpkgs to the format and enforce it in CI +- Create a process for maintenance of the implementation + +### Nix formatting style + +We introduce a set of general *guidelines* that describe the Nix formatting style (**Appendix A**). + +From these style guidelines, a set of more specific formatting rules is derived (**Appendix B**). + +The interactions between different language features are complex and producing a style that matches the expectations involves a lot of special cases. Edge cases abound. +Any attempt at creating an exhaustive rule set would be futile. +Therefore, the formatting rules are intentionally under-specified, leaving room for the formatter implementation. +However, the most important or potentially controversial rules are included. +The test suite of the formatter implementation specifies the exact formatting, it is up to the formatter team to adjust it as needed. +TODO?: While the style guidelines are fixed in this RFC, the formatting rules will be documented at TODO and maintained by the formatter team (see below). + +### Formatter tooling + +In order to pick a formatter, we looked at the following criteria: + +- Output: Capability of matching the desired output style +- Maintainability: The code base needs to be updated over time + - The language the formatter is written in matters + - The quality of the codebase + - The libraries the formatter depends on and their maintaindness + - Maintainability is more important than current maintenance situation, since this is a very long-term project TODO +- Tests: The code should be tested well +- Approach: + +In contrast, these are not criteria: + +- Diff size when applied to current Nixpkgs: As noted in the formatting recommendations, the desired output does take into account existing Nixpkgs idioms and preferred format, but the size of the treewide change is not significant +- Current usage metrics / Popularity +- Performance: Beyond a certain threshold optimizing for this will detract from other criteria. + +We looked at all the potential formatter tools and evaluated these criteria to our best knowledge: + +| Formatter | [alejandra](https://github.com/kamadorueda/alejandra) | [nixfmt](https://github.com/serokell/nixfmt) | [nixpkgs-fmt](https://github.com/nix-community/nixpkgs-fmt) | +| --- | --- | --- | --- | +| Output | ✅ | ✅ | ⚠️[^outputs] | +| Maintainability (language) | ✅(Rust) | ⚠️(Haskell)[^haskell] | ✅(Rust) | +| Maintainability (code quality) | ⚠️ | ✅ | ⚠️ | +| Maintainability (dependencies) | ✅ (rnix) | ❌[^internal] | ✅(rnix) | +| Test suite | ✅ | ❌ | ✅ | +| Approach[^transformation] | ✅ | ✅ | ❌[^transformation] | +| --- | --- | --- | --- | +| Nixpkgs test run | [#157382](https://github.com/NixOS/nixpkgs/pull/157382) | [#147608](https://github.com/NixOS/nixpkgs/pull/147608) | [#147623](https://github.com/NixOS/nixpkgs/pull/147623) | + +From this evaluation, we decide that the code base of alejandra will be used as the base. + +[^outputs]: formatting inside of anti-quotation +[^transformation]: Nixpkgs-fmt has the unique approach of applying modification rules to the Nix code, instead of parsing and pretty-printing it like most others. (See their README for details.) TODO something about maintainability +[^internal]: nixfmt ships with its own parser +[^haskell]: the Nix ecosystem is showing interest and signs of adopting Rust tooling; Nix bindings, Nickel, parsers, formatters, lsp, etc. + +### Formatting Nixpkgs + +Once the tooling fully conforms to the style guide, Nixpkgs will be fully reformatted. The output of the chosen formatter will be authoritative for Nixpkgs. + +The Nix Formatter Team is responsible for the migration of Nixpkgs, and will do so in close coordination with the release managers. + +CI will enforce the formatting on every pull request. The formatting hook will pin a version of the formatter for reproducibility, which is independent from the one provided by Nixpkgs. In order to minimize conflicts, the format used for Nixpkgs may only be updated shortly before release branch-off, at which point old pull requests will need to be rebased. + +In order to not clutter the history, formatting will be done all at once and the respective commit will be added to `.git-blame-ignore-revs`. In order to not cause any conflicts with back-porting, this will have to be done shortly before a release branch-off. Merge conflicts are unavoidable for most open pull requests. + +#### Staging + +Coordinate with release managers to merge the formatting PR in between two staging runs to avoid conflicts there. + +Other long-running branches (haskell, python, etc.) will not be considered, there will however be an announcement so that contributors can coordinate to not have any of these branches in use at that time either. + +In case a separate branch can't be merged into master again due to a formatting conflict, `git filter-branch` and a `git rebase` can still be used to resolve it without trouble. + +### Maintenance + +To help maintaining the formatter, a team is created. +It is given commit access the formatter repository living in the NixOS GitHub organization and has the authority to change the formatting rules. +It is bound to the style guide and rules specified in this RFC. + +Should new syntax features be introduced into Nix, the formatter team should be consulted prior to their introduction. -- 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. -- Potentially agree on a hook to enforce formatting +The team initially consists of: +- @infinisil +- @tomberek +- @piegames +- @0x4A6F +- @kamadorueda (if available, TODO: @tomberek sent a message) -# Examples and Interactions +The team has the authority to remove and add members as needed. + +## Examples and Interactions [examples-and-interactions]: #examples-and-interactions -This section is not needed for this RFC. +### Documentation + +Update [Section 20.1 (Coding conventiones → Syntax)](https://nixos.org/manual/nixpkgs/stable/#sec-syntax) of the Nixpkgs manual. All formatting-specific guidance is removed and replaced with instructions on how to automatically format Nixpkgs instead. + +### Automatically generated files + +TODO discuss that https://github.com/NixOS/rfcs/pull/101#discussion_r690272696 +- exclude option in tooling +- 2nix apply fomatting style -# Drawbacks +## Drawbacks [drawbacks]: #drawbacks - Having a commit that changes the formatting, can make git blame harder to use. It will need `--ignore-rev` flag. -- Every formatter will have bugs. An example of a bug for nixpkgs-fmt can be found [here](https://github.com/NixOS/nixpkgs/pull/129392) +- Every formatter will have bugs. -# Alternatives +## Alternatives [alternatives]: #alternatives -- Alternative formatter [nixfmt](https://github.com/serokell/nixfmt) - 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). -# Unresolved questions +### Incremental migration + +A more gradual approach, where only changed files get formatted for some period of time to make a more graceful transition has been rejected for practical concerns. Among other things, such an approach would not improve the amount of merge conflicts, and increase the workload for contributors during that time significantly. + +## 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 work [future]: #future-work -What future work, if any, would be implied or impacted by this feature -without being directly part of the work? +- TODO General style guidelines beyond AST reformatting + +## Appendix A: Formatting style guidelines + +**Nomenclature:** +Brackets: `[]` +Braces: `{}` +Parentheses: `()` + +*These are not strict rules but guidelines to help create more consistent formatting rules. +Nevertheless, deviations should be documented and explained.* + +- When deciding between two *equally good* options, currently prevalent formatting style in `nixpkgs` should be followed. +- On the top level (indentation zero), special rules may apply. +- Two spaces are used for each indentation level, there is no vertical alignment +- Consistency across language features is important. Syntax that behaves similarly should be formatted similarly. +- The Nix formatting style is space heavy. Where there can be a space, there should be a space. + - Brackets and braces are generally written with a space on the inside, like `[ `, ` ]`, `{ ` and ` }`. + - `[]` is written as `[ ]`, same for `{ }`. + - Exception: Parentheses are written *without* a space on the inside +- Statements are either written on one line, or maximally spread out across lines, with no in-between (e.g. grouping). + - Multi-line statements increment the indentation level in the statement's "body". + - Grouping should be done by adding blank lines or comments. +- Where ~~possible~~ sensible indentation depth should be minimized, "double-indentations" should be avoided. + - Examples are `… in {` and `… else if` + +## Appendix B: Formatting rules + +### Function declaration + +> Discussions: +> [1](https://github.com/kamadorueda/alejandra/issues/95) + +#### With Destructured Arguments + +✅ Good: + +```nix +{ mkDerivation, ... } @ attrs: + mkDerivation # ... +``` + +- Indenting the body relative to the function signature + hints that a new scope is introduced by the + function arguments. +- Keeping the signature in one line + when there is only 1 argument in the destructuring (`mkDerivation`) + helps saving vertical space. +- Spacing between elements of the destructuring, + and between opening and closing elements + is consistent with _List_ and _Attrset_. + +✅ Good: + +```nix +{ mkDerivation, ... } @ attrs: { + url, + sha256, + ... +}: +mkDerivation # ... +``` + +- When a file starts with one or more of nested function declarations, + it's valid not to indent the body of the function + because it's clear when reading the file from top to bottom + that the whole remaining of the file + is the scope of the function(s), + Therefore saving an unneeded indent. + +✅ Good: + +```nix +{ + mkDerivation, + lib, + fetchurl, + ... +} @ attrs: + stdenv.mkDerivation # ... +``` + +- Adding an argument produces a minimal diff + (including the first and last elements): + + ```patch + mkDerivation, + lib, + fetchurl, + + google-chrome-stable, + ``` + +- Removing an argument produces a minimal diff + (including the first and last elements): + + ```patch + mkDerivation, + - lib, + fetchurl, + ``` + +- The comma at the end is consistent with _Let-In_, and _Attrset_, + where the separator goes after the element + instead of at the beginning. + +❌ Bad: + +```nix +{ lib +, mkDerivation +, fetchurl +, ... +} @ attrs: +stdenv.mkDerivation # ... +``` + +- Removing the first element + produces a diff in two elements: + + ```diff + - { lib + - , mkDerivation + + { mkDerivation + , fetchurl + , ... + } @ attrs: + stdenv.mkDerivation # ... + ``` + +- Documenting the first argument creates an inconsistency + between the way arguments start: + + ```nix + { + # Lorem Ipsum + lib + , mkDerivation + , fetchurl + , ... + } @ attrs: + stdenv.mkDerivation # ... + ``` + +- This is not consistent with _Let-In_, and _AttrSet_, + where the separator goes after the element + instead of at the beginning. +- It ruins "folding by indentation" modes + on Vim, Neovim, VSCode, and other major code editors, + because the data-structure has the same indentation + as the opening brace. + +❌ Bad: + +```nix +{ mkDerivation, lib, fetchurl, ... }@attrs: stdenv.mkDerivation # ... +``` + +- One-liners are unreadable and produce bad diffs. + +❌ Bad: + +```nix +{ mkDerivation, lib, fetchurl, extra-cmake-modules, kdoctools, wrapGAppsHook +, karchive, kconfig, kcrash, kguiaddons, kinit, kparts, kwind, ... }@attrs: +stdenv.mkDerivation # ... +``` + +- It's hard to tell this destructuring has an ellipsis (`...`) at a first glance, + because it's mixed with the other arguments. +- Moving elements becomes harder + than a simple whole-line movement. + (Moving a whole line is normally a keyboard-shortcut + or command in major code editors). +- Excessively compact: + adding, removing, or editing an argument + produces a diff in more than one argument. +- `}@attrs` is not intuitive + with the rules of written english, + where you add whitespace + after the end of the previous phrase + (`phrase. Other phrase`). + +### Inherit + +✅ Good: +```nix +inherit foo bar; +inherit (attrs) foo bar; +inherit + foo + bar + ; +inherit (attrs) + foo + bar + ; +``` + +- Inherit expressions can have their symbols either all on the same line as the `inherit`, or one line per element each. +- The `(attrs)` is always on the same line as the `inherit` statement. There are no spaces on the inside of its parentheses. + - TODO what if attrs is long like in https://github.com/kamadorueda/alejandra/issues/367 ? + +❌ Bad: +```nix +TODO +inherit (attrs) foo + bar + ; +``` + +TODO + +Bad??? +```nix +inherit + (attrs) + foo + bar + ; +``` + +### If-Then-Else + +✅ Good: + +```nix +if predicate +then foo +else bar +``` + +- The keyword at the beginning of the line + states clearly the meaning of the content that follows. +- Produces a clean diff when you add more code. + For example: adding content to the `else` + only produces a diff in the `else`. + +❌ Bad: + +```nix +if predicate then foo else bar +``` + +- One-liners are hard to understand, + specially when nested, + or when logic gets long. +- Adding content produces a diff in the entire `if-then-else`. + +✅ Good: + +```nix +if something <= 2.0 +then + if somethingElse + then foo + else bar +else if something <= 4.0 +then { + foo = 10; +} +else if something <= 6.0 +then [ + 10 + 20 +] +else bar +``` + +- It's easy to follow that there are many conditionals. +- The indentation makes it easy to read + which expression is associated to each conditional. +- Adding or modifying the branches produces a clean diff. + +❌ Bad: + +```nix +if cond +then if + looooooooooooooooooooooooooooooooooooooooooooooooooooong +then foo +else bar +else if cond +then foo +else bar +``` + +- It's complex to distinguish the parent `if-then-else` + from the child `if-then-else` + +### Lists + +✅ Good: +```nix +[ foo bar baz ] +[ + foo + bar + baz +] +[ ] +``` + +- For consistency with other formatting rules, more spacing is preferred + +❌ Bad: +```nix +[foo bar baz] +[] +``` + +### Function applications + +✅ Good: +```nix +fun arg1 arg2 arg3 + + +callPackage ./some/path { + foo = "bar"; +} ( + if foo + then bar + else baz +) + + +if something <= 2.0 +then + if somethingElse + then foo + else bar +else if something <= 4.0 +then + { + foo = 10; + } // { + bar = 20; + } +else if something <= 6.0 +then ( + if foo + then bar + else baz +) +else bar + + + +callPackage + ./some/path + { + foo = "bar"; + } + ( + if foo + then bar + else baz + ) +``` + +- Consistent with `if then else` (except that the + +❌ Bad: +```nix + +# - Rightward drift +# - Does not minimize indentation +callPackage ./some/path { + foo = "bar"; +} { + bar = "baz"; + } { + baz = "foobar"; + } +``` + +### String interpolation + +TODO https://github.com/kamadorueda/alejandra/issues/242 + +Find a solution which is friendly to formatter implementations From c040ff48a00810f5f2176f6bd044dcb012c87d7d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 18 Apr 2023 23:00:13 +0200 Subject: [PATCH 6/7] Update after today's meeting Co-Authored-By: piegames --- 0101-nix formatting.md | 131 ++++++++++++++++++++++++++++++++++------- 1 file changed, 109 insertions(+), 22 deletions(-) diff --git a/0101-nix formatting.md b/0101-nix formatting.md index f6824469c..ae52f4b50 100644 --- a/0101-nix formatting.md +++ b/0101-nix formatting.md @@ -128,6 +128,30 @@ The team has the authority to remove and add members as needed. [examples-and-interactions]: #examples-and-interactions +TODO sort that in somewhere else +```nix +[ + "--some-flag" "some-value" +] +``` + +is formatted as + +``` +[ + "--some-flag" + "some-value" +] +``` + +workaround: + +``` +lib.cli.toGNUCommandLine {} { + some-flag = "some-value"; +} +``` + ### Documentation Update [Section 20.1 (Coding conventiones → Syntax)](https://nixos.org/manual/nixpkgs/stable/#sec-syntax) of the Nixpkgs manual. All formatting-specific guidance is removed and replaced with instructions on how to automatically format Nixpkgs instead. @@ -185,11 +209,11 @@ Nevertheless, deviations should be documented and explained.* - Brackets and braces are generally written with a space on the inside, like `[ `, ` ]`, `{ ` and ` }`. - `[]` is written as `[ ]`, same for `{ }`. - Exception: Parentheses are written *without* a space on the inside -- Statements are either written on one line, or maximally spread out across lines, with no in-between (e.g. grouping). +- Expressions are either written on one line, or maximally spread out across lines, with no in-between (e.g. grouping). - Multi-line statements increment the indentation level in the statement's "body". - Grouping should be done by adding blank lines or comments. -- Where ~~possible~~ sensible indentation depth should be minimized, "double-indentations" should be avoided. - - Examples are `… in {` and `… else if` + - If there is more than one element in a list or attribute set, it should be expanded to put each on its own line + - Rationale: singleton lists are common ## Appendix B: Formatting rules @@ -390,17 +414,30 @@ inherit ✅ Good: +```nix +if predicate then + foo +else + bar +``` + +- "Pulling up" the `then` keyword is consistent with other similar syntax constructs +- The bodies are always indented + +❌ Bad (old): + ```nix if predicate then foo else bar ``` -- The keyword at the beginning of the line - states clearly the meaning of the content that follows. -- Produces a clean diff when you add more code. +- TODO explain why bad +- ~~The keyword at the beginning of the line + states clearly the meaning of the content that follows.~~ +- ~~Produces a clean diff when you add more code. For example: adding content to the `else` - only produces a diff in the `else`. + only produces a diff in the `else`.~~ ❌ Bad: @@ -416,21 +453,27 @@ if predicate then foo else bar ✅ Good: ```nix -if something <= 2.0 -then - if somethingElse - then foo - else bar -else if something <= 4.0 -then { - foo = 10; -} -else if something <= 6.0 -then [ - 10 - 20 -] -else bar +if something <= 2.0 then + if somethingElse then + foo + else + bar +else if something <= 4.0 then + { + foo = 10; + } +else if something <= 6.0 then + [ + 10 + 20 + ] +else if something <= 7.0 then + let + … + in + … +else + bar ``` - It's easy to follow that there are many conditionals. @@ -454,6 +497,50 @@ else bar - It's complex to distinguish the parent `if-then-else` from the child `if-then-else` +❌ Bad: + +```nix +if cond1 then [ + foo +] else if cond2 then + bar +else if cond3 then { + baz = "…"; +} else if cond4 then let + qux = "…"; +in + qux +else + true +``` + +- Inconsistent start of line +- if/else keywords not aligned: the first `else` is indented like if it was nested +- Easy to miss the opening brace/bracket, especially when the closing one is far below +- No clear distinction whether items are in a list or not + +✅ Good: + +```nix +if + multiline + && condition +then + content +else if + myPredicate [ + multi + line + ] +then + other content +else + more +``` + +- If the condition does not fit one line, expand with indentation, and put the `then` keyword on a new line +- Recommendation: avoid long conditions + ### Lists ✅ Good: From afaa2856680a4bfab1619fc2e9130be7bb1ac9ae Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 3 May 2023 00:36:37 +0200 Subject: [PATCH 7/7] Updates from the meeting --- 0101-nix formatting.md | 47 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/0101-nix formatting.md b/0101-nix formatting.md index ae52f4b50..ab20f5137 100644 --- a/0101-nix formatting.md +++ b/0101-nix formatting.md @@ -203,14 +203,17 @@ Nevertheless, deviations should be documented and explained.* - When deciding between two *equally good* options, currently prevalent formatting style in `nixpkgs` should be followed. - On the top level (indentation zero), special rules may apply. + - TODO: Is this still relevant? - Two spaces are used for each indentation level, there is no vertical alignment - Consistency across language features is important. Syntax that behaves similarly should be formatted similarly. - The Nix formatting style is space heavy. Where there can be a space, there should be a space. - Brackets and braces are generally written with a space on the inside, like `[ `, ` ]`, `{ ` and ` }`. - `[]` is written as `[ ]`, same for `{ }`. - Exception: Parentheses are written *without* a space on the inside +- Any two expressions that are fully on a single line must have a common (transitive) parent expression which is also fully on that line. + - Equivalently: If a maximally parenthesized form of a line fully contains a parenthesis pair, there must be a single outermost pair on that line, meaning it contains all of the others. + - TODO: Consider moving this - Expressions are either written on one line, or maximally spread out across lines, with no in-between (e.g. grouping). - - Multi-line statements increment the indentation level in the statement's "body". - Grouping should be done by adding blank lines or comments. - If there is more than one element in a list or attribute set, it should be expanded to put each on its own line - Rationale: singleton lists are common @@ -389,7 +392,6 @@ inherit (attrs) - Inherit expressions can have their symbols either all on the same line as the `inherit`, or one line per element each. - The `(attrs)` is always on the same line as the `inherit` statement. There are no spaces on the inside of its parentheses. - - TODO what if attrs is long like in https://github.com/kamadorueda/alejandra/issues/367 ? ❌ Bad: ```nix @@ -399,9 +401,7 @@ inherit (attrs) foo ; ``` -TODO - -Bad??? +❌ Bad: ```nix inherit (attrs) @@ -410,6 +410,27 @@ inherit ; ``` +✅ Good: + +```nix +{ + inherit + (mkFruit { + a = "apple"; + b = "banana"; + c = "cherry"; + d = "date"; + e = "elderberry"; + }) + feijoa + grape + honeydew + ; +} +``` + +- Technically this is inconsistent because the attribute set is sometimes on the same line as the `inherit` and other times not. However, this is a really niche use case and generally you won't need to write such code anyways so we are willing to sacrifice consistency for a better default. + ### If-Then-Else ✅ Good: @@ -633,3 +654,19 @@ callPackage ./some/path { TODO https://github.com/kamadorueda/alejandra/issues/242 Find a solution which is friendly to formatter implementations + + + + + + + + + + + + + + + +