Skip to content

Commit

Permalink
config: move resolved_config_values() to free function
Browse files Browse the repository at this point in the history
This will help replace cli LayeredConfigs with new StackedConfig. I originally
considered moving this function to jj-lib, but the current API is very specific
to the CLI use case, and wouldn't be reused for building a merged sub table. We
might instead want to extract some bits as a library function.
  • Loading branch information
yuja committed Nov 25, 2024
1 parent 5559b18 commit 75295b2
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 51 deletions.
6 changes: 2 additions & 4 deletions cli/src/commands/config/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -78,10 +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
.layered_configs()
.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;
Expand Down
96 changes: 49 additions & 47 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,50 +178,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((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,
});
}
}
}
}

// 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 keys_found = HashSet::new();
for val in config_vals.iter_mut().rev() {
val.is_overridden = !keys_found.insert(&val.path);
}

Ok(config_vals)
}

enum ConfigPath {
Expand Down Expand Up @@ -732,20 +734,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()
Expand All @@ -767,7 +767,7 @@ 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 {
Expand Down Expand Up @@ -890,7 +890,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()
Expand All @@ -908,9 +908,11 @@ 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 {
Expand Down

0 comments on commit 75295b2

Please sign in to comment.