cli: load revset/template aliases in order of config layers

Since we abuse TOML table syntax to define function aliases, an identical
function alias can be found more than once in the merged config. The merged
config doesn't preserve the definition order, so we need to load aliases
table per layer.

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<ReadonlyRepo>,
     ) -> Result<Self, CommandError> {
-        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<RevsetAliasesMap, CommandError> {
     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<TemplateAliasesMap, CommandError> {
     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)

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

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();

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();