Implement "config set" subcommand

Uses toml_edit to support simple config edits like:
  jj config set --repo user.email "[email protected]"

CHANGELOG.md

@@ -26,6 +26,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * `jj git fetch` now supports a `--branch` argument to fetch some of the
   branches only.
 
+* `jj config set` command allows simple config edits like
+  `jj config set --repo user.email "[email protected]"`
+
 ### Fixed bugs
 
 * Modify/delete conflicts now include context lines

Cargo.toml

@@ -62,6 +62,7 @@ thiserror = "1.0.38"
 tracing = "0.1.37"
 tracing-subscriber = { version = "0.3.16", default-features = false, features = ["std", "ansi", "env-filter", "fmt"] }
 indexmap = "1.9.2"
+toml_edit = { version = "0.19.1", features = ["serde"] }
 
 [target.'cfg(unix)'.dependencies]
 libc = { version = "0.2.139" }

src/cli_util.rs

@@ -21,6 +21,7 @@ use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
 use std::rc::Rc;
+use std::str::FromStr;
 use std::sync::Arc;
 
 use clap::builder::{NonEmptyStringValueParser, TypedValueParser, ValueParserFactory};
@@ -55,10 +56,13 @@ use jujutsu_lib::working_copy::{
 use jujutsu_lib::workspace::{Workspace, WorkspaceInitError, WorkspaceLoadError, WorkspaceLoader};
 use jujutsu_lib::{dag_walk, file_util, git, revset};
 use thiserror::Error;
+use toml_edit;
 use tracing_subscriber::prelude::*;
 
 use crate::commit_templater;
-use crate::config::{AnnotatedValue, CommandNameAndArgs, LayeredConfigs};
+use crate::config::{
+    config_path, AnnotatedValue, CommandNameAndArgs, ConfigSource, LayeredConfigs,
+};
 use crate::formatter::{Formatter, PlainTextFormatter};
 use crate::merge_tools::{ConflictResolveError, DiffEditError};
 use crate::template_parser::{TemplateAliasesMap, TemplateParseError};
@@ -1596,6 +1600,85 @@ pub fn serialize_config_value(value: &config::Value) -> String {
     }
 }
 
+pub fn write_config_value_to_file(
+    key: &str,
+    value_str: &str,
+    path: &PathBuf,
+) -> Result<(), CommandError> {
+    // Read config
+    let config_toml = std::fs::read_to_string(path).or_else(|err| {
+        match err.kind() {
+            // If config doesn't exist yet, read as empty and we'll write one.
+            std::io::ErrorKind::NotFound => Ok("".to_string()),
+            _ => Err(user_error(format!(
+                "Failed to read file {path}: {err:?}",
+                path = path.display()
+            ))),
+        }
+    })?;
+    let mut doc = config_toml.parse::<toml_edit::Document>().map_err(|err| {
+        user_error(format!(
+            "Failed to parse file {path}: {err:?}",
+            path = path.display()
+        ))
+    })?;
+
+    // Apply config value
+    // Iterpret value as string unless it's another simple scalar type.
+    // TODO(#531): Infer types based on schema (w/ --type arg to override).
+    let item = match toml_edit::Value::from_str(value_str) {
+        Ok(value @ toml_edit::Value::Boolean(..))
+        | Ok(value @ toml_edit::Value::Integer(..))
+        | Ok(value @ toml_edit::Value::Float(..))
+        | Ok(value @ toml_edit::Value::String(..)) => toml_edit::value(value),
+        _ => toml_edit::value(value_str),
+    };
+    // TODO(#531): Prefer to create new structured tables vs inline.
+    let mut target_entry = doc.as_item_mut();
+    let mut key_parts_iter = key.split('.');
+    // Note: split guarantees at least one item.
+    let mut last_key_part = key_parts_iter.next().unwrap();
+    for key_part in &mut key_parts_iter {
+        target_entry = &mut target_entry[last_key_part];
+        last_key_part = key_part;
+    }
+    target_entry[last_key_part] = item;
+
+    // Write config back
+    if let Err(err) = std::fs::write(path, doc.to_string()) {
+        return Err(user_error(format!(
+            "Failed to write file {path}: {err:?}",
+            path = path.display()
+        )));
+    }
+    Ok(())
+}
+
+pub fn get_config_file_path(
+    config_source: &ConfigSource,
+    workspace_command: &WorkspaceCommandHelper,
+) -> Result<PathBuf, CommandError> {
+    let edit_path = match config_source {
+        // TODO(#531): Special-case for editors that can't handle viewing directories?
+        ConfigSource::User => {
+            config_path()?.ok_or_else(|| user_error("No repo config path found to edit"))?
+        }
+        ConfigSource::Repo => {
+            let workspace_path = workspace_command.workspace_root();
+            WorkspaceLoader::init(workspace_path)
+                .unwrap()
+                .repo_path()
+                .join("config.toml")
+        }
+        _ => {
+            return Err(user_error(format!(
+                "Can't get path for config source {config_source:?}"
+            )));
+        }
+    };
+    Ok(edit_path)
+}
+
 pub fn run_ui_editor(settings: &UserSettings, edit_path: &PathBuf) -> Result<(), CommandError> {
     let editor: CommandNameAndArgs = settings
         .config()

src/commands/mod.rs

@@ -42,17 +42,18 @@ use jujutsu_lib::revset_graph_iterator::{
 use jujutsu_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit, DescendantRebaser};
 use jujutsu_lib::settings::UserSettings;
 use jujutsu_lib::tree::{merge_trees, Tree};
-use jujutsu_lib::workspace::{Workspace, WorkspaceLoader};
+use jujutsu_lib::workspace::Workspace;
 use jujutsu_lib::{conflicts, file_util, revset};
 use maplit::{hashmap, hashset};
 
 use crate::cli_util::{
-    self, check_stale_working_copy, print_checkout_stats, resolve_multiple_nonempty_revsets,
-    resolve_mutliple_nonempty_revsets_flag_guarded, run_ui_editor, serialize_config_value,
-    short_commit_hash, user_error, user_error_with_hint, Args, CommandError, CommandHelper,
-    DescriptionArg, RevisionArg, WorkspaceCommandHelper,
+    self, check_stale_working_copy, get_config_file_path, print_checkout_stats,
+    resolve_multiple_nonempty_revsets, resolve_mutliple_nonempty_revsets_flag_guarded,
+    run_ui_editor, serialize_config_value, short_commit_hash, user_error, user_error_with_hint,
+    write_config_value_to_file, Args, CommandError, CommandHelper, DescriptionArg, RevisionArg,
+    WorkspaceCommandHelper,
 };
-use crate::config::{config_path, AnnotatedValue, ConfigSource};
+use crate::config::{AnnotatedValue, ConfigSource};
 use crate::diff_util::{self, DiffFormat, DiffFormatArgs};
 use crate::formatter::{Formatter, PlainTextFormatter};
 use crate::graphlog::{get_graphlog, Edge};
@@ -151,6 +152,19 @@ struct ConfigArgs {
     repo: bool,
 }
 
+impl ConfigArgs {
+    fn get_source_kind(&self) -> ConfigSource {
+        if self.user {
+            ConfigSource::User
+        } else if self.repo {
+            ConfigSource::Repo
+        } else {
+            // Shouldn't be reachable unless clap ArgGroup is broken.
+            panic!("No config_level provided");
+        }
+    }
+}
+
 /// Manage config options
 ///
 /// Operates on jj configuration, which comes from the config file and
@@ -161,14 +175,12 @@ struct ConfigArgs {
 ///
 /// For supported config options and more details about jj config, see
 /// https://github.com/martinvonz/jj/blob/main/docs/config.md.
-///
-/// Note: Currently only supports getting config options and editing config
-/// files, but support for setting options is also planned (see
-/// https://github.com/martinvonz/jj/issues/531).
 #[derive(clap::Subcommand, Clone, Debug)]
 enum ConfigSubcommand {
     #[command(visible_alias("l"))]
     List(ConfigListArgs),
+    #[command(visible_alias("s"))]
+    Set(ConfigSetArgs),
     #[command(visible_alias("e"))]
     Edit(ConfigEditArgs),
 }
@@ -186,6 +198,18 @@ struct ConfigListArgs {
     // TODO(#1047): Support ConfigArgs (--user or --repo).
 }
 
+/// Update config file to set the given option to a given value.
+#[derive(clap::Args, Clone, Debug)]
+struct ConfigSetArgs {
+    #[arg(required = true)]
+    name: String,
+    #[arg(required = true)]
+    value: String,
+    #[clap(flatten)]
+    config_args: ConfigArgs,
+}
+
+/// Start an editor on a jj config file.
 #[derive(clap::Args, Clone, Debug)]
 struct ConfigEditArgs {
     #[clap(flatten)]
@@ -1062,6 +1086,7 @@ fn cmd_config(
 ) -> Result<(), CommandError> {
     match subcommand {
         ConfigSubcommand::List(sub_args) => cmd_config_list(ui, command, sub_args),
+        ConfigSubcommand::Set(sub_args) => cmd_config_set(ui, command, sub_args),
         ConfigSubcommand::Edit(sub_args) => cmd_config_edit(ui, command, sub_args),
     }
 }
@@ -1108,27 +1133,34 @@ fn cmd_config_list(
     Ok(())
 }
 
+fn cmd_config_set(
+    ui: &mut Ui,
+    command: &CommandHelper,
+    args: &ConfigSetArgs,
+) -> Result<(), CommandError> {
+    let config_path = get_config_file_path(
+        &args.config_args.get_source_kind(),
+        &command.workspace_helper(ui)?,
+    )?;
+    if config_path.is_dir() {
+        return Err(user_error(format!(
+            "Can't set config in path {path} (dirs not supported)",
+            path = config_path.display()
+        )));
+    }
+    write_config_value_to_file(&args.name, &args.value, &config_path)
+}
+
 fn cmd_config_edit(
     ui: &mut Ui,
     command: &CommandHelper,
     args: &ConfigEditArgs,
 ) -> Result<(), CommandError> {
-    let edit_path = if args.config_args.user {
-        // TODO(#531): Special-case for editors that can't handle viewing directories?
-        config_path()?.ok_or_else(|| user_error("No repo config path found to edit"))?
-    } else if args.config_args.repo {
-        let workspace_command = command.workspace_helper(ui)?;
-        let workspace_path = workspace_command.workspace_root();
-        WorkspaceLoader::init(workspace_path)
-            .unwrap()
-            .repo_path()
-            .join("config.toml")
-    } else {
-        // Shouldn't be reachable unless clap ArgGroup is broken.
-        panic!("No config_level provided");
-    };
-    run_ui_editor(command.settings(), &edit_path)?;
-    Ok(())
+    let config_path = get_config_file_path(
+        &args.config_args.get_source_kind(),
+        &command.workspace_helper(ui)?,
+    )?;
+    run_ui_editor(command.settings(), &config_path)
 }
 
 fn cmd_checkout(

tests/common/mod.rs

@@ -24,7 +24,7 @@ pub struct TestEnvironment {
     _temp_dir: TempDir,
     env_root: PathBuf,
     home_dir: PathBuf,
-    config_dir: PathBuf,
+    config_path: PathBuf,
     env_vars: HashMap<String, String>,
     config_file_number: RefCell<i64>,
     command_number: RefCell<i64>,
@@ -45,7 +45,7 @@ impl Default for TestEnvironment {
             _temp_dir: tmp_dir,
             env_root,
             home_dir,
-            config_dir,
+            config_path: config_dir,
             env_vars,
             config_file_number: RefCell::new(0),
             command_number: RefCell::new(0),
@@ -64,7 +64,7 @@ impl TestEnvironment {
         }
         cmd.env("RUST_BACKTRACE", "1");
         cmd.env("HOME", self.home_dir.to_str().unwrap());
-        cmd.env("JJ_CONFIG", self.config_dir.to_str().unwrap());
+        cmd.env("JJ_CONFIG", self.config_path.to_str().unwrap());
         cmd.env("JJ_USER", "Test User");
         cmd.env("JJ_EMAIL", "[email protected]");
         cmd.env("JJ_OP_HOSTNAME", "host.example.com");
@@ -111,18 +111,25 @@ impl TestEnvironment {
         &self.home_dir
     }
 
-    pub fn config_dir(&self) -> &PathBuf {
-        &self.config_dir
+    pub fn config_path(&self) -> &PathBuf {
+        &self.config_path
+    }
+
+    pub fn set_config_path(&mut self, config_path: PathBuf) {
+        self.config_path = config_path;
     }
 
     pub fn add_config(&self, content: &str) {
+        if self.config_path.is_file() {
+            panic!("add_config not supported when config_path is a file");
+        }
         // Concatenating two valid TOML files does not (generally) result in a valid
         // TOML file, so we use create a new file every time instead.
         let mut config_file_number = self.config_file_number.borrow_mut();
         *config_file_number += 1;
         let config_file_number = *config_file_number;
         std::fs::write(
-            self.config_dir
+            self.config_path
                 .join(format!("config{config_file_number:04}.toml")),
             content,
         )