cli: add --config-file=PATH argument

This would be useful for scripting purpose. Maybe we can also replace the
current --config-toml=<TOML> use cases by --config-file=<PATH> and simpler
--config=<KEY>=<VALUE>.

#4926 (comment)

If we want to add more source variants (such as fd number), it might be better
to add --config-from=<type>:<path|fd|..>. In any case, we'll probably want
--config=<KEY>=<VALUE>, and therefore, we'll need to merge more than one
--config* arguments.

Author: yuja

CHANGELOG.md

@@ -35,6 +35,9 @@ 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.
+
 * Templates now support the `>=`, `>`, `<=`, and `<` relational operators for
   `Integer` types.
 

cli/src/cli_util.rs

@@ -157,6 +157,7 @@ use crate::complete;
 use crate::config::config_from_environment;
 use crate::config::parse_config_args;
 use crate::config::CommandNameAndArgs;
+use crate::config::ConfigArg;
 use crate::config::ConfigEnv;
 use crate::diff_util;
 use crate::diff_util::DiffFormat;
@@ -3102,6 +3103,26 @@ pub struct EarlyArgs {
     //  cases, designed so that `--config ui.color=auto` works
     #[arg(long, value_name = "TOML", global = true)]
     pub config_toml: Vec<String>,
+    /// Additional configuration files (can be repeated)
+    #[arg(long, value_name = "PATH", global = true, value_hint = clap::ValueHint::FilePath)]
+    pub config_file: Vec<String>,
+}
+
+impl EarlyArgs {
+    fn merged_config_args(&self, matches: &ArgMatches) -> Vec<ConfigArg> {
+        merge_args_with(
+            matches,
+            &[
+                ("config_toml", &self.config_toml),
+                ("config_file", &self.config_file),
+            ],
+            |id, value| match id {
+                "config_toml" => ConfigArg::Toml(value.clone()),
+                "config_file" => ConfigArg::File(value.clone()),
+                _ => unreachable!("unexpected id {id:?}"),
+            },
+        )
+    }
 }
 
 /// Wrapper around revset expression argument.
@@ -3143,6 +3164,29 @@ impl ValueParserFactory for RevisionArg {
     }
 }
 
+/// Merges multiple clap args in order of appearance.
+///
+/// The `id_values` is a list of `(id, values)` pairs, where `id` is the name of
+/// the clap `Arg`, and `values` are the parsed values for that arg. The
+/// `convert` function transforms each `(id, value)` pair to e.g. an enum.
+///
+/// This is a workaround for <https://github.com/clap-rs/clap/issues/3146>.
+pub fn merge_args_with<'k, 'v, T, U>(
+    matches: &ArgMatches,
+    id_values: &[(&'k str, &'v [T])],
+    mut convert: impl FnMut(&'k str, &'v T) -> U,
+) -> Vec<U> {
+    let mut pos_values: Vec<(usize, U)> = Vec::new();
+    for (id, values) in id_values {
+        pos_values.extend(itertools::zip_eq(
+            matches.indices_of(id).into_iter().flatten(),
+            values.iter().map(|v| convert(id, v)),
+        ));
+    }
+    pos_values.sort_unstable_by_key(|&(pos, _)| pos);
+    pos_values.into_iter().map(|(_, value)| value).collect()
+}
+
 fn get_string_or_array(
     config: &StackedConfig,
     key: &'static str,
@@ -3279,7 +3323,7 @@ fn handle_early_args(
     let args = EarlyArgs::from_arg_matches(&early_matches).unwrap();
 
     let old_layers_len = config.layers().len();
-    config.extend_layers(parse_config_args(&args.config_toml)?);
+    config.extend_layers(parse_config_args(&args.merged_config_args(&early_matches))?);
 
     // Command arguments overrides any other configuration including the
     // variables loaded from --config* arguments.
@@ -3711,3 +3755,45 @@ fn format_template_aliases_hint(template_aliases: &TemplateAliasesMap) -> String
     );
     hint
 }
+
+#[cfg(test)]
+mod tests {
+    use clap::CommandFactory as _;
+
+    use super::*;
+
+    #[derive(clap::Parser, Clone, Debug)]
+    pub struct TestArgs {
+        #[arg(long)]
+        pub foo: Vec<u32>,
+        #[arg(long)]
+        pub bar: Vec<u32>,
+        #[arg(long)]
+        pub baz: bool,
+    }
+
+    #[test]
+    fn test_merge_args_with() {
+        let command = TestArgs::command();
+        let parse = |args: &[&str]| -> Vec<(&'static str, u32)> {
+            let matches = command.clone().try_get_matches_from(args).unwrap();
+            let args = TestArgs::from_arg_matches(&matches).unwrap();
+            merge_args_with(
+                &matches,
+                &[("foo", &args.foo), ("bar", &args.bar)],
+                |id, value| (id, *value),
+            )
+        };
+
+        assert_eq!(parse(&["jj"]), vec![]);
+        assert_eq!(parse(&["jj", "--foo=1"]), vec![("foo", 1)]);
+        assert_eq!(
+            parse(&["jj", "--foo=1", "--bar=2"]),
+            vec![("foo", 1), ("bar", 2)]
+        );
+        assert_eq!(
+            parse(&["jj", "--foo=1", "--baz", "--bar=2", "--foo", "3"]),
+            vec![("foo", 1), ("bar", 2), ("foo", 3)]
+        );
+    }
+}

cli/src/config.rs

@@ -319,7 +319,7 @@ impl ConfigEnv {
 /// 4. Repo config `.jj/repo/config.toml`
 /// 5. TODO: Workspace config `.jj/config.toml`
 /// 6. Override environment variables
-/// 7. Command-line arguments `--config-toml`
+/// 7. Command-line arguments `--config-toml`, `--config-file`
 ///
 /// This function sets up 1, 2, and 6.
 pub fn config_from_environment(
@@ -401,15 +401,28 @@ fn env_overrides_layer() -> ConfigLayer {
     layer
 }
 
+/// Configuration source/data provided as command-line argument.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub enum ConfigArg {
+    /// `--config-toml=TOML`
+    Toml(String),
+    /// `--config-file=PATH`
+    File(String),
+}
+
 /// Parses `--config-toml` arguments.
-pub fn parse_config_args(toml_strs: &[String]) -> Result<Vec<ConfigLayer>, ConfigLoadError> {
+pub fn parse_config_args(toml_strs: &[ConfigArg]) -> Result<Vec<ConfigLayer>, 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.
+    let source = ConfigSource::CommandArg;
     toml_strs
         .iter()
-        .map(|text| ConfigLayer::parse(ConfigSource::CommandArg, text))
+        .map(|arg| match arg {
+            ConfigArg::Toml(text) => ConfigLayer::parse(source, text),
+            ConfigArg::File(path) => ConfigLayer::load_from_file(source, path.into()),
+        })
         .try_collect()
 }
 

cli/tests/[email protected]

@@ -196,6 +196,7 @@ 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-toml <TOML>` — Additional configuration options (can be repeated)
+* `--config-file <PATH>` — Additional configuration files (can be repeated)
 
 
 

cli/tests/test_completion.rs

@@ -83,6 +83,7 @@ fn test_bookmark_names() {
     --quiet	Silence non-primary command output
     --no-pager	Disable the pager
     --config-toml	Additional configuration options (can be repeated)
+    --config-file	Additional configuration files (can be repeated)
     --help	Print help (see more with '--help')
     ");
 

cli/tests/test_global_opts.rs

@@ -14,6 +14,8 @@
 
 use std::ffi::OsString;
 
+use indoc::indoc;
+
 use crate::common::get_stderr_string;
 use crate::common::strip_last_line;
 use crate::common::TestEnvironment;
@@ -595,6 +597,72 @@ fn test_early_args() {
     );
 }
 
+#[test]
+fn test_config_args() {
+    let test_env = TestEnvironment::default();
+    let list_config = |args: &[&str]| {
+        test_env.jj_cmd_success(
+            test_env.env_root(),
+            &[&["config", "list", "--include-overridden", "test"], args].concat(),
+        )
+    };
+
+    std::fs::write(
+        test_env.env_root().join("file1.toml"),
+        indoc! {"
+            test.key1 = 'file1'
+            test.key2 = 'file1'
+        "},
+    )
+    .unwrap();
+    std::fs::write(
+        test_env.env_root().join("file2.toml"),
+        indoc! {"
+            test.key3 = 'file2'
+        "},
+    )
+    .unwrap();
+
+    let stdout = list_config(&["--config-toml=test.key1='arg1'"]);
+    insta::assert_snapshot!(stdout, @"test.key1 = 'arg1'");
+    let stdout = list_config(&["--config-file=file1.toml"]);
+    insta::assert_snapshot!(stdout, @r"
+    test.key1 = 'file1'
+    test.key2 = 'file1'
+    ");
+
+    // --config* arguments are processed in order of appearance
+    let stdout = list_config(&[
+        "--config-toml=test.key1='arg1'",
+        "--config-file=file1.toml",
+        "--config-toml=test.key2='arg3'",
+        "--config-file=file2.toml",
+    ]);
+    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-file=unknown.toml"],
+    );
+    insta::with_settings!({
+        filters => [("(?m)^([2-9]): .*", "$1: <redacted>")],
+    }, {
+        insta::assert_snapshot!(stderr, @r"
+        Config error: Failed to read configuration file
+        Caused by:
+        1: Cannot access unknown.toml
+        2: <redacted>
+        For help, see https://martinvonz.github.io/jj/latest/config/.
+        ");
+    });
+}
+
 #[test]
 fn test_invalid_config() {
     // Test that we get a reasonable error if the config is invalid (#55)
@@ -708,6 +776,7 @@ fn test_help() {
           --quiet                        Silence non-primary command output
           --no-pager                     Disable the pager
           --config-toml <TOML>           Additional configuration options (can be repeated)
+          --config-file <PATH>           Additional configuration files (can be repeated)
     ");
 }
 

docs/config.md

@@ -1203,9 +1203,9 @@ 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` 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-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
@@ -1221,3 +1221,9 @@ files with the config specified in `.jjconfig.toml`:
 ```shell
 jj --config-toml="$(cat extra-config.toml)" log
 ```
+
+This is equivalent to
+
+```shell
+jj --config-file=extra-config.toml log
+```

lib/src/config.rs

@@ -292,7 +292,8 @@ impl ConfigLayer {
         Ok(Self::with_data(source, data.into_mut()))
     }
 
-    fn load_from_file(source: ConfigSource, path: PathBuf) -> Result<Self, ConfigLoadError> {
+    /// Loads TOML file from the specified `path`.
+    pub fn load_from_file(source: ConfigSource, path: PathBuf) -> Result<Self, ConfigLoadError> {
         let text = fs::read_to_string(&path)
             .context(&path)
             .map_err(ConfigLoadError::Read)?;