lix-project/lix
20
55
Fork 31
Code Issues 344 Code Review (Gerrit) Projects 3 Releases Wiki Activity

Merge changes I829581a3,I0016970d,I5dac8e77,Ib7560fe5 into main

Browse source 
* changes:
  doc/release-notes: add for pretty printing improvements
  libexpr/print: do not show elided nested items when there are none
  libexpr/print: never show empty attrsets or derivations as «repeated»
  libexpr/print: pretty-print idempotently
This commit is contained in:
alois31 2024-07-19 06:40:13 +00:00 committed by Gerrit Code Review
parent 5ee1e6ea98 768d1f29a2
commit aba5f19680
5 changed files with 132 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

58
doc/manual/rl-next/pretty-printing.md Normal file
View file

@ -0,0 +1,58 @@
---
synopsis: "Eliminate some pretty-printing surprises"
cls: [1616, 1617, 1618]
prs: [11100]
credits: [alois31, roberth]
category: Improvements
---
Some inconsistent and surprising behaviours have been eliminated from the pretty-printing used by the REPL and `nix eval`:
* Lists and attribute sets that contain only a single item without nested structures are no longer sometimes inappropriately indented in the REPL, depending on internal state of the evaluator.
* Empty attribute sets and derivations are no longer shown as `«repeated»`, since they are always cheap to print.
This matches the existing behaviour of `nix-instantiate` on empty attribute sets.
Empty lists were never printed as `«repeated»` already.
* The REPL by default does not print nested attribute sets and lists, and indicates elided items with an ellipsis.
Previously, the ellipsis was printed even when the structure was empty, so that such items do not in fact exist.
Since this behaviour was confusing, it does not happen any more.
Before:
```
nix-repl> :p let x = 1 + 2; in [ [ x ] [ x ] ]
[
[
3
]
[ 3 ]
]
nix-repl> let inherit (import <nixpkgs> { }) hello; in [ hello hello ]
[
«derivation /nix/store/fqs92lzychkm6p37j7fnj4d65nq9fzla-hello-2.12.1.drv»
«repeated»
]
nix-repl> let x = {}; in [ x ]
[
{ ... }
]
```
After:
```
nix-repl> :p let x = 1 + 2; in [ [ x ] [ x ] ]
[
[ 3 ]
[ 3 ]
]
nix-repl> let inherit (import <nixpkgs> { }) hello; in [ hello hello ]
[
«derivation /nix/store/fqs92lzychkm6p37j7fnj4d65nq9fzla-hello-2.12.1.drv»
«derivation /nix/store/fqs92lzychkm6p37j7fnj4d65nq9fzla-hello-2.12.1.drv»
]
nix-repl> let x = {}; in [ x ]
[
{ }
]
```

25
src/libexpr/print.cc
View file

@ -264,22 +264,24 @@ private:
return true;
}
if (options.force) {
// The item is going to be forced during printing anyway, but we need its type now.
state.forceValue(*item, item->determinePos(noPos));
}
// Pretty-print single-item attrsets only if they contain nested
// structures.
auto itemType = item->type();
return itemType == nList || itemType == nAttrs || itemType == nThunk;
return itemType == nList || itemType == nAttrs;
}
void printAttrs(Value & v, size_t depth)
{
if (seen && !seen->insert(v.attrs).second) {
printRepeated();
return;
}
if (options.force && options.derivationPaths && state.isDerivation(v)) {
printDerivation(v);
} else if (depth < options.maxDepth) {
} else if (seen && !v.attrs->empty() && !seen->insert(v.attrs).second) {
printRepeated();
} else if (depth < options.maxDepth || v.attrs->empty()) {
increaseIndent();
output << "{";
@ -335,10 +337,15 @@ private:
return true;
}
if (options.force) {
// The item is going to be forced during printing anyway, but we need its type now.
state.forceValue(*item, item->determinePos(noPos));
}
// Pretty-print single-item lists only if they contain nested
// structures.
auto itemType = item->type();
return itemType == nList || itemType == nAttrs || itemType == nThunk;
return itemType == nList || itemType == nAttrs;
}
void printList(Value & v, size_t depth)
@ -348,7 +355,7 @@ private:
return;
}
if (depth < options.maxDepth) {
if (depth < options.maxDepth || v.listSize() == 0) {
increaseIndent();
output << "[";
auto listItems = v.listItems();

13
tests/functional/repl_characterization/data/idempotent.test Normal file
View file

@ -0,0 +1,13 @@
A previously unforced thunk in an attribute set does not lead to indentation when it won't evaluate to a nested structure:
nix-repl> :p let x = 1 + 2; in [ { inherit x; } { inherit x; } ]
[
{ x = 3; }
{ x = 3; }
]
Same for a list:
nix-repl> :p let x = 1 + 2; in [ [ x ] [ x ] ]
[
[ 3 ]
[ 3 ]
]

1
tests/functional/repl_characterization/repl_characterization.cc
View file

@ -185,5 +185,6 @@ REPL_TEST(repl_overlays_error);
REPL_TEST(repl_printing);
REPL_TEST(stack_vars);
REPL_TEST(errors);
REPL_TEST(idempotent);
}; // namespace nix

67
tests/unit/libexpr/value/print.cc
View file

@ -193,6 +193,9 @@ TEST_F(ValuePrintingTests, vBlackhole)
TEST_F(ValuePrintingTests, depthAttrs)
{
Value vZero;
vZero.mkInt(0);
Value vOne;
vOne.mkInt(1);
@ -203,10 +206,16 @@ TEST_F(ValuePrintingTests, depthAttrs)
Value vAttrsEmpty;
vAttrsEmpty.mkAttrs(builderEmpty.finish());
BindingsBuilder builderNested(state, state.allocBindings(1));
builderNested.insert(state.symbols.create("zero"), &vZero);
Value vAttrsNested;
vAttrsNested.mkAttrs(builderNested.finish());
BindingsBuilder builder(state, state.allocBindings(10));
builder.insert(state.symbols.create("one"), &vOne);
builder.insert(state.symbols.create("two"), &vTwo);
builder.insert(state.symbols.create("nested"), &vAttrsEmpty);
builder.insert(state.symbols.create("empty"), &vAttrsEmpty);
builder.insert(state.symbols.create("nested"), &vAttrsNested);
Value vAttrs;
vAttrs.mkAttrs(builder.finish());
@ -220,9 +229,9 @@ TEST_F(ValuePrintingTests, depthAttrs)
vNested.mkAttrs(builder2.finish());
test(vNested, "{ nested = { ... }; one = 1; two = 2; }", PrintOptions { .maxDepth = 1 });
test(vNested, "{ nested = { nested = { ... }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 2 });
test(vNested, "{ nested = { nested = { }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 3 });
test(vNested, "{ nested = { nested = { }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 4 });
test(vNested, "{ nested = { empty = { }; nested = { ... }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 2 });
test(vNested, "{ nested = { empty = { }; nested = { zero = 0; }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 3 });
test(vNested, "{ nested = { empty = { }; nested = { zero = 0; }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 4 });
}
TEST_F(ValuePrintingTests, depthList)
@ -641,20 +650,24 @@ TEST_F(ValuePrintingTests, ansiColorsBlackhole)
TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated)
{
BindingsBuilder emptyBuilder(state, state.allocBindings(1));
Value vZero;
vZero.mkInt(0);
Value vEmpty;
vEmpty.mkAttrs(emptyBuilder.finish());
BindingsBuilder innerBuilder(state, state.allocBindings(1));
innerBuilder.insert(state.symbols.create("x"), &vZero);
Value vInner;
vInner.mkAttrs(innerBuilder.finish());
BindingsBuilder builder(state, state.allocBindings(10));
builder.insert(state.symbols.create("a"), &vEmpty);
builder.insert(state.symbols.create("b"), &vEmpty);
builder.insert(state.symbols.create("a"), &vInner);
builder.insert(state.symbols.create("b"), &vInner);
Value vAttrs;
vAttrs.mkAttrs(builder.finish());
test(vAttrs,
"{ a = { }; b = " ANSI_MAGENTA "«repeated»" ANSI_NORMAL "; }",
"{ a = { x = " ANSI_CYAN "0" ANSI_NORMAL "; }; b = " ANSI_MAGENTA "«repeated»" ANSI_NORMAL "; }",
PrintOptions {
.ansiColors = true
});
@ -662,19 +675,23 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated)
TEST_F(ValuePrintingTests, ansiColorsListRepeated)
{
BindingsBuilder emptyBuilder(state, state.allocBindings(1));
Value vZero;
vZero.mkInt(0);
Value vEmpty;
vEmpty.mkAttrs(emptyBuilder.finish());
BindingsBuilder innerBuilder(state, state.allocBindings(1));
innerBuilder.insert(state.symbols.create("x"), &vZero);
Value vInner;
vInner.mkAttrs(innerBuilder.finish());
Value vList;
state.mkList(vList, 3);
vList.bigList.elems[0] = &vEmpty;
vList.bigList.elems[1] = &vEmpty;
vList.bigList.elems[0] = &vInner;
vList.bigList.elems[1] = &vInner;
vList.bigList.size = 2;
test(vList,
"[ { } " ANSI_MAGENTA "«repeated»" ANSI_NORMAL " ]",
"[ { x = " ANSI_CYAN "0" ANSI_NORMAL "; } " ANSI_MAGENTA "«repeated»" ANSI_NORMAL " ]",
PrintOptions {
.ansiColors = true
});
@ -682,20 +699,24 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated)
TEST_F(ValuePrintingTests, listRepeated)
{
BindingsBuilder emptyBuilder(state, state.allocBindings(1));
Value vZero;
vZero.mkInt(0);
Value vEmpty;
vEmpty.mkAttrs(emptyBuilder.finish());
BindingsBuilder innerBuilder(state, state.allocBindings(1));
innerBuilder.insert(state.symbols.create("x"), &vZero);
Value vInner;
vInner.mkAttrs(innerBuilder.finish());
Value vList;
state.mkList(vList, 3);
vList.bigList.elems[0] = &vEmpty;
vList.bigList.elems[1] = &vEmpty;
vList.bigList.elems[0] = &vInner;
vList.bigList.elems[1] = &vInner;
vList.bigList.size = 2;
test(vList, "[ { } «repeated» ]", PrintOptions { });
test(vList, "[ { x = 0; } «repeated» ]", PrintOptions { });
test(vList,
"[ { } { } ]",
"[ { x = 0; } { x = 0; } ]",
PrintOptions {
.trackRepeated = false
});