From 41997292b82efdb8ee5bb654080b1692e48b72d3 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 17 Dec 2024 16:53:38 +0900 Subject: [PATCH] cli: resolve conditional config scopes This is an alternative way to achieve includeIf of Git without adding "include" directive. Conditional include (or include in general) is a bit trickier to implement than loading all files and filtering the contents. Closes #616 --- CHANGELOG.md | 3 ++ cli/src/cli_util.rs | 6 +-- cli/src/complete.rs | 6 ++- cli/src/config.rs | 25 +++++++++++- cli/tests/test_config_command.rs | 70 ++++++++++++++++++++++++++++++++ cli/tests/test_global_opts.rs | 51 +++++++++++++++++++++++ docs/config.md | 24 +++++++++++ 7 files changed, 178 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c94e2cbe4c..09adc28ae3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,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. +* Configuration files now support [conditional + variables](docs/config.md#conditional-variables). + * New command options `--config=NAME=VALUE` and `--config-file=PATH` to set string value without quoting and to load additional configuration from files. diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 49c5f1ee30..b5aa55ee94 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -3650,7 +3650,7 @@ impl CliRunner { config_env.reset_repo_path(loader.repo_path()); config_env.reload_repo_config(&mut raw_config)?; } - let mut config = raw_config.as_ref().clone(); // TODO + let mut config = config_env.resolve_config(&raw_config)?; ui.reset(&config)?; if env::var_os("COMPLETE").is_some() { @@ -3661,7 +3661,7 @@ impl CliRunner { let (args, config_layers) = parse_early_args(&self.app, &string_args)?; if !config_layers.is_empty() { raw_config.as_mut().extend_layers(config_layers); - config = raw_config.as_ref().clone(); // TODO + config = config_env.resolve_config(&raw_config)?; ui.reset(&config)?; } if !args.config_toml.is_empty() { @@ -3687,7 +3687,7 @@ impl CliRunner { .map_err(|err| map_workspace_load_error(err, Some(path)))?; config_env.reset_repo_path(loader.repo_path()); config_env.reload_repo_config(&mut raw_config)?; - config = raw_config.as_ref().clone(); // TODO + config = config_env.resolve_config(&raw_config)?; Ok(loader) } else { maybe_cwd_workspace_loader diff --git a/cli/src/complete.rs b/cli/src/complete.rs index e92578b9a4..c10278a59f 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -693,7 +693,7 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { config_env.reset_repo_path(loader.repo_path()); let _ = config_env.reload_repo_config(&mut raw_config); } - let mut config = raw_config.as_ref().clone(); /* TODO */ + let mut config = config_env.resolve_config(&raw_config)?; // skip 2 because of the clap_complete prelude: jj -- jj let args = std::env::args_os().skip(2); let args = expand_args(&ui, &app, args, &config)?; @@ -710,7 +710,9 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { if let Ok(loader) = DefaultWorkspaceLoaderFactory.create(&cwd.join(&repository)) { config_env.reset_repo_path(loader.repo_path()); let _ = config_env.reload_repo_config(&mut raw_config); - config = raw_config.as_ref().clone(); // TODO + if let Ok(new_config) = config_env.resolve_config(&raw_config) { + config = new_config; + } } cmd_args.push("--repository".into()); cmd_args.push(repository); diff --git a/cli/src/config.rs b/cli/src/config.rs index 1a83e3ebab..c9881b4b56 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -23,9 +23,11 @@ use std::process::Command; use itertools::Itertools; use jj_lib::config::ConfigFile; +use jj_lib::config::ConfigGetError; use jj_lib::config::ConfigLayer; use jj_lib::config::ConfigLoadError; use jj_lib::config::ConfigNamePathBuf; +use jj_lib::config::ConfigResolutionContext; use jj_lib::config::ConfigSource; use jj_lib::config::ConfigValue; use jj_lib::config::StackedConfig; @@ -247,6 +249,8 @@ impl UnresolvedConfigEnv { #[derive(Clone, Debug)] pub struct ConfigEnv { + home_dir: Option, + repo_path: Option, user_config_path: ConfigPath, repo_config_path: ConfigPath, } @@ -254,12 +258,15 @@ pub struct ConfigEnv { impl ConfigEnv { /// Initializes configuration loader based on environment variables. pub fn from_environment() -> Result { + let home_dir = dirs::home_dir(); let env = UnresolvedConfigEnv { config_dir: dirs::config_dir(), - home_dir: dirs::home_dir(), + home_dir: home_dir.clone(), jj_config: env::var("JJ_CONFIG").ok(), }; Ok(ConfigEnv { + home_dir, + repo_path: None, user_config_path: env.resolve()?, repo_config_path: ConfigPath::Unavailable, }) @@ -324,6 +331,7 @@ impl ConfigEnv { /// Sets the directory where repo-specific config file is stored. The path /// is usually `.jj/repo`. pub fn reset_repo_path(&mut self, path: &Path) { + self.repo_path = Some(path.to_owned()); self.repo_config_path = ConfigPath::new(Some(path.join("config.toml"))); } @@ -371,6 +379,16 @@ impl ConfigEnv { } Ok(()) } + + /// Resolves conditional scopes within the current environment. Returns new + /// resolved config. + pub fn resolve_config(&self, config: &RawConfig) -> Result { + let context = ConfigResolutionContext { + home_dir: self.home_dir.as_deref(), + repo_path: self.repo_path.as_deref(), + }; + jj_lib::config::resolve(config.as_ref(), &context) + } } fn config_files_for( @@ -1188,9 +1206,10 @@ mod tests { impl TestCase { fn resolve(&self, root: &Path) -> Result { + let home_dir = self.env.home_dir.as_ref().map(|p| root.join(p)); let env = UnresolvedConfigEnv { config_dir: self.env.config_dir.as_ref().map(|p| root.join(p)), - home_dir: self.env.home_dir.as_ref().map(|p| root.join(p)), + home_dir: home_dir.clone(), jj_config: self .env .jj_config @@ -1198,6 +1217,8 @@ mod tests { .map(|p| root.join(p).to_str().unwrap().to_string()), }; Ok(ConfigEnv { + home_dir, + repo_path: None, user_config_path: env.resolve()?, repo_config_path: ConfigPath::Unavailable, }) diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index 566da658db..79bbf214d3 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -1047,6 +1047,76 @@ fn test_config_path_syntax() { "###); } +#[test] +fn test_config_conditional() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.home_dir(), &["git", "init", "repo1"]); + test_env.jj_cmd_ok(test_env.home_dir(), &["git", "init", "repo2"]); + let repo1_path = test_env.home_dir().join("repo1"); + let repo2_path = test_env.home_dir().join("repo2"); + // Test with fresh new config file + let user_config_path = test_env.env_root().join("config.toml"); + test_env.set_config_path(user_config_path.clone()); + std::fs::write( + &user_config_path, + indoc! {" + foo = 'global' + [[--scope]] + --when.repositories = ['~/repo1'] + foo = 'repo1' + [[--scope]] + --when.repositories = ['~/repo2'] + foo = 'repo2' + "}, + ) + .unwrap(); + + // get and list should refer to the resolved config + let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "get", "foo"]); + insta::assert_snapshot!(stdout, @"global"); + let stdout = test_env.jj_cmd_success(&repo1_path, &["config", "get", "foo"]); + insta::assert_snapshot!(stdout, @"repo1"); + let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "--user"]); + insta::assert_snapshot!(stdout, @"foo = 'global'"); + let stdout = test_env.jj_cmd_success(&repo1_path, &["config", "list", "--user"]); + insta::assert_snapshot!(stdout, @"foo = 'repo1'"); + let stdout = test_env.jj_cmd_success(&repo2_path, &["config", "list", "--user"]); + insta::assert_snapshot!(stdout, @"foo = 'repo2'"); + + // relative workspace path + let stdout = test_env.jj_cmd_success(&repo2_path, &["config", "list", "--user", "-R../repo1"]); + insta::assert_snapshot!(stdout, @"foo = 'repo1'"); + + // set and unset should refer to the source config + // (there's no option to update scoped table right now.) + let (_stdout, stderr) = test_env.jj_cmd_ok( + test_env.env_root(), + &["config", "set", "--user", "bar", "new value"], + ); + insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(std::fs::read_to_string(&user_config_path).unwrap(), @r#" + foo = 'global' + bar = "new value" + [[--scope]] + --when.repositories = ['~/repo1'] + foo = 'repo1' + [[--scope]] + --when.repositories = ['~/repo2'] + foo = 'repo2' + "#); + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo1_path, &["config", "unset", "--user", "foo"]); + insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(std::fs::read_to_string(&user_config_path).unwrap(), @r#" + bar = "new value" + [[--scope]] + --when.repositories = ['~/repo1'] + foo = 'repo1' + [[--scope]] + --when.repositories = ['~/repo2'] + foo = 'repo2' + "#); +} + #[test] fn test_config_show_paths() { let test_env = TestEnvironment::default(); diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index cc6ffa3ae4..f21d209e4d 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -721,6 +721,57 @@ fn test_invalid_config_value() { "); } +#[test] +fn test_conditional_config() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.home_dir(), &["git", "init", "repo1"]); + test_env.jj_cmd_ok(test_env.home_dir(), &["git", "init", "repo2"]); + test_env.add_config(indoc! {" + aliases.foo = ['new', 'root()', '-mglobal'] + [[--scope]] + --when.repositories = ['~'] + aliases.foo = ['new', 'root()', '-mhome'] + [[--scope]] + --when.repositories = ['~/repo1'] + aliases.foo = ['new', 'root()', '-mrepo1'] + "}); + + let stdout = test_env.jj_cmd_success( + test_env.env_root(), + &["config", "list", "--include-overridden", "aliases"], + ); + insta::assert_snapshot!(stdout, @"aliases.foo = ['new', 'root()', '-mglobal']"); + let stdout = test_env.jj_cmd_success( + &test_env.home_dir().join("repo1"), + &["config", "list", "--include-overridden", "aliases"], + ); + insta::assert_snapshot!(stdout, @r" + # aliases.foo = ['new', 'root()', '-mglobal'] + # aliases.foo = ['new', 'root()', '-mhome'] + aliases.foo = ['new', 'root()', '-mrepo1'] + "); + let stdout = test_env.jj_cmd_success( + &test_env.home_dir().join("repo2"), + &["config", "list", "--include-overridden", "aliases"], + ); + insta::assert_snapshot!(stdout, @r" + # aliases.foo = ['new', 'root()', '-mglobal'] + aliases.foo = ['new', 'root()', '-mhome'] + "); + + // Aliases can be expanded by using the conditional tables + let (_stdout, stderr) = test_env.jj_cmd_ok(&test_env.home_dir().join("repo1"), &["foo"]); + insta::assert_snapshot!(stderr, @r" + Working copy now at: royxmykx 82899b03 (empty) repo1 + Parent commit : zzzzzzzz 00000000 (empty) (no description set) + "); + let (_stdout, stderr) = test_env.jj_cmd_ok(&test_env.home_dir().join("repo2"), &["foo"]); + insta::assert_snapshot!(stderr, @r" + Working copy now at: yqosqzyt 3bd315a9 (empty) home + Parent commit : zzzzzzzz 00000000 (empty) (no description set) + "); +} + #[test] fn test_no_user_configured() { // Test that the user is reminded if they haven't configured their name or email diff --git a/docs/config.md b/docs/config.md index be9292d3d9..6ca5e62461 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1219,3 +1219,27 @@ To load an entire TOML document, use `--config-file`: ```shell jj --config-file=extra-config.toml log ``` + +### Conditional variables + +You can conditionally enable config variables by using `--when` and +`[[--scope]]` tables. Variables defined in the `[[--scope]]` table are expanded +to the root table. `--when` specifies the condition to enable the scope table. + +```toml +[user] +name = "YOUR NAME" +email = "YOUR_DEFAULT_EMAIL@example.com" + +# override user.email if the repository is located under ~/oss +[[--scope]] +--when.repositories = ["~/oss"] +[--scope.user] +email = "YOUR_OSS_EMAIL@example.org" +``` + +Condition keys: + +* `--when.repositories`: List of paths to match the repository path prefix. + +If no condition is specified, the table is always enabled.