config: move resolved_config_values() to free function

cli/src/cli_util.rs

@@ -58,7 +58,6 @@ use jj_lib::backend::MergedTreeId;
 use jj_lib::backend::TreeValue;
 use jj_lib::commit::Commit;
 use jj_lib::config::ConfigError;
-use jj_lib::config::ConfigNamePathBuf;
 use jj_lib::config::ConfigSource;
 use jj_lib::conflicts::ConflictMarkerStyle;
 use jj_lib::file_util;
@@ -149,9 +148,7 @@ use crate::commit_templater::CommitTemplateLanguage;
 use crate::commit_templater::CommitTemplateLanguageExtension;
 use crate::complete;
 use crate::config::new_config_path;
-use crate::config::AnnotatedValue;
 use crate::config::CommandNameAndArgs;
-use crate::config::ConfigEnvError;
 use crate::config::LayeredConfigs;
 use crate::diff_util;
 use crate::diff_util::DiffFormat;
@@ -319,11 +316,9 @@ impl CommandHelper {
         &self.data.settings
     }
 
-    pub fn resolved_config_values(
-        &self,
-        prefix: &ConfigNamePathBuf,
-    ) -> Result<Vec<AnnotatedValue>, ConfigEnvError> {
-        self.data.layered_configs.resolved_config_values(prefix)
+    // TODO: will be moved to UserSettings
+    pub fn layered_configs(&self) -> &LayeredConfigs {
+        &self.data.layered_configs
     }
 
     pub fn revset_extensions(&self) -> &Arc<RevsetExtensions> {

cli/src/commands/config/list.rs

@@ -21,6 +21,7 @@ use super::ConfigLevelArgs;
 use crate::cli_util::CommandHelper;
 use crate::command_error::CommandError;
 use crate::complete;
+use crate::config::resolved_config_values;
 use crate::config::to_toml_value;
 use crate::config::AnnotatedValue;
 use crate::generic_templater::GenericTemplateLanguage;
@@ -78,7 +79,7 @@ pub fn cmd_config_list(
     let mut formatter = ui.stdout_formatter();
     let name_path = args.name.clone().unwrap_or_else(ConfigNamePathBuf::root);
     let mut wrote_values = false;
-    for annotated in command.resolved_config_values(&name_path)? {
+    for annotated in resolved_config_values(command.layered_configs(), &name_path)? {
         // Remove overridden values.
         if annotated.is_overridden && !args.include_overridden {
             continue;
@@ -115,9 +116,8 @@ pub fn cmd_config_list(
 fn config_template_language() -> GenericTemplateLanguage<'static, AnnotatedValue> {
     type L = GenericTemplateLanguage<'static, AnnotatedValue>;
     let mut language = L::new();
-    // "name" instead of "path" to avoid confusion with the source file path
     language.add_keyword("name", |self_property| {
-        let out_property = self_property.map(|annotated| annotated.path.to_string());
+        let out_property = self_property.map(|annotated| annotated.name.to_string());
         Ok(L::wrap_string(out_property))
     });
     language.add_keyword("value", |self_property| {

cli/src/config.rs

@@ -87,11 +87,17 @@ pub enum ConfigEnvError {
     ConfigCreateError(#[from] std::io::Error),
 }
 
+/// Configuration variable with its source information.
 #[derive(Clone, Debug, PartialEq)]
 pub struct AnnotatedValue {
-    pub path: ConfigNamePathBuf,
+    /// Dotted name path to the configuration variable.
+    pub name: ConfigNamePathBuf,
+    /// Configuration value.
     pub value: config::Value,
+    /// Source of the configuration value.
     pub source: ConfigSource,
+    // TODO: add source file path
+    /// True if this value is overridden in higher precedence layers.
     pub is_overridden: bool,
 }
 
@@ -178,50 +184,52 @@ impl LayeredConfigs {
             .iter()
             .map(|layer| (layer.source, &layer.data))
     }
+}
 
-    pub fn resolved_config_values(
-        &self,
-        filter_prefix: &ConfigNamePathBuf,
-    ) -> Result<Vec<AnnotatedValue>, ConfigEnvError> {
-        // Collect annotated values from each config.
-        let mut config_vals = vec![];
-        for (source, config) in self.sources() {
-            let Some(top_value) = filter_prefix.lookup_value(config).optional()? else {
-                continue;
-            };
-            let mut config_stack = vec![(filter_prefix.clone(), &top_value)];
-            while let Some((path, value)) = config_stack.pop() {
-                match &value.kind {
-                    config::ValueKind::Table(table) => {
-                        // TODO: Remove sorting when config crate maintains deterministic ordering.
-                        for (k, v) in table.iter().sorted_by_key(|(k, _)| *k).rev() {
-                            let mut key_path = path.clone();
-                            key_path.push(k);
-                            config_stack.push((key_path, v));
-                        }
-                    }
-                    _ => {
-                        config_vals.push(AnnotatedValue {
-                            path,
-                            value: value.to_owned(),
-                            source,
-                            // Note: Value updated below.
-                            is_overridden: false,
-                        });
+/// Collects values under the given `filter_prefix` name recursively, from all
+/// layers.
+pub fn resolved_config_values(
+    layered_configs: &LayeredConfigs,
+    filter_prefix: &ConfigNamePathBuf,
+) -> Result<Vec<AnnotatedValue>, ConfigError> {
+    // Collect annotated values from each config.
+    let mut config_vals = vec![];
+    for (source, config) in layered_configs.sources() {
+        let Some(top_value) = filter_prefix.lookup_value(config).optional()? else {
+            continue;
+        };
+        let mut config_stack = vec![(filter_prefix.clone(), &top_value)];
+        while let Some((name, value)) = config_stack.pop() {
+            match &value.kind {
+                config::ValueKind::Table(table) => {
+                    // TODO: Remove sorting when config crate maintains deterministic ordering.
+                    for (k, v) in table.iter().sorted_by_key(|(k, _)| *k).rev() {
+                        let mut sub_name = name.clone();
+                        sub_name.push(k);
+                        config_stack.push((sub_name, v));
                     }
                 }
+                _ => {
+                    config_vals.push(AnnotatedValue {
+                        name,
+                        value: value.to_owned(),
+                        source,
+                        // Note: Value updated below.
+                        is_overridden: false,
+                    });
+                }
             }
         }
+    }
 
-        // Walk through config values in reverse order and mark each overridden value as
-        // overridden.
-        let mut keys_found = HashSet::new();
-        for val in config_vals.iter_mut().rev() {
-            val.is_overridden = !keys_found.insert(&val.path);
-        }
-
-        Ok(config_vals)
+    // Walk through config values in reverse order and mark each overridden value as
+    // overridden.
+    let mut names_found = HashSet::new();
+    for val in config_vals.iter_mut().rev() {
+        val.is_overridden = !names_found.insert(&val.name);
     }
+
+    Ok(config_vals)
 }
 
 enum ConfigPath {
@@ -732,20 +740,18 @@ mod tests {
     }
 
     #[test]
-    fn test_layered_configs_resolved_config_values_empty() {
+    fn test_resolved_config_values_empty() {
         let layered_configs = LayeredConfigs {
             inner: StackedConfig::empty(),
         };
         assert_eq!(
-            layered_configs
-                .resolved_config_values(&ConfigNamePathBuf::root())
-                .unwrap(),
+            resolved_config_values(&layered_configs, &ConfigNamePathBuf::root()).unwrap(),
             []
         );
     }
 
     #[test]
-    fn test_layered_configs_resolved_config_values_single_key() {
+    fn test_resolved_config_values_single_key() {
         let env_base_config = config::Config::builder()
             .set_override("user.name", "base-user-name")
             .unwrap()
@@ -767,11 +773,11 @@ mod tests {
         let layered_configs = LayeredConfigs { inner };
         // Note: "email" is alphabetized, before "name" from same layer.
         insta::assert_debug_snapshot!(
-            layered_configs.resolved_config_values(&ConfigNamePathBuf::root()).unwrap(),
+            resolved_config_values(&layered_configs, &ConfigNamePathBuf::root()).unwrap(),
             @r#"
         [
             AnnotatedValue {
-                path: ConfigNamePathBuf(
+                name: ConfigNamePathBuf(
                     [
                         Key {
                             key: "user",
@@ -809,7 +815,7 @@ mod tests {
                 is_overridden: true,
             },
             AnnotatedValue {
-                path: ConfigNamePathBuf(
+                name: ConfigNamePathBuf(
                     [
                         Key {
                             key: "user",
@@ -847,7 +853,7 @@ mod tests {
                 is_overridden: false,
             },
             AnnotatedValue {
-                path: ConfigNamePathBuf(
+                name: ConfigNamePathBuf(
                     [
                         Key {
                             key: "user",
@@ -890,7 +896,7 @@ mod tests {
     }
 
     #[test]
-    fn test_layered_configs_resolved_config_values_filter_path() {
+    fn test_resolved_config_values_filter_path() {
         let user_config = config::Config::builder()
             .set_override("test-table1.foo", "user-FOO")
             .unwrap()
@@ -908,13 +914,15 @@ mod tests {
         inner.add_layer(ConfigLayer::with_data(ConfigSource::Repo, repo_config));
         let layered_configs = LayeredConfigs { inner };
         insta::assert_debug_snapshot!(
-            layered_configs
-                .resolved_config_values(&ConfigNamePathBuf::from_iter(["test-table1"]))
-                .unwrap(),
+            resolved_config_values(
+                &layered_configs,
+                &ConfigNamePathBuf::from_iter(["test-table1"]),
+            )
+            .unwrap(),
             @r#"
         [
             AnnotatedValue {
-                path: ConfigNamePathBuf(
+                name: ConfigNamePathBuf(
                     [
                         Key {
                             key: "test-table1",
@@ -952,7 +960,7 @@ mod tests {
                 is_overridden: false,
             },
             AnnotatedValue {
-                path: ConfigNamePathBuf(
+                name: ConfigNamePathBuf(
                     [
                         Key {
                             key: "test-table1",