TEST: reformat with latest nixfmt#2209
TEST: reformat with latest nixfmt#2209MattSturgeon wants to merge 2 commits intonix-community:mainfrom
Conversation
This one looks cleanest to me |
|
Tried playing with some (mkRenamedOptionModule [ "foo" "bar" ] [
"hello"
"world"
])(mkRenamedOptionModule [ "foo" "bar" ] (
foo
++ [
"hello"
"world"
]
))(mkRenamedOptionModule
(
foo
++ [
"bar"
"baz"
]
)
[
"hello"
"world"
]
)The final argument is always expanded to multiple lines, even if the line length would remain short. Additionally, any concatenation changes the semantics as far as nixfmt is concerned. Concatenation no longer qualifies as a "list used as a function argument"; instead it is treated as "part of a concatenation expression". It is therefore always expanded to multiple lines 😭 This isn't the best place to discuss nix formatting; we should probably raise this as feedback in the nixfmt repo I guess. Although there's already NixOS/nixfmt#206 and NixOS/nixfmt#228, which are related. For now I'll just cc some of the formatting team: |
|
I'm not a member of the formatting team :) though I play one on TV. |
I was going to try blaming you when I saw you tagged but then saw you weren't the one who did the commit :P |
|
With NixOS/nixfmt#257 this is fixed now! Though be aware of NixOS/nixfmt#224 |
ad7e1c9 to
bf736d8
Compare
plugins/by-name/lazygit/default.nix
Outdated
| config_file_path = defaultNullOpts.mkNullable ( | ||
| with types; either str (listOf str) | ||
| ) [ ] "Custom config file path or list of custom config file paths."; | ||
| config_file_path = defaultNullOpts.mkNullable (with types; either str (listOf str)) [ | ||
| ] "Custom config file path or list of custom config file paths."; |
There was a problem hiding this comment.
TBH, I'm not a huge fan of either of these formats, but breaking within an empty [ ] seems particularly strange. Is that intended @infinisil ?
There was a problem hiding this comment.
Personally, I'd like to see long multi-arg function calls like this wrapped with one arg per line, e.g.
config_file_path =
defaultNullOpts.mkNullable
(with types; either str (listOf str))
[ ]
"Custom config file path or list of custom config file paths.";or
config_file_path = defaultNullOpts.mkNullable
(with types; either str (listOf str))
[ ]
"Custom config file path or list of custom config file paths.";Should I raise this in the formatting matrix, open an issue upstream, or has this already been discussed elsewhere?
There was a problem hiding this comment.
Sure thing. I've opened NixOS/nixfmt#267 and NixOS/nixfmt#268, for the proposed formatting and weird line-break on empty list, respectively.
This must be done manually, because nixfmt won't "join" lists that are already multi-line.
d5358ca to
65dd7a9
Compare
Just to satisfy my curiosity, this is what the repo looks like when running
Note: some of the new formatting uses input-formatting-based heuristics, so if you (for example) wanted to reformat the following, it may require manual intervention.
We can experiment with additional commits on this branch, manually modifying input formatting and re-running nixfmt to see what it now permits.