From b5da823138023957d91eb013c2a6350ef6032a80 Mon Sep 17 00:00:00 2001
From: Alois Wohlschlager <alois1@gmx-topmail.de>
Date: Mon, 15 Jul 2024 18:24:16 +0200
Subject: [PATCH] =?UTF-8?q?libexpr/print:=20never=20show=20empty=20attrset?=
 =?UTF-8?q?s=20or=20derivations=20as=20=C2=ABrepeated=C2=BB?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The repeated value detection logic exists so that the occurrence of large
common substructures does not fill up the screen or the computer's memory.
However, empty attribute sets and derivations (when their detection is enabled)
are always cheap to print, and in practice I have observed them to make up a
significant majority of the cases where I was annoyed by the repeated value
detection kicking in. Furthermore, `nix-instantiate --eval` already disables
this logic for empty attribute sets, and empty lists are already exempted
everywhere. For these reasons, always print empty attribute sets and
derivations as what they are.

Change-Id: I5dac8e7739f9d726b76fd0521ec46f38af94463f
---
 src/libexpr/print.cc              |  7 ++---
 tests/unit/libexpr/value/print.cc | 50 +++++++++++++++++++------------
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc
index 6eb321c43..2ee11d9d3 100644
--- a/src/libexpr/print.cc
+++ b/src/libexpr/print.cc
@@ -277,13 +277,10 @@ private:
 
     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 (seen && !v.attrs->empty() && !seen->insert(v.attrs).second) {
+            printRepeated();
         } else if (depth < options.maxDepth) {
             increaseIndent();
             output << "{";
diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc
index cdbc8f8f9..1f99b562d 100644
--- a/tests/unit/libexpr/value/print.cc
+++ b/tests/unit/libexpr/value/print.cc
@@ -641,20 +641,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 +666,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 +690,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
          });