New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve generators.toPretty
#97133
Improve generators.toPretty
#97133
Conversation
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 love this, I’ve been wanting to have multiline toPretty in forever.
Two additional things:
- I’d leave the
if/then/else
form instead of going vialtypeOf
, since it’s more straightforward and easier to adjust. - I’d render lists and attrsets that only have one element on one line to be slightly more compact. From 2 elements, split onto newlines. Though that might be easier said than done …
7657cf4
to
3d014c5
Compare
As a preparation to the following commit
- These symbols can be confusing for those not familiar with them - There's no harm in making these more obvious - Terminals may not print them correctly either Also changes the function argument printing slightly to be more obvious
Not attribute sets. So move the function case forward
9335702
to
15c5ba9
Compare
Did some changes:
I decided to not render single-element lists/attrs without newlines, because I fear this makes it more complicated to understand the output, as the indentation can't be used to see the nesting level anymore. Here's a sample output for this Nix expression: {
value = {
fun = lib.id;
funargs = lib.types.submoduleWith;
multiline = {
multi = ''
hello
there!
'';
multi' = ''
hello
there!'';
single = "hello there!";
};
pkg = pkgs.hello;
single = { foo = [ { bar = [ null ]; } ]; };
} Which renders as
|
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.
Looks very good to me 👍
Haven’t reviewed the logic in-depth, but we have tests so it’s fine. LGTM
I suppose eventually we'd want this to be a builtin function in Nix, since it's needed in the UI anyway, and it's easier on the C++ side to handle circular values etc. |
Motivation for this change
For big Nix expressions,
toPretty
can be very hard to read, as it doesn't output any newlines. This PR implements themultiline
option fortoPretty
, enabled by default, which makes it output newlines with proper indentation. Example output:In addition, I made some other improvements:
escapeNixIdentifier
, introduced in f9eb3d1Ping @Profpatsch @rycee
Best reviewed commit-by-commit. Specifically there's one commit in there that just refactors the function without changing its behavior.
Things done