From d2ce2e89b178e7e7467cf4c1e572704a4c2ca75e Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Fri, 19 May 2023 10:56:59 -0400
Subject: Split `OptionalPathSetting` from `PathSetting`

Rather than doing `allowEmpty` as boolean, have separate types and use
`std::optional`. This makes it harder to forget the possibility of an
empty path.

The `build-hook` setting was categorized as a `PathSetting`, but
actually it was split into arguments. No good! Now, it is
`Setting<Strings>` which actually reflects what it means and how it is
used.

Because of the subtyping, we now also have support for
`Setting<std::optional<String>>` in general. I imagine this can be used
to clean up many more settings also.
---
 src/libutil/config.cc | 63 +++++++++++++++++++--------------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

(limited to 'src/libutil/config.cc')

diff --git a/src/libutil/config.cc b/src/libutil/config.cc
index 085a884dc..38d406e8a 100644
--- a/src/libutil/config.cc
+++ b/src/libutil/config.cc
@@ -219,29 +219,6 @@ void AbstractSetting::convertToArg(Args & args, const std::string & category)
 {
 }
 
-template<typename T>
-void BaseSetting<T>::convertToArg(Args & args, const std::string & category)
-{
-    args.addFlag({
-        .longName = name,
-        .description = fmt("Set the `%s` setting.", name),
-        .category = category,
-        .labels = {"value"},
-        .handler = {[this](std::string s) { overridden = true; set(s); }},
-        .experimentalFeature = experimentalFeature,
-    });
-
-    if (isAppendable())
-        args.addFlag({
-            .longName = "extra-" + name,
-            .description = fmt("Append to the `%s` setting.", name),
-            .category = category,
-            .labels = {"value"},
-            .handler = {[this](std::string s) { overridden = true; set(s, true); }},
-            .experimentalFeature = experimentalFeature,
-        });
-}
-
 template<> std::string BaseSetting<std::string>::parse(const std::string & str) const
 {
     return str;
@@ -252,21 +229,17 @@ template<> std::string BaseSetting<std::string>::to_string() const
     return value;
 }
 
-template<typename T>
-T BaseSetting<T>::parse(const std::string & str) const
+template<> std::optional<std::string> BaseSetting<std::optional<std::string>>::parse(const std::string & str) const
 {
-    static_assert(std::is_integral<T>::value, "Integer required.");
-    if (auto n = string2Int<T>(str))
-        return *n;
+    if (str == "")
+        return std::nullopt;
     else
-        throw UsageError("setting '%s' has invalid value '%s'", name, str);
+        return { str };
 }
 
-template<typename T>
-std::string BaseSetting<T>::to_string() const
+template<> std::string BaseSetting<std::optional<std::string>>::to_string() const
 {
-    static_assert(std::is_integral<T>::value, "Integer required.");
-    return std::to_string(value);
+    return value ? *value : "";
 }
 
 template<> bool BaseSetting<bool>::parse(const std::string & str) const
@@ -403,17 +376,27 @@ template class BaseSetting<StringSet>;
 template class BaseSetting<StringMap>;
 template class BaseSetting<std::set<ExperimentalFeature>>;
 
-Path PathSetting::parse(const std::string & str) const
+static Path parsePath(const AbstractSetting & s, const std::string & str)
 {
-    if (str == "") {
-        if (allowEmpty)
-            return "";
-        else
-            throw UsageError("setting '%s' cannot be empty", name);
-    } else
+    if (str == "")
+        throw UsageError("setting '%s' is a path and paths cannot be empty", s.name);
+    else
         return canonPath(str);
 }
 
+Path PathSetting::parse(const std::string & str) const
+{
+    return parsePath(*this, str);
+}
+
+std::optional<Path> OptionalPathSetting::parse(const std::string & str) const
+{
+    if (str == "")
+        return std::nullopt;
+    else
+        return parsePath(*this, str);
+}
+
 bool GlobalConfig::set(const std::string & name, const std::string & value)
 {
     for (auto & config : *configRegistrations)
-- 
cgit v1.2.3