Remove `HintFmt::operator%`

Change-Id: Ibcf1a7848b4b18ec9b0807628ff229079ae7a0fe

5 files changed, 113 insertions, 119 deletions

diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc
index e2ada70cd..f7a159829 100644
--- a/src/build-remote/build-remote.cc
+++ b/src/build-remote/build-remote.cc
@@ -185,16 +185,6 @@ static int main_build_remote(int argc, char * * argv)
                         std::cerr << "# postpone\n";
                     else
                     {
-                        // build the hint template.
-                        std::string errorText =
-                            "Failed to find a machine for remote build!\n"
-                            "derivation: %s\nrequired (system, features): (%s, [%s])";
-                        errorText += "\n%s available machines:";
-                        errorText += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)";
-
-                        for (unsigned int i = 0; i < machines.size(); ++i)
-                            errorText += "\n([%s], %s, [%s], [%s])";
-
                         // add the template values.
                         std::string drvstr;
                         if (drvPath.has_value())
@@ -202,19 +192,30 @@ static int main_build_remote(int argc, char * * argv)
                         else
                             drvstr = "<unknown>";
 
-                        auto error = HintFmt::fromFormatString(errorText);
-                        error
-                            % drvstr
-                            % neededSystem
-                            % concatStringsSep<StringSet>(", ", requiredFeatures)
-                            % machines.size();
-
-                        for (auto & m : machines)
-                            error
-                                % concatStringsSep<StringSet>(", ", m.systemTypes)
-                                % m.maxJobs
-                                % concatStringsSep<StringSet>(", ", m.supportedFeatures)
-                                % concatStringsSep<StringSet>(", ", m.mandatoryFeatures);
+                        std::string machinesFormatted;
+
+                        for (auto & m : machines) {
+                            machinesFormatted += HintFmt(
+                                "\n([%s], %s, [%s], [%s])",
+                                concatStringsSep<StringSet>(", ", m.systemTypes),
+                                m.maxJobs,
+                                concatStringsSep<StringSet>(", ", m.supportedFeatures),
+                                concatStringsSep<StringSet>(", ", m.mandatoryFeatures)
+                            ).str();
+                        }
+
+                        auto error = HintFmt(
+                            "Failed to find a machine for remote build!\n"
+                            "derivation: %s\n"
+                            "required (system, features): (%s, [%s])\n"
+                            "%s available machines:\n"
+                            "(systems, maxjobs, supportedFeatures, mandatoryFeatures)%s",
+                            drvstr,
+                            neededSystem,
+                            concatStringsSep<StringSet>(", ", requiredFeatures),
+                            machines.size(),
+                            Uncolored(machinesFormatted)
+                       );
 
                         printMsg(couldBuildLocally ? lvlChatty : lvlWarn, error.str());
 
diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc
index 71ab1edbf..1033c0059 100644
--- a/src/libexpr/print.cc
+++ b/src/libexpr/print.cc
@@ -607,7 +607,7 @@ std::ostream & operator<<(std::ostream & output, const ValuePrinter & printer)
 }
 
 template<>
-HintFmt & HintFmt::operator%(const ValuePrinter & value)
+fmt_internal::HintFmt & fmt_internal::HintFmt::operator%(const ValuePrinter & value)
 {
         fmt % value;
         return *this;
diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh
index 7ddda81b8..94cb11ca7 100644
--- a/src/libexpr/print.hh
+++ b/src/libexpr/print.hh
@@ -86,6 +86,6 @@ std::ostream & operator<<(std::ostream & output, const ValuePrinter & printer);
  * magenta.
  */
 template<>
-HintFmt & HintFmt::operator%(const ValuePrinter & value);
+fmt_internal::HintFmt & fmt_internal::HintFmt::operator%(const ValuePrinter & value);
 
 }
diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh
index 84a3e3e11..c68591b72 100644
--- a/src/libutil/fmt.hh
+++ b/src/libutil/fmt.hh
@@ -5,43 +5,94 @@
 #include <string>
 #include "ansicolor.hh"
 
-
 namespace nix {
 
-namespace {
 /**
- * A helper for writing `boost::format` expressions.
- *
- * These are equivalent:
+ * Values wrapped in this struct are printed in magenta.
  *
- * ```
- * formatHelper(formatter, a_0, ..., a_n)
- * formatter % a_0 % ... % a_n
- * ```
+ * By default, arguments to `HintFmt` are printed in magenta. To avoid this,
+ * either wrap the argument in `Uncolored` or add a specialization of
+ * `HintFmt::operator%`.
+ */
+template<class T>
+struct Magenta
+{
+    Magenta(const T & s) : value(s) {}
+    const T & value;
+};
+
+template<class T>
+std::ostream & operator<<(std::ostream & out, const Magenta<T> & y)
+{
+    return out << ANSI_MAGENTA << y.value << ANSI_NORMAL;
+}
+
+/**
+ * Values wrapped in this class are printed without coloring.
  *
- * With a single argument, `formatHelper(s)` is a no-op.
+ * By default, arguments to `HintFmt` are printed in magenta (see `Magenta`).
  */
-template<class F>
-inline void formatHelper(F & f)
-{ }
+template<class T>
+struct Uncolored
+{
+    Uncolored(const T & s) : value(s) {}
+    const T & value;
+};
 
-template<class F, typename T, typename... Args>
-inline void formatHelper(F & f, const T & x, const Args & ... args)
+template<class T>
+std::ostream & operator<<(std::ostream & out, const Uncolored<T> & y)
 {
-    // Interpolate one argument and then recurse.
-    formatHelper(f % x, args...);
+    return out << ANSI_NORMAL << y.value;
 }
 
+namespace fmt_internal {
+
 /**
  * Set the correct exceptions for `fmt`.
  */
-void setExceptions(boost::format & fmt)
+inline void setExceptions(boost::format & fmt)
 {
     fmt.exceptions(
-        boost::io::all_error_bits ^
-        boost::io::too_many_args_bit ^
-        boost::io::too_few_args_bit);
+        boost::io::all_error_bits ^ boost::io::too_many_args_bit ^ boost::io::too_few_args_bit
+    );
 }
+
+/**
+ * Helper class for `HintFmt` that supports the evil `operator%`.
+ *
+ * See: https://git.lix.systems/lix-project/lix/issues/178
+ */
+struct HintFmt
+{
+    boost::format fmt;
+
+    template<typename... Args>
+    HintFmt(boost::format && fmt, const Args &... args) : fmt(std::move(fmt))
+    {
+        setExceptions(fmt);
+        (*this % ... % args);
+    }
+
+    template<class T>
+    HintFmt & operator%(const T & value)
+    {
+        fmt % Magenta(value);
+        return *this;
+    }
+
+    template<class T>
+    HintFmt & operator%(const Uncolored<T> & value)
+    {
+        fmt % value.value;
+        return *this;
+    }
+
+    boost::format into_format()
+    {
+        return std::move(fmt);
+    }
+};
+
 }
 
 /**
@@ -77,53 +128,15 @@ inline std::string fmt(const char * s)
 }
 
 template<typename... Args>
-inline std::string fmt(const std::string & fs, const Args & ... args)
+inline std::string fmt(const std::string & fs, const Args &... args)
 {
     boost::format f(fs);
-    setExceptions(f);
-    formatHelper(f, args...);
+    fmt_internal::setExceptions(f);
+    (f % ... % args);
     return f.str();
 }
 
 /**
- * Values wrapped in this struct are printed in magenta.
- *
- * By default, arguments to `HintFmt` are printed in magenta. To avoid this,
- * either wrap the argument in `Uncolored` or add a specialization of
- * `HintFmt::operator%`.
- */
-template <class T>
-struct Magenta
-{
-    Magenta(const T &s) : value(s) {}
-    const T & value;
-};
-
-template <class T>
-std::ostream & operator<<(std::ostream & out, const Magenta<T> & y)
-{
-    return out << ANSI_WARNING << y.value << ANSI_NORMAL;
-}
-
-/**
- * Values wrapped in this class are printed without coloring.
- *
- * By default, arguments to `HintFmt` are printed in magenta (see `Magenta`).
- */
-template <class T>
-struct Uncolored
-{
-    Uncolored(const T & s) : value(s) {}
-    const T & value;
-};
-
-template <class T>
-std::ostream & operator<<(std::ostream & out, const Uncolored<T> & y)
-{
-    return out << ANSI_NORMAL << y.value;
-}
-
-/**
  * A wrapper around `boost::format` which colors interpolated arguments in
  * magenta by default.
  */
@@ -137,46 +150,28 @@ public:
      * Format the given string literally, without interpolating format
      * placeholders.
      */
-    HintFmt(const std::string & literal)
-        : HintFmt("%s", Uncolored(literal))
-    { }
-
-    static HintFmt fromFormatString(const std::string & format) {
-        return HintFmt(boost::format(format));
-    }
+    HintFmt(const std::string & literal) : HintFmt("%s", Uncolored(literal)) {}
 
     /**
      * Interpolate the given arguments into the format string.
      */
     template<typename... Args>
-    HintFmt(const std::string & format, const Args & ... args)
+    HintFmt(const std::string & format, const Args &... args)
         : HintFmt(boost::format(format), args...)
-    { }
-
-    HintFmt(const HintFmt & hf)
-        : fmt(hf.fmt)
-    { }
-
-    template<typename... Args>
-    HintFmt(boost::format && fmt, const Args & ... args)
-        : fmt(std::move(fmt))
     {
-        setExceptions(fmt);
-        formatHelper(*this, args...);
     }
 
-    template<class T>
-    HintFmt & operator%(const T & value)
-    {
-        fmt % Magenta(value);
-        return *this;
-    }
+    HintFmt(const HintFmt & hf) : fmt(hf.fmt) {}
 
-    template<class T>
-    HintFmt & operator%(const Uncolored<T> & value)
+    template<typename... Args>
+    HintFmt(boost::format && fmt, const Args &... args)
+        : fmt(fmt_internal::HintFmt(std::move(fmt), args...).into_format())
     {
-        fmt % value.value;
-        return *this;
+        if (this->fmt.remaining_args() != 0) {
+            throw boost::io::too_few_args(
+                this->fmt.bound_args() + this->fmt.fed_args(), this->fmt.expected_args()
+            );
+        }
     }
 
     std::string str() const
diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh
index 7a6341d70..298d66060 100644
--- a/src/libutil/logging.hh
+++ b/src/libutil/logging.hh
@@ -242,9 +242,7 @@ extern Verbosity verbosity;
 template<typename... Args>
 inline void warn(const std::string & fs, const Args & ... args)
 {
-    boost::format f(fs);
-    formatHelper(f, args...);
-    logger->warn(f.str());
+    logger->warn(HintFmt(fs, args...).str());
 }
 
 #define warnOnce(haveWarned, args...) \