From 4e0804c920558575a4b3486df1e595445bf67555 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 9 Apr 2023 22:42:20 +0200 Subject: [PATCH] Deduplicate string literal rendering, fix 4909 --- src/libcmd/repl.cc | 20 ++++++-------------- src/libexpr/eval.cc | 13 +++---------- src/libexpr/nixexpr.cc | 19 ++++--------------- src/libexpr/value/print.cc | 26 ++++++++++++++++++++++++++ src/libexpr/value/print.hh | 30 ++++++++++++++++++++++++++++++ src/libstore/derivations.cc | 9 +++++++++ tests/repl.sh | 8 ++++++++ 7 files changed, 86 insertions(+), 39 deletions(-) create mode 100644 src/libexpr/value/print.cc create mode 100644 src/libexpr/value/print.hh diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 57848a5d3..1366622c7 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -40,6 +40,7 @@ extern "C" { #include "markdown.hh" #include "local-fs-store.hh" #include "progress-bar.hh" +#include "value/print.hh" #if HAVE_BOEHMGC #define GC_INCLUDE_NEW @@ -894,17 +895,6 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m } -std::ostream & printStringValue(std::ostream & str, const char * string) { - str << "\""; - for (const char * i = string; *i; i++) - if (*i == '\"' || *i == '\\') str << "\\" << *i; - else if (*i == '\n') str << "\\n"; - else if (*i == '\r') str << "\\r"; - else if (*i == '\t') str << "\\t"; - else str << *i; - str << "\""; - return str; -} // FIXME: lot of cut&paste from Nix's eval.cc. @@ -922,12 +912,14 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m break; case nBool: - str << ANSI_CYAN << (v.boolean ? "true" : "false") << ANSI_NORMAL; + str << ANSI_CYAN; + printLiteral(str, v.boolean); + str << ANSI_NORMAL; break; case nString: str << ANSI_WARNING; - printStringValue(str, v.string.s); + printLiteral(str, v.string.s); str << ANSI_NORMAL; break; @@ -967,7 +959,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m if (isVarName(i.first)) str << i.first; else - printStringValue(str, i.first.c_str()); + printLiteral(str, i.first); str << " = "; if (seen.count(i.second)) str << "«repeated»"; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 18cfd9531..06208897f 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -9,6 +9,7 @@ #include "filetransfer.hh" #include "function-trace.hh" #include "profiles.hh" +#include "value/print.hh" #include #include @@ -104,18 +105,10 @@ void Value::print(const SymbolTable & symbols, std::ostream & str, str << integer; break; case tBool: - str << (boolean ? "true" : "false"); + printLiteral(str, boolean); break; case tString: - str << "\""; - for (const char * i = string.s; *i; i++) - if (*i == '\"' || *i == '\\') str << "\\" << *i; - else if (*i == '\n') str << "\\n"; - else if (*i == '\r') str << "\\r"; - else if (*i == '\t') str << "\\t"; - else if (*i == '$' && *(i+1) == '{') str << "\\" << *i; - else str << *i; - str << "\""; + printLiteral(str, string.s); break; case tPath: str << path; // !!! escaping? diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index eb6f062b4..ca6df0af3 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -3,6 +3,7 @@ #include "eval.hh" #include "symbol-table.hh" #include "util.hh" +#include "value/print.hh" #include @@ -62,18 +63,6 @@ Pos::operator std::shared_ptr() const /* Displaying abstract syntax trees. */ -static void showString(std::ostream & str, std::string_view s) -{ - str << '"'; - for (auto c : s) - if (c == '"' || c == '\\' || c == '$') str << "\\" << c; - else if (c == '\n') str << "\\n"; - else if (c == '\r') str << "\\r"; - else if (c == '\t') str << "\\t"; - else str << c; - str << '"'; -} - std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol) { std::string_view s = symbol; @@ -85,7 +74,7 @@ std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol) else { char c = s[0]; if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_')) { - showString(str, s); + printLiteral(str, s); return str; } for (auto c : s) @@ -93,7 +82,7 @@ std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol) (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_' || c == '\'' || c == '-')) { - showString(str, s); + printLiteral(str, s); return str; } str << s; @@ -118,7 +107,7 @@ void ExprFloat::show(const SymbolTable & symbols, std::ostream & str) const void ExprString::show(const SymbolTable & symbols, std::ostream & str) const { - showString(str, s); + printLiteral(str, s); } void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const diff --git a/src/libexpr/value/print.cc b/src/libexpr/value/print.cc new file mode 100644 index 000000000..cf97def46 --- /dev/null +++ b/src/libexpr/value/print.cc @@ -0,0 +1,26 @@ +#include "value/print.hh" + +namespace nix { + +std::ostream & +printLiteral(std::ostream & str, const std::string_view string) { + str << "\""; + for (auto i = string.begin(); i != string.end(); ++i) { + if (*i == '\"' || *i == '\\') str << "\\" << *i; + else if (*i == '\n') str << "\\n"; + else if (*i == '\r') str << "\\r"; + else if (*i == '\t') str << "\\t"; + else if (*i == '$' && *(i+1) == '{') str << "\\" << *i; + else str << *i; + } + str << "\""; + return str; +} + +std::ostream & +printLiteral(std::ostream & str, bool boolean) { + str << (boolean ? "true" : "false"); + return str; +} + +} diff --git a/src/libexpr/value/print.hh b/src/libexpr/value/print.hh new file mode 100644 index 000000000..31c94eb85 --- /dev/null +++ b/src/libexpr/value/print.hh @@ -0,0 +1,30 @@ +#pragma once +/** + * @file + * @brief Common printing functions for the Nix language + * + * While most types come with their own methods for printing, they share some + * functions that are placed here. + */ + +#include + +namespace nix { + /** + * Print a string as a Nix string literal. + * + * Quotes and fairly minimal escaping are added. + * + * @param s The logical string + */ + std::ostream & printLiteral(std::ostream & o, std::string_view s); + inline std::ostream & printLiteral(std::ostream & o, const char * s) { + return printLiteral(o, std::string_view(s)); + } + inline std::ostream & printLiteral(std::ostream & o, const std::string & s) { + return printLiteral(o, std::string_view(s)); + } + + /** Print `true` or `false`. */ + std::ostream & printLiteral(std::ostream & o, bool b); +} diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index abdfb1978..9948862e5 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -313,6 +313,15 @@ Derivation parseDerivation(const Store & store, std::string && s, std::string_vi } +/** + * Print a derivation string literal to an std::string. + * + * This syntax does not generalize to the expression language, which needs to + * escape `$`. + * + * @param res Where to print to + * @param s Which logical string to print + */ static void printString(std::string & res, std::string_view s) { boost::container::small_vector buffer; diff --git a/tests/repl.sh b/tests/repl.sh index be8adb742..2b3789521 100644 --- a/tests/repl.sh +++ b/tests/repl.sh @@ -79,6 +79,14 @@ testReplResponse ' "result: ${a}" ' "result: 2" +# check dollar escaping https://github.com/NixOS/nix/issues/4909 +# note the escaped \, +# \\ +# because the second argument is a regex +testReplResponse ' +"$" + "{hi}" +' '"\\${hi}"' + testReplResponse ' drvPath ' '".*-simple.drv"' \