diff --git a/CHANGELOG.md b/CHANGELOG.md index f072c05d6b..28823c809f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,8 +45,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). `snapshot.max-new-file-size` config option. It will print a warning and large files will be left untracked. -* New command option `--config-file=PATH` to load additional configuration from - files. +* New command options `--config=NAME=VALUE` and `--config-file=PATH` to set + string value without quoting and to load additional configuration from files. * Templates now support the `>=`, `>`, `<=`, and `<` relational operators for `Integer` types. diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 411350ade9..4a28897444 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -3099,8 +3099,13 @@ pub struct EarlyArgs { // Option. pub no_pager: Option, /// Additional configuration options (can be repeated) - // TODO: Introduce a `--config` option with simpler syntax for simple - // cases, designed so that `--config ui.color=auto` works + /// + /// The name should be specified as TOML dotted keys. The value should be + /// specified in either TOML syntax or as a string. If string value contains + /// TOML constructs (such as array notation), it must be quoted. + #[arg(long, value_name = "NAME=VALUE", global = true)] + pub config: Vec, + /// Additional configuration options (can be repeated) #[arg(long, value_name = "TOML", global = true)] pub config_toml: Vec, /// Additional configuration files (can be repeated) @@ -3113,10 +3118,12 @@ impl EarlyArgs { merge_args_with( matches, &[ + ("config", &self.config), ("config_toml", &self.config_toml), ("config_file", &self.config_file), ], |id, value| match id { + "config" => (ConfigArgKind::Item, value.as_ref()), "config_toml" => (ConfigArgKind::Toml, value.as_ref()), "config_file" => (ConfigArgKind::File, value.as_ref()), _ => unreachable!("unexpected id {id:?}"), diff --git a/cli/src/complete.rs b/cli/src/complete.rs index 80ed52a0fc..9a0bc80a23 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -748,6 +748,7 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { } for (kind, value) in args.early_args.merged_config_args(&arg_matches) { let arg = match kind { + ConfigArgKind::Item => format!("--config={value}"), ConfigArgKind::Toml => format!("--config-toml={value}"), ConfigArgKind::File => format!("--config-file={value}"), }; diff --git a/cli/src/config.rs b/cli/src/config.rs index 8e3fe51014..e547d59c47 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -34,6 +34,10 @@ use regex::Regex; use thiserror::Error; use tracing::instrument; +use crate::command_error::config_error; +use crate::command_error::config_error_with_message; +use crate::command_error::CommandError; + // TODO(#879): Consider generating entire schema dynamically vs. static file. pub const CONFIG_SCHEMA: &str = include_str!("config-schema.json"); @@ -376,7 +380,7 @@ fn config_files_for( /// 4. Repo config `.jj/repo/config.toml` /// 5. TODO: Workspace config `.jj/config.toml` /// 6. Override environment variables -/// 7. Command-line arguments `--config-toml`, `--config-file` +/// 7. Command-line arguments `--config`, `--config-toml`, `--config-file` /// /// This function sets up 1, 2, and 6. pub fn config_from_environment( @@ -461,24 +465,34 @@ fn env_overrides_layer() -> ConfigLayer { /// Configuration source/data type provided as command-line argument. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum ConfigArgKind { + /// `--config=NAME=VALUE` + Item, /// `--config-toml=TOML` Toml, /// `--config-file=PATH` File, } -/// Parses `--config-toml` arguments. +/// Parses `--config*` arguments. pub fn parse_config_args( toml_strs: &[(ConfigArgKind, &str)], -) -> Result, ConfigLoadError> { - // It might look silly that a layer is constructed per argument, but - // --config-toml argument can contain a full TOML document, and it makes - // sense to preserve line numbers within the doc. If we add - // --config=KEY=VALUE, multiple values might be loaded into one layer. +) -> Result, CommandError> { let source = ConfigSource::CommandArg; let mut layers = Vec::new(); for (kind, chunk) in &toml_strs.iter().chunk_by(|&(kind, _)| kind) { match kind { + ConfigArgKind::Item => { + let mut layer = ConfigLayer::empty(source); + for (_, item) in chunk { + let (name, value) = parse_config_arg_item(item)?; + // Can fail depending on the argument order, but that + // wouldn't matter in practice. + layer.set_value(name, value).map_err(|err| { + config_error_with_message("--config argument cannot be set", err) + })?; + } + layers.push(layer); + } ConfigArgKind::Toml => { for (_, text) in chunk { layers.push(ConfigLayer::parse(source, text)?); @@ -494,6 +508,24 @@ pub fn parse_config_args( Ok(layers) } +/// Parses `NAME=VALUE` string. +fn parse_config_arg_item(item_str: &str) -> Result<(ConfigNamePathBuf, ConfigValue), CommandError> { + // split NAME=VALUE at the first parsable position + let split_candidates = item_str.as_bytes().iter().positions(|&b| b == b'='); + let Some((name, value_str)) = split_candidates + .map(|p| (&item_str[..p], &item_str[p + 1..])) + .map(|(name, value)| name.parse().map(|name| (name, value))) + .find_or_last(Result::is_ok) + .transpose() + .map_err(|err| config_error_with_message("--config name cannot be parsed", err))? + else { + return Err(config_error("--config must be specified as NAME=VALUE")); + }; + let value = parse_value_or_bare_string(value_str) + .map_err(|err| config_error_with_message("--config value cannot be parsed", err))?; + Ok((name, value)) +} + /// Command name and arguments specified by config. #[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize)] #[serde(untagged)] @@ -678,6 +710,46 @@ mod tests { assert!(parse("[table]\nkey = 'value'").is_err()); } + #[test] + fn test_parse_config_arg_item() { + assert!(parse_config_arg_item("").is_err()); + assert!(parse_config_arg_item("a").is_err()); + assert!(parse_config_arg_item("=").is_err()); + assert!(parse_config_arg_item("a=b=c").is_err()); + // The value parser is sensitive to leading whitespaces, which seems + // good because the parsing falls back to a bare string. + assert!(parse_config_arg_item("a = 'b'").is_err()); + + let (name, value) = parse_config_arg_item("a=b").unwrap(); + assert_eq!(name, ConfigNamePathBuf::from_iter(["a"])); + assert_eq!(value.as_str(), Some("b")); + + let (name, value) = parse_config_arg_item("a=").unwrap(); + assert_eq!(name, ConfigNamePathBuf::from_iter(["a"])); + assert_eq!(value.as_str(), Some("")); + + let (name, value) = parse_config_arg_item("a= ").unwrap(); + assert_eq!(name, ConfigNamePathBuf::from_iter(["a"])); + assert_eq!(value.as_str(), Some(" ")); + + let (name, value) = parse_config_arg_item("a.b=true").unwrap(); + assert_eq!(name, ConfigNamePathBuf::from_iter(["a", "b"])); + assert_eq!(value.as_bool(), Some(true)); + + let (name, value) = parse_config_arg_item("a='b=c'").unwrap(); + assert_eq!(name, ConfigNamePathBuf::from_iter(["a"])); + assert_eq!(value.as_str(), Some("b=c")); + + let (name, value) = parse_config_arg_item("'a=b'=c").unwrap(); + assert_eq!(name, ConfigNamePathBuf::from_iter(["a=b"])); + assert_eq!(value.as_str(), Some("c")); + + let (name, value) = parse_config_arg_item("'a = b=c '={d = 'e=f'}").unwrap(); + assert_eq!(name, ConfigNamePathBuf::from_iter(["a = b=c "])); + assert!(value.is_inline_table()); + assert_eq!(value.to_string(), "{d = 'e=f'}"); + } + #[test] fn test_command_args() { let mut config = StackedConfig::empty(); diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 8fe0fb0e65..0c356eb9cf 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -195,6 +195,9 @@ To get started, see the tutorial at https://martinvonz.github.io/jj/latest/tutor Warnings and errors will still be printed. * `--no-pager` — Disable the pager +* `--config ` — Additional configuration options (can be repeated) + + The name should be specified as TOML dotted keys. The value should be specified in either TOML syntax or as a string. If string value contains TOML constructs (such as array notation), it must be quoted. * `--config-toml ` — Additional configuration options (can be repeated) * `--config-file ` — Additional configuration files (can be repeated) diff --git a/cli/tests/test_completion.rs b/cli/tests/test_completion.rs index 9cec9a0927..9a79c8d320 100644 --- a/cli/tests/test_completion.rs +++ b/cli/tests/test_completion.rs @@ -82,6 +82,7 @@ fn test_bookmark_names() { --color When to colorize output (always, never, debug, auto) --quiet Silence non-primary command output --no-pager Disable the pager + --config Additional configuration options (can be repeated) --config-toml Additional configuration options (can be repeated) --config-file Additional configuration files (can be repeated) --help Print help (see more with '--help') diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index 06e3393815..eaf86a562a 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -623,6 +623,8 @@ fn test_config_args() { ) .unwrap(); + let stdout = list_config(&["--config=test.key1=arg1"]); + insta::assert_snapshot!(stdout, @r#"test.key1 = "arg1""#); let stdout = list_config(&["--config-toml=test.key1='arg1'"]); insta::assert_snapshot!(stdout, @"test.key1 = 'arg1'"); let stdout = list_config(&["--config-file=file1.toml"]); @@ -631,19 +633,36 @@ fn test_config_args() { test.key2 = 'file1' "); + // --config items are inserted to a single layer internally + let stdout = list_config(&[ + "--config=test.key1='arg1'", + "--config=test.key2.sub=true", + "--config=test.key1=arg3", + ]); + insta::assert_snapshot!(stdout, @r#" + test.key1 = "arg3" + test.key2.sub = true + "#); + // --config* arguments are processed in order of appearance let stdout = list_config(&[ - "--config-toml=test.key1='arg1'", + "--config=test.key1=arg1", "--config-file=file1.toml", "--config-toml=test.key2='arg3'", "--config-file=file2.toml", ]); - insta::assert_snapshot!(stdout, @r" - # test.key1 = 'arg1' + insta::assert_snapshot!(stdout, @r##" + # test.key1 = "arg1" test.key1 = 'file1' # test.key2 = 'file1' test.key2 = 'arg3' test.key3 = 'file2' + "##); + + let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["config", "list", "--config=foo"]); + insta::assert_snapshot!(stderr, @r" + Config error: --config must be specified as NAME=VALUE + For help, see https://martinvonz.github.io/jj/latest/config/. "); let stderr = test_env.jj_cmd_failure( @@ -775,6 +794,7 @@ fn test_help() { --color When to colorize output (always, never, debug, auto) --quiet Silence non-primary command output --no-pager Disable the pager + --config Additional configuration options (can be repeated) --config-toml Additional configuration options (can be repeated) --config-file Additional configuration files (can be repeated) "); diff --git a/docs/config.md b/docs/config.md index a232986813..e6ce73f0b5 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1203,17 +1203,18 @@ env JJ_CONFIG=/dev/null jj log # Ignores any settings specified in the con ### Specifying config on the command-line -You can use one or more `--config-toml`/`--config-file` options on the command -line to specify additional configuration settings. This overrides settings -defined in config files or environment variables. For example, +You can use one or more `--config`/`--config-toml`/`--config-file` options on +the command line to specify additional configuration settings. This overrides +settings defined in config files or environment variables. For example, ```shell -jj --config-toml='ui.color="always"' --config-toml='ui.diff-editor="kdiff3"' split +jj --config=ui.color=always --config-toml='ui.diff-editor="kdiff3"' split ``` -Config specified this way must be valid TOML. In particular, string values must -be surrounded by quotes. To pass these quotes to `jj`, most shells require -surrounding those quotes with single quotes as shown above. +Config specified by `--config-toml` must be valid TOML. In particular, string +values must be surrounded by quotes. To pass these quotes to `jj`, most shells +require surrounding those quotes with single quotes as shown above. On the other +hand, `--config` can accept a bare string value. In `sh`-compatible shells, `--config-toml` can be used to merge entire TOML files with the config specified in `.jjconfig.toml`: