diff --git a/src/cli_util.rs b/src/cli_util.rs index 8e3a71888..e6e70cd5b 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -50,7 +50,7 @@ use jujutsu_lib::revset::{ RevsetIteratorExt, RevsetParseError, RevsetParseErrorKind, RevsetResolutionError, RevsetWorkspaceContext, }; -use jujutsu_lib::settings::UserSettings; +use jujutsu_lib::settings::{ConfigResultExt as _, UserSettings}; use jujutsu_lib::transaction::Transaction; use jujutsu_lib::tree::{Tree, TreeMergeError}; use jujutsu_lib::working_copy::{ @@ -579,8 +579,8 @@ impl WorkspaceCommandHelper { workspace: Workspace, repo: Arc, ) -> Result { - let revset_aliases_map = load_revset_aliases(ui, &command.settings)?; - let template_aliases_map = load_template_aliases(ui, &command.settings)?; + let revset_aliases_map = load_revset_aliases(ui, &command.layered_configs)?; + let template_aliases_map = load_template_aliases(ui, &command.layered_configs)?; // Parse commit_summary template early to report error before starting mutable // operation. // TODO: Parsed template can be cached if it doesn't capture repo @@ -1626,18 +1626,26 @@ fn resolve_single_op_from_store( fn load_revset_aliases( ui: &mut Ui, - settings: &UserSettings, + layered_configs: &LayeredConfigs, ) -> Result { const TABLE_KEY: &str = "revset-aliases"; let mut aliases_map = RevsetAliasesMap::new(); - let table = settings.config().get_table(TABLE_KEY)?; - for (decl, value) in table.into_iter().sorted_by(|a, b| a.0.cmp(&b.0)) { - let r = value - .into_string() - .map_err(|e| e.to_string()) - .and_then(|v| aliases_map.insert(&decl, v).map_err(|e| e.to_string())); - if let Err(s) = r { - writeln!(ui.warning(), r#"Failed to load "{TABLE_KEY}.{decl}": {s}"#)?; + // Load from all config layers in order. 'f(x)' in default layer should be + // overridden by 'f(a)' in user. + for (_, config) in layered_configs.sources() { + let table = if let Some(table) = config.get_table(TABLE_KEY).optional()? { + table + } else { + continue; + }; + for (decl, value) in table.into_iter().sorted_by(|a, b| a.0.cmp(&b.0)) { + let r = value + .into_string() + .map_err(|e| e.to_string()) + .and_then(|v| aliases_map.insert(&decl, v).map_err(|e| e.to_string())); + if let Err(s) = r { + writeln!(ui.warning(), r#"Failed to load "{TABLE_KEY}.{decl}": {s}"#)?; + } } } Ok(aliases_map) @@ -1733,18 +1741,26 @@ pub fn update_working_copy( fn load_template_aliases( ui: &mut Ui, - settings: &UserSettings, + layered_configs: &LayeredConfigs, ) -> Result { const TABLE_KEY: &str = "template-aliases"; let mut aliases_map = TemplateAliasesMap::new(); - let table = settings.config().get_table(TABLE_KEY)?; - for (decl, value) in table.into_iter().sorted_by(|a, b| a.0.cmp(&b.0)) { - let r = value - .into_string() - .map_err(|e| e.to_string()) - .and_then(|v| aliases_map.insert(&decl, v).map_err(|e| e.to_string())); - if let Err(s) = r { - writeln!(ui.warning(), r#"Failed to load "{TABLE_KEY}.{decl}": {s}"#)?; + // Load from all config layers in order. 'f(x)' in default layer should be + // overridden by 'f(a)' in user. + for (_, config) in layered_configs.sources() { + let table = if let Some(table) = config.get_table(TABLE_KEY).optional()? { + table + } else { + continue; + }; + for (decl, value) in table.into_iter().sorted_by(|a, b| a.0.cmp(&b.0)) { + let r = value + .into_string() + .map_err(|e| e.to_string()) + .and_then(|v| aliases_map.insert(&decl, v).map_err(|e| e.to_string())); + if let Err(s) = r { + writeln!(ui.warning(), r#"Failed to load "{TABLE_KEY}.{decl}": {s}"#)?; + } } } Ok(aliases_map) diff --git a/src/config.rs b/src/config.rs index 27d5b6c0a..148d1d18d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -117,7 +117,7 @@ impl LayeredConfigs { .expect("loaded configs should be merged without error") } - fn sources(&self) -> Vec<(ConfigSource, &config::Config)> { + pub fn sources(&self) -> Vec<(ConfigSource, &config::Config)> { let config_sources = [ (ConfigSource::Default, Some(&self.default)), (ConfigSource::Env, Some(&self.env_base)), diff --git a/tests/test_revset_output.rs b/tests/test_revset_output.rs index 0da5fae02..a3e9d792b 100644 --- a/tests/test_revset_output.rs +++ b/tests/test_revset_output.rs @@ -337,6 +337,36 @@ fn test_alias() { "###); } +#[test] +fn test_alias_override() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.add_config( + r###" + [revset-aliases] + 'f(x)' = 'user' + "###, + ); + + // 'f(x)' should be overridden by --config-toml 'f(a)'. If aliases were sorted + // purely by name, 'f(a)' would come first. + let stderr = test_env.jj_cmd_failure( + &repo_path, + &[ + "log", + "-r", + "f(_)", + "--config-toml", + "revset-aliases.'f(a)' = 'arg'", + ], + ); + insta::assert_snapshot!(stderr, @r###" + Error: Revision "arg" doesn't exist + "###); +} + #[test] fn test_bad_alias_decl() { let test_env = TestEnvironment::default(); diff --git a/tests/test_templater.rs b/tests/test_templater.rs index fe76fad8c..93e155d25 100644 --- a/tests/test_templater.rs +++ b/tests/test_templater.rs @@ -808,6 +808,36 @@ fn test_templater_alias() { "###); } +#[test] +fn test_templater_alias_override() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.add_config( + r###" + [template-aliases] + 'f(x)' = '"user"' + "###, + ); + + // 'f(x)' should be overridden by --config-toml 'f(a)'. If aliases were sorted + // purely by name, 'f(a)' would come first. + let stdout = test_env.jj_cmd_success( + &repo_path, + &[ + "log", + "--no-graph", + "-r@", + "-T", + r#"f(_)"#, + "--config-toml", + r#"template-aliases.'f(a)' = '"arg"'"#, + ], + ); + insta::assert_snapshot!(stdout, @"arg"); +} + #[test] fn test_templater_bad_alias_decl() { let test_env = TestEnvironment::default();