[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 170257.
JDevlieghere added a comment.

Only clear setting when the force flag is set (as suggested offline by @jingham).


https://reviews.llvm.org/D52772

Files:
  lit/Settings/TestSettingsSet.test
  source/Commands/CommandObjectSettings.cpp


Index: source/Commands/CommandObjectSettings.cpp
===================================================================
--- source/Commands/CommandObjectSettings.cpp
+++ source/Commands/CommandObjectSettings.cpp
@@ -30,7 +30,8 @@
 
 static constexpr OptionDefinition g_settings_set_options[] = {
     // clang-format off
-  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." }
+  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." },
+  { LLDB_OPT_SET_2, false, "force",  'f', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Force an empty value to be accepted as the default." }
     // clang-format on
 };
 
@@ -108,6 +109,9 @@
       const int short_option = m_getopt_table[option_idx].val;
 
       switch (short_option) {
+      case 'f':
+        m_force = true;
+        break;
       case 'g':
         m_global = true;
         break;
@@ -122,15 +126,16 @@
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_global = false;
+      m_force = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_settings_set_options);
     }
 
     // Instance variables to hold the values for command options.
-
     bool m_global;
+    bool m_force;
   };
 
   int HandleArgumentCompletion(
@@ -184,8 +189,10 @@
     if (!ParseOptions(cmd_args, result))
       return false;
 
+    const size_t min_argc = m_options.m_force ? 1 : 2;
     const size_t argc = cmd_args.GetArgumentCount();
-    if ((argc < 2) && (!m_options.m_global)) {
+
+    if ((argc < min_argc) && (!m_options.m_global)) {
       result.AppendError("'settings set' takes more arguments");
       result.SetStatus(eReturnStatusFailed);
       return false;
@@ -199,6 +206,19 @@
       return false;
     }
 
+    // A missing value corresponds to clearing the setting when "force" is
+    // specified.
+    if (argc == 1 && m_options.m_force) {
+      Status error(m_interpreter.GetDebugger().SetPropertyValue(
+          &m_exe_ctx, eVarSetOperationClear, var_name, llvm::StringRef()));
+      if (error.Fail()) {
+        result.AppendError(error.AsCString());
+        result.SetStatus(eReturnStatusFailed);
+        return false;
+      }
+      return result.Succeeded();
+    }
+
     // Split the raw command into var_name and value pair.
     llvm::StringRef raw_str(command);
     std::string var_value_string = raw_str.split(var_name).second.str();
Index: lit/Settings/TestSettingsSet.test
===================================================================
--- /dev/null
+++ lit/Settings/TestSettingsSet.test
@@ -0,0 +1,15 @@
+# This tests setting setting values.
+
+# Check that setting an empty value with -f(orce) clears the value.
+# RUN: %lldb -b -s %s 2>&1 | FileCheck %s
+
+settings set tab-size 16
+settings show tab-size
+# CHECK: tab-size (unsigned) = 16
+
+settings set -f tab-size
+settings show tab-size
+# CHECK: tab-size (unsigned) = 4
+
+settings set tab-size
+# CHECK: error: 'settings set' takes more arguments


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52772.170257.patch
Type: text/x-patch
Size: 3207 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181019/9e08015e/attachment.bin>