demo of nixpkgs-fmt 35ae8e6498e23d1b05091976acc0ae2a6f749096 #82932
Conversation
ofborg
bot
added
6.topic: emacs
6.topic: erlang
6.topic: fetch
6.topic: GNOME
|
Is evaluation failure caused by a merge problem, or by the original revision being broken? Random observation: switch-like «else if» chains are better off without ever-increasing indent level. Not sure what is the best criterion; maybe «else» with the body containing only an «if» (possibly with another «else») should be indented as a special «else+if» thing? |
| (import ./default.nix { }).overrideAttrs | ||
| (x: { |
There was a problem hiding this comment.
I think the original version was more readable.
| @@ -7,7 +7,6 @@ | |||
|
|
|||
| outputs = { self }: | |||
| let | |||
|
|
|||
There was a problem hiding this comment.
as a general rule nixpkgs-fmt doesn't collapse lines, but not 100% sure in this case
| } | ||
| ); |
There was a problem hiding this comment.
these two should be collapsed. I think the rule is that if N containers are opened on the same line, then they should be closed on the same line as well
| if attrPath == [ ] then e | ||
| else | ||
| if e ? ${attr} | ||
| then attrByPath (tail attrPath) default e.${attr} |
There was a problem hiding this comment.
not sure if it's worth the extra complexity to handle the else if case. the flat version looks nicer.
There was a problem hiding this comment.
I agree this case would be better with else if but not worth it. I have commented on a larger example.
| fold ( | ||
| n: a: |
There was a problem hiding this comment.
not sure why n: a: was pushed to the next line here. to be investigated.
| if builtins.stringLength k == 1 | ||
| then "-${k}" | ||
| else "--${k}" |
There was a problem hiding this comment.
You meant relative to k:, right?
| }: | ||
| options: | ||
| options: |
There was a problem hiding this comment.
this should not be indented. function bodies stay on the same level.
| { inherit (drv) name system meta; inherit outputs; | ||
| } |
There was a problem hiding this comment.
not sure why } was pushed to the next line
| if isList v | ||
| then noQuotes "[…]" v | ||
| else | ||
| if isAttrs v | ||
| then noQuotes "{…}" v | ||
| else v; |
There was a problem hiding this comment.
the original formatting was more readable to me
this is a bug in the rnix parser that needs to be fixed. It looks like it's not parsing URLs exactly the same way yet and |
OK, so it is a good thing that the single-pass auto-quoting did not happen immediately… |
| in | ||
| if isInt v then toString v | ||
| else | ||
| if isFloat v | ||
| then "~${toString v}" | ||
| else | ||
| if isString v | ||
| then ''"${libStr.escape [ ''"'' ] v}"'' | ||
| else | ||
| if true == v | ||
| then "true" | ||
| else | ||
| if false == v | ||
| then "false" | ||
| else |
There was a problem hiding this comment.
This is where «else if» seems worth the complexity.
| else | ||
| if isFloat v | ||
| then err "floats" v | ||
| else err "this value is" (toString v); |
There was a problem hiding this comment.
Storage space for some cheese 🧀
Motivation for this change
Get some feedback on the nixpkgs-fmt formatting engine. This is the output from the latest version of master.
Things done
Not intended to be merged