Keep comments when using -M/-F/-o to set config values

diff --git a/src/Config.cpp b/src/Config.cpp
index 03d547579a3bc7214d671e810720e8e6b057a727..c7168e06da8898823533c3fa0005e11d88faed63 100644 (file)
--- a/src/Config.cpp
+++ b/src/Config.cpp
@@ -158,9 +158,6 @@ const std::unordered_map<std::string, std::string> k_env_variable_table = {
   {"UMASK", "umask"},
 };
 
-using ConfigLineHandler = std::function<void(
-  const std::string& line, const std::string& key, const std::string& value)>;
-
 bool
 parse_bool(const std::string& value,
            const optional<std::string> env_var_key,
@@ -354,6 +351,12 @@ parse_line(const std::string& line,
   return true;
 }
 
+// `line` is the full configuration line excluding newline. `key` will be empty
+// for comments and blank lines. `value` does not include newline.
+using ConfigLineHandler = std::function<void(
+  const std::string& line, const std::string& key, const std::string& value)>;
+
+// Call `config_line_handler` for each line in `path`.
 bool
 parse_config_file(const std::string& path,
                   const ConfigLineHandler& config_line_handler)
@@ -376,10 +379,7 @@ parse_config_file(const std::string& path,
       if (!parse_line(line, &key, &value, &error_message)) {
         throw Error(error_message);
       }
-      if (!key.empty()) {
-        // key is empty if comment or blank line.
-        config_line_handler(line, key, value);
-      }
+      config_line_handler(line, key, value);
     } catch (const Error& e) {
       throw Error("{}:{}: {}", path, line_number, e.what());
     }
@@ -420,7 +420,9 @@ Config::update_from_file(const std::string& path)
                            [&](const std::string& /*line*/,
                                const std::string& key,
                                const std::string& value) {
-                             set_item(key, value, nullopt, false, path);
+                             if (!key.empty()) {
+                               set_item(key, value, nullopt, false, path);
+                             }
                            });
 }
 

diff --git a/unittest/test_Config.cpp b/unittest/test_Config.cpp
index 60f672ed0be814a803b7ba533f7ec4245850b8a2..95787dbac9f391e6cd4678bf77c85c5d68f3d03a 100644 (file)
--- a/unittest/test_Config.cpp
+++ b/unittest/test_Config.cpp
@@ -319,6 +319,15 @@ TEST_CASE("Config::set_value_in_file")
     std::string content = Util::read_file("ccache.conf");
     CHECK(content == "path = vanilla\nsloppiness = foo\n");
   }
+
+  SUBCASE("comments are kept")
+  {
+    Util::write_file("ccache.conf", "# c1\npath = blueberry\n#c2\n");
+    Config::set_value_in_file("ccache.conf", "path", "vanilla");
+    Config::set_value_in_file("ccache.conf", "compiler", "chocolate");
+    std::string content = Util::read_file("ccache.conf");
+    CHECK(content == "# c1\npath = vanilla\n#c2\ncompiler = chocolate\n");
+  }
 }
 
 TEST_CASE("Config::get_string_value")