config: do not parse TOML value expression by write_config_value_to_f…

…ile()

It's a bit weird that a write() function parses user input, and some callers
doesn't want such flakiness.

cli/src/commands/config/set.rs

@@ -17,7 +17,9 @@ use tracing::instrument;
 use super::ConfigLevelArgs;
 use crate::cli_util::{get_new_config_file_path, CommandHelper};
 use crate::command_error::{user_error, CommandError};
-use crate::config::{write_config_value_to_file, ConfigNamePathBuf};
+use crate::config::{
+    parse_toml_value_or_bare_string, write_config_value_to_file, ConfigNamePathBuf,
+};
 use crate::ui::Ui;
 
 /// Update config file to set the given option to a given value.
@@ -44,5 +46,7 @@ pub fn cmd_config_set(
             path = config_path.display()
         )));
     }
-    write_config_value_to_file(&args.name, &args.value, &config_path)
+    // TODO(#531): Infer types based on schema (w/ --type arg to override).
+    let value = parse_toml_value_or_bare_string(&args.value);
+    write_config_value_to_file(&args.name, value, &config_path)
 }

cli/src/commands/git/clone.rs

@@ -149,7 +149,7 @@ pub fn cmd_git_clone(
         let config_path = workspace_command.repo().repo_path().join("config.toml");
         write_config_value_to_file(
             &ConfigNamePathBuf::from_iter(["revset-aliases", "trunk()"]),
-            &format!("{default_branch}@{remote_name}"),
+            format!("{default_branch}@{remote_name}").into(),
             &config_path,
         )?;
         writeln!(

cli/src/commands/git/init.rs

@@ -238,7 +238,7 @@ pub fn maybe_set_repository_level_trunk_alias(
                 let config_path = repo.repo_path().join("config.toml");
                 write_config_value_to_file(
                     &ConfigNamePathBuf::from_iter(["revset-aliases", "trunk()"]),
-                    &format!("{default_branch}@origin"),
+                    format!("{default_branch}@origin").into(),
                     &config_path,
                 )?;
                 writeln!(

cli/src/config.rs

@@ -28,6 +28,17 @@ use tracing::instrument;
 
 use crate::command_error::{user_error, user_error_with_message, CommandError};
 
+/// Parses a TOML value expression. Interprets the given value as string if it
+/// can't be parsed.
+pub fn parse_toml_value_or_bare_string(value_str: &str) -> toml_edit::Value {
+    match value_str.parse() {
+        Ok(value) => value,
+        // TODO: might be better to reject meta characters. A typo in TOML value
+        // expression shouldn't be silently converted to string.
+        _ => value_str.into(),
+    }
+}
+
 pub fn to_toml_value(value: &config::Value) -> Result<toml_edit::Value, config::ConfigError> {
     fn type_error<T: fmt::Display>(message: T) -> config::ConfigError {
         config::ConfigError::Message(message.to_string())
@@ -566,7 +577,7 @@ fn read_config_path(config_path: &Path) -> Result<config::Config, config::Config
 
 pub fn write_config_value_to_file(
     key: &ConfigNamePathBuf,
-    value_str: &str,
+    value: toml_edit::Value,
     path: &Path,
 ) -> Result<(), CommandError> {
     // Read config
@@ -588,12 +599,6 @@ pub fn write_config_value_to_file(
     })?;
 
     // Apply config value
-    // Interpret value as string if it can't be parsed as a TOML value.
-    // TODO(#531): Infer types based on schema (w/ --type arg to override).
-    let item = match value_str.parse() {
-        Ok(value) => toml_edit::Item::Value(value),
-        _ => toml_edit::value(value_str),
-    };
     let mut target_table = doc.as_table_mut();
     let mut key_parts_iter = key.components();
     let last_key_part = key_parts_iter.next_back().expect("key must not be empty");
@@ -618,7 +623,7 @@ pub fn write_config_value_to_file(
             )));
         }
     }
-    target_table[last_key_part] = item;
+    target_table[last_key_part] = toml_edit::Item::Value(value);
 
     // Write config back
     std::fs::write(path, doc.to_string()).map_err(|err| {