From 81a0624d7613dbd5a4fc69cde95314e7db47bd43 Mon Sep 17 00:00:00 2001
From: Alois Wohlschlager <alois1@gmx-topmail.de>
Date: Mon, 15 Jul 2024 18:00:29 +0200
Subject: [PATCH] libexpr/print: pretty-print idempotently
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When pretty-printing is enabled, previously an unforced thunk would trigger
indentation, even when it subsequently does not evaluate to a nested structure.
The resulting output looked inconsistent, and furthermore pretty-printing was
not idempotent (since pretty-printing the same value again, which is now fully
evaluated, will not trigger indentation).
When strict evaluation is enabled, force the item before inspecting its type,
so that it is properly known whether it contains a nested structure.
Furthermore, there is no need to cause indentation for unforced thunks, since
the very next operation will be printing them as `«thunk»`.

This is mostly a port of https://github.com/NixOS/nix/pull/11100 , but we only
force the item when it's going to be forced anyway due to strict
pretty-printing, and a new test was written since the REPL testing framework in
Lix is different.

Co-Authored-By: Robert Hensing <robert@roberthensing.nl>
Change-Id: Ib7560fe531d09e05ca6b2037a523fe21a26d9d58
---
 src/libexpr/print.cc                               | 14 ++++++++++++--
 .../repl_characterization/data/idempotent.test     | 13 +++++++++++++
 .../repl_characterization/repl_characterization.cc |  1 +
 3 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 tests/functional/repl_characterization/data/idempotent.test

diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc
index 87db004b2..6eb321c43 100644
--- a/src/libexpr/print.cc
+++ b/src/libexpr/print.cc
@@ -264,10 +264,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 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)
@@ -335,10 +340,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)
diff --git a/tests/functional/repl_characterization/data/idempotent.test b/tests/functional/repl_characterization/data/idempotent.test
new file mode 100644
index 000000000..4ab087d45
--- /dev/null
+++ b/tests/functional/repl_characterization/data/idempotent.test
@@ -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 ]
+  ]
diff --git a/tests/functional/repl_characterization/repl_characterization.cc b/tests/functional/repl_characterization/repl_characterization.cc
index c91d6c1e3..f2cf0ca3d 100644
--- a/tests/functional/repl_characterization/repl_characterization.cc
+++ b/tests/functional/repl_characterization/repl_characterization.cc
@@ -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