Extend single-line list formatting beyond function arguments

[dyegoaurelio]

Previously, single-line list formatting was limited to lists within function arguments. This change extends the behavior to all lists, applying the same rules:

• Lists with >6 items always: expand
• Lists with >1 items and at least one non-simple item: expand
• Otherwise: try to fit on one line

nixpkgs diff is empty because if the brackets are on different lines, we keep them like that (unless we're on--strict mode)

The diff on the test cases were positive IMO.

closes #206

[github-actions]

Nixpkgs diff

[MattSturgeon]

I agree, most of the diff to our testsuite is good. However our testsuite is biased toward trivial examples.

For context, the reason nixfmt usually prefers expanding to multiline lists is to avoid diff noise when those lists are modified in future PRs.

This is most obvious in lists passed to mkDerivation, such as meta.maintainers, buildInputs, etc. These inputs should almost-always be multiline lists, even if they currently only contain a few items.

On the other hand more "general" non-package code, such as library functions, may be using more trivial lists that benefit from being single line and where diff noise is less of a concern. The challenge is implementing a way to distinguish between lists that benefit from being multiline vs list that benefit from being singleline.

You could argue that this is on the author to decide, however that then opens the door for nit-pick PR feedback, where reviewers waste time commenting on singleline lists that should really be multiline (and vice versa).

This is why nixfmt has (so far) had a firmer stance of any list with multiple items must be expanded.

All that said, I'm personally in favour of finding a way to allow more singleline lists. I'm just conscious of doing it in a way that still enforces lists that are likely to change over time remain expanded to multiline formatting.

Maybe one heuristic could be "any token in the list is more than X chars long"? Or "any element in the list references a variable binding"? Not sure, just throwing ideas out there.

Another heuristic may be that if the list is itself the value of an attribute binding, then it is likely to change over time. Although, this won't catch foo = [ x y z ] ++ optionals bar [ a b c ], since in that example [ a b c ] is not being bound directly to foo.

[MattSturgeon]

This is a case where the expanded formatting is preferred:

Suggested change

	[ pkgs.xorg.fontadobe100dpi pkgs.xorg.fontadobe75dpi ];
	[
	pkgs.xorg.fontadobe100dpi
	pkgs.xorg.fontadobe75dpi
	];

[MattSturgeon]

Personally, I'd prefer that we retain multiline/expanded formatting when concatenating as part of a chain of lists.

Suggested change

	++ optionals config.boot.vesa [ "vga=0x317" "nomodeset" ];
	++ optionals config.boot.vesa [
	"vga=0x317"
	"nomodeset"
	];

[MattSturgeon]

I could live with this more trivial list being contracted, but again I'd prefer it to remain expanded:

Suggested change

	++ lib.optionals config.security.pam.krb5.enable [ pam_krb5 pam_ccreds ];
	++ lib.optionals config.security.pam.krb5.enable [
	pam_krb5
	pam_ccreds
	];

[MattSturgeon]

Another example that benefits from being expanded:

Suggested change

	specialAttrs = [ "__functor" "__functionArgs" "__toString" "__pretty" ];
	specialAttrs = [
	"__functor"
	"__functionArgs"
	"__toString"
	"__pretty"
	];

[MattSturgeon]

I agree, trivial cases like this benefit from being allowed to contract.

[dyegoaurelio]

Another heuristic may be that if the list is itself the value of an attribute binding, then it is likely to change over time. Although, this won't catch "foo = [ x y z ] ++ optionals bar [ a b c ], since in that example [ a b c ]is not being bound _directly_ tofoo`.

We're already using the isSimple check, which removes things like sets and functions with a lot arguments.

I think we could do something like this

allowedToContract (Token (LoneAnn (Identifier txt))) = txt  == "false" || txt == "true" -- allow only boolean literals.
allowedToContract (Selection {}) = False -- e.g pkgs.xorg
allowedToContract t = isSimple (Term t)

this prevents changes such as this one

index 063dc21..fd242fb 100644
--- a/test/diff/monsters_3/out-pure.nix
+++ b/test/diff/monsters_3/out-pure.nix
@@ -45,13 +45,7 @@ stdenv.mkDerivation rec {
     wrapGAppsHook4
     glib # for glib-compile-resources
   ];
-  buildInputs = [
-    cairo
-    glib
-    gtk4
-    libadwaita
-    pango
-  ];
+  buildInputs = [ cairo glib gtk4 libadwaita pango ];

while allowing changes such as this one

-  (map (
-    buildAllowCommand "allow" [
-      "snapshot"
-      "mount"
-      "destroy"
-    ]
-  ))
+  (map (buildAllowCommand "allow" [ "snapshot" "mount" "destroy" ]))

[jfly]

We discussed this in today's team meeting, and agree with @MattSturgeon's comments above. We'd like to see either heuristic(s) that handle those cases, or a cli option (as requested in #206).

It would also be great to discuss this in real time at our next team meeting!

[nixos-discourse]

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

https://discourse.nixos.org/t/formatting-team-meeting-2025-11-11/72019/1

[bentenjamin]

This is an awesome change, I really like the way it takes care of single brackets on their own line. I made this eldritch abomination (which is probably not very good nix code) that would benefit greatly:

  # Flatten this set/file into the format expected by firefox config file, for example `"browser.urlbar.trimHttps" = true"`
  # A functor allows a set to be called like a function. It's first argument is always itself.
  # Nesting the arguments like below allows this function to be called on either itself or another set
  # flatten = import ./settings.nix
  # settings = flatten {};
  # settings = flatten default-profile-settings-set;
  __functor =
    self:
    {
      settings ? self,
    }:
    let
      recurse =
        path:
        lib.concatMapAttrs (
          name: value:
          if builtins.isAttrs value then
            recurse (path ++ [ name ]) value
          else if builtins.isFunction value then
            { }
          else
            {
              ${
                builtins.concatStringsSep "." (
                  path
                  ++ [
                    name
                  ]
                )
              } =
                value;
            }
        );
    in
    recurse [ ] settings;