cli: add --config-file=PATH argument
Some checks are pending
binaries / Build binary artifacts (push) Waiting to run
nix / flake check (push) Waiting to run
build / build (, macos-13) (push) Waiting to run
build / build (, macos-14) (push) Waiting to run
build / build (, ubuntu-latest) (push) Waiting to run
build / build (, windows-latest) (push) Waiting to run
build / build (--all-features, ubuntu-latest) (push) Waiting to run
build / Build jj-lib without Git support (push) Waiting to run
build / Check protos (push) Waiting to run
build / Check formatting (push) Waiting to run
build / Check that MkDocs can build the docs (push) Waiting to run
build / Check that MkDocs can build the docs with latest Python and uv (push) Waiting to run
build / cargo-deny (advisories) (push) Waiting to run
build / cargo-deny (bans licenses sources) (push) Waiting to run
build / Clippy check (push) Waiting to run
Codespell / Codespell (push) Waiting to run
website / prerelease-docs-build-deploy (ubuntu-latest) (push) Waiting to run
Scorecards supply-chain security / Scorecards analysis (push) Waiting to run

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>.

https://github.com/martinvonz/jj/issues/4926#issuecomment-2506672165

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.
This commit is contained in:
Yuya Nishihara 2024-12-11 12:33:59 +09:00
parent c29b5a2aa3
commit fca92f1e16
8 changed files with 188 additions and 8 deletions

View file

@ -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.

View file

@ -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)]
);
}
}

View file

@ -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()
}

View file

@ -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)

View file

@ -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')
");

View file

@ -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)
");
}

View file

@ -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
```

View file

@ -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)?;