From b6125772d7d5f82d48873fc93a7f261832154b14 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 16 Apr 2023 14:07:35 +0200
Subject: [PATCH] libexpr: Move identifier-like printing to print.cc

---
 src/libcmd/repl.cc     |  6 ++---
 src/libexpr/nixexpr.cc | 27 +++--------------------
 src/libexpr/print.cc   | 50 ++++++++++++++++++++++++++++++++++++++++++
 src/libexpr/print.hh   | 18 +++++++++++++++
 4 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc
index 806dce024..80c08bf1c 100644
--- a/src/libcmd/repl.cc
+++ b/src/libcmd/repl.cc
@@ -426,6 +426,7 @@ StringSet NixRepl::completePrefix(const std::string & prefix)
 }
 
 
+// FIXME: DRY and match or use the parser
 static bool isVarName(std::string_view s)
 {
     if (s.size() == 0) return false;
@@ -956,10 +957,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
                 sorted.emplace(state->symbols[i.name], i.value);
 
             for (auto & i : sorted) {
-                if (isVarName(i.first))
-                    str << i.first;
-                else
-                    printLiteralString(str, i.first);
+                printAttributeName(str, i.first);
                 str << " = ";
                 if (seen.count(i.second))
                     str << "«repeated»";
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index d8f3cd701..1557cbbeb 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -61,33 +61,12 @@ Pos::operator std::shared_ptr<AbstractPos>() const
     return pos;
 }
 
-/* Displaying abstract syntax trees. */
-
+// FIXME: remove, because *symbols* are abstract and do not have a single
+//        textual representation; see printIdentifier()
 std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol)
 {
     std::string_view s = symbol;
-
-    if (s.empty())
-        str << "\"\"";
-    else if (s == "if") // FIXME: handle other keywords
-        str << '"' << s << '"';
-    else {
-        char c = s[0];
-        if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_')) {
-            printLiteralString(str, s);
-            return str;
-        }
-        for (auto c : s)
-            if (!((c >= 'a' && c <= 'z') ||
-                  (c >= 'A' && c <= 'Z') ||
-                  (c >= '0' && c <= '9') ||
-                  c == '_' || c == '\'' || c == '-')) {
-                printLiteralString(str, s);
-                return str;
-            }
-        str << s;
-    }
-    return str;
+    return printIdentifier(str, s);
 }
 
 void Expr::show(const SymbolTable & symbols, std::ostream & str) const
diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc
index 282903b72..d08672cfc 100644
--- a/src/libexpr/print.cc
+++ b/src/libexpr/print.cc
@@ -25,4 +25,54 @@ printLiteralBool(std::ostream & str, bool boolean)
     return str;
 }
 
+std::ostream &
+printIdentifier(std::ostream & str, std::string_view s) {
+    if (s.empty())
+        str << "\"\"";
+    else if (s == "if") // FIXME: handle other keywords
+        str << '"' << s << '"';
+    else {
+        char c = s[0];
+        if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_')) {
+            printLiteralString(str, s);
+            return str;
+        }
+        for (auto c : s)
+            if (!((c >= 'a' && c <= 'z') ||
+                  (c >= 'A' && c <= 'Z') ||
+                  (c >= '0' && c <= '9') ||
+                  c == '_' || c == '\'' || c == '-')) {
+                printLiteralString(str, s);
+                return str;
+            }
+        str << s;
+    }
+    return str;
+}
+
+// FIXME: keywords
+static bool isVarName(std::string_view s)
+{
+    if (s.size() == 0) return false;
+    char c = s[0];
+    if ((c >= '0' && c <= '9') || c == '-' || c == '\'') return false;
+    for (auto & i : s)
+        if (!((i >= 'a' && i <= 'z') ||
+              (i >= 'A' && i <= 'Z') ||
+              (i >= '0' && i <= '9') ||
+              i == '_' || i == '-' || i == '\''))
+            return false;
+    return true;
+}
+
+std::ostream &
+printAttributeName(std::ostream & str, std::string_view name) {
+    if (isVarName(name))
+        str << name;
+    else
+        printLiteralString(str, name);
+    return str;
+}
+
+
 }
diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh
index 98dd2008d..f9cfc3964 100644
--- a/src/libexpr/print.hh
+++ b/src/libexpr/print.hh
@@ -27,4 +27,22 @@ namespace nix {
 
     /** Print `true` or `false`. */
     std::ostream & printLiteralBool(std::ostream & o, bool b);
+
+    /**
+     * Print a string as an attribute name in the Nix expression language syntax.
+     *
+     * Prints a quoted string if necessary.
+     */
+    std::ostream & printAttributeName(std::ostream & o, std::string_view s);
+
+    /**
+     * Print a string as an identifier in the Nix expression language syntax.
+     *
+     * FIXME: "identifier" is ambiguous. Identifiers do not have a single
+     *        textual representation. They can be used in variable references,
+     *        let bindings, left-hand sides or attribute names in a select
+     *        expression, or something else entirely, like JSON. Use one of the
+     *        `print*` functions instead.
+     */
+    std::ostream & printIdentifier(std::ostream & o, std::string_view s);
 }