Skip to content

Commit

Permalink
config: introduce newtype for dotted config name path
Browse files Browse the repository at this point in the history 
"config list NAME" argument is now parsed as TOML key, but it's still broken
since config.get() expects a query expression in different syntax.

The other config commands will be migrated later.
  • Loading branch information
@yuja
yuja committed May 23, 2024
1 parent f65ba88 commit a127fd9
Show file tree
Hide file tree
Showing 4 changed files with 197 additions and 48 deletions.
5 changes: 3 additions & 2 deletions cli/src/cli_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ use crate::command_error::{
};
use crate::commit_templater::{CommitTemplateLanguage, CommitTemplateLanguageExtension};
use crate::config::{
new_config_path, AnnotatedValue, CommandNameAndArgs, ConfigSource, LayeredConfigs,
new_config_path, AnnotatedValue, CommandNameAndArgs, ConfigNamePathBuf, ConfigSource,
LayeredConfigs,
};
use crate::diff_util::{self, DiffFormat, DiffFormatArgs, DiffRenderer, DiffWorkspaceContext};
use crate::formatter::{FormatRecorder, Formatter, PlainTextFormatter};
Expand Down Expand Up @@ -234,7 +235,7 @@ impl CommandHelper {

pub fn resolved_config_values(
&self,
prefix: &[&str],
prefix: &ConfigNamePathBuf,
) -> Result<Vec<AnnotatedValue>, crate::config::ConfigError> {
self.layered_configs.resolved_config_values(prefix)
}
Expand Down
18 changes: 4 additions & 14 deletions cli/src/commands/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@
use std::fmt;
use std::io::Write;

use clap::builder::NonEmptyStringValueParser;
use itertools::Itertools;
use tracing::instrument;

use crate::cli_util::{get_new_config_file_path, run_ui_editor, CommandHelper};
use crate::command_error::{config_error, user_error, CommandError};
use crate::config::{write_config_value_to_file, AnnotatedValue, ConfigSource};
use crate::config::{write_config_value_to_file, AnnotatedValue, ConfigNamePathBuf, ConfigSource};
use crate::generic_templater::GenericTemplateLanguage;
use crate::template_builder::TemplateLanguage as _;
use crate::templater::TemplatePropertyExt as _;
Expand Down Expand Up @@ -81,8 +80,7 @@ pub(crate) enum ConfigCommand {
#[command(mut_group("config_level", |g| g.required(false)))]
pub(crate) struct ConfigListArgs {
/// An optional name of a specific config option to look up.
#[arg(value_parser = NonEmptyStringValueParser::new())]
pub name: Option<String>,
pub name: Option<ConfigNamePathBuf>,
/// Whether to explicitly include built-in default values in the list.
#[arg(long, conflicts_with = "config_level")]
pub include_defaults: bool,
Expand Down Expand Up @@ -168,10 +166,6 @@ pub(crate) fn cmd_config(
}
}

fn toml_escape_key(key: String) -> String {
toml_edit::Key::from(key).to_string()
}

fn to_toml_value(value: &config::Value) -> Result<toml_edit::Value, config::ConfigError> {
fn type_error<T: fmt::Display>(message: T) -> config::ConfigError {
config::ConfigError::Message(message.to_string())
Expand Down Expand Up @@ -205,8 +199,7 @@ fn config_template_language() -> 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.into_iter().map(toml_escape_key).join("."));
let out_property = self_property.map(|annotated| annotated.path.to_string());
Ok(L::wrap_string(out_property))
});
language.add_keyword("value", |self_property| {
Expand Down Expand Up @@ -244,10 +237,7 @@ pub(crate) fn cmd_config_list(

ui.request_pager();
let mut formatter = ui.stdout_formatter();
let name_path = args
.name
.as_ref()
.map_or(vec![], |name| name.split('.').collect_vec());
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)? {
// Remove overridden values.
Expand Down
205 changes: 173 additions & 32 deletions cli/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::borrow::Cow;
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use std::process::Command;
use std::str::FromStr;
use std::{env, fmt};

use config::Source;
Expand All @@ -36,6 +37,61 @@ pub enum ConfigError {
ConfigCreateError(#[from] std::io::Error),
}

/// Dotted config name path.
#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct ConfigNamePathBuf(Vec<toml_edit::Key>);

impl ConfigNamePathBuf {
/// Creates an empty path pointing to the root table.
///
/// This isn't a valid TOML key expression, but provided for convenience.
pub fn root() -> Self {
ConfigNamePathBuf(vec![])
}

/// Returns true if the path is empty (i.e. pointing to the root table.)
pub fn is_root(&self) -> bool {
self.0.is_empty()
}

/// Appends the given `key` component.
pub fn push(&mut self, key: impl Into<toml_edit::Key>) {
self.0.push(key.into());
}
}

impl<K: Into<toml_edit::Key>> FromIterator<K> for ConfigNamePathBuf {
fn from_iter<I: IntoIterator<Item = K>>(iter: I) -> Self {
let keys = iter.into_iter().map(|k| k.into()).collect();
ConfigNamePathBuf(keys)
}
}

impl FromStr for ConfigNamePathBuf {
type Err = toml_edit::TomlError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
// TOML parser ensures that the returned vec is not empty.
toml_edit::Key::parse(s).map(ConfigNamePathBuf)
}
}

impl AsRef<[toml_edit::Key]> for ConfigNamePathBuf {
fn as_ref(&self) -> &[toml_edit::Key] {
&self.0
}
}

impl fmt::Display for ConfigNamePathBuf {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut components = self.0.iter().fuse();
if let Some(key) = components.next() {
write!(f, "{key}")?;
}
components.try_for_each(|key| write!(f, ".{key}"))
}
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum ConfigSource {
Default,
Expand All @@ -48,7 +104,7 @@ pub enum ConfigSource {

#[derive(Clone, Debug, PartialEq)]
pub struct AnnotatedValue {
pub path: Vec<String>,
pub path: ConfigNamePathBuf,
pub value: config::Value,
pub source: ConfigSource,
pub is_overridden: bool,
Expand Down Expand Up @@ -141,17 +197,18 @@ impl LayeredConfigs {

pub fn resolved_config_values(
&self,
filter_prefix: &[&str],
filter_prefix: &ConfigNamePathBuf,
) -> Result<Vec<AnnotatedValue>, ConfigError> {
// Collect annotated values from each config.
let mut config_vals = vec![];

let prefix_key = match filter_prefix {
// TODO: don't reinterpret TOML path as config path expression
let prefix_key = match filter_prefix.as_ref() {
&[] => None,
_ => Some(filter_prefix.join(".")),
_ => Some(filter_prefix.to_string()),
};
for (source, config) in self.sources() {
let top_value = match prefix_key {
let top_value: config::Value = match prefix_key {
Some(ref key) => {
if let Some(val) = config.get(key).optional()? {
val
Expand All @@ -161,21 +218,20 @@ impl LayeredConfigs {
}
None => config.collect()?.into(),
};
let mut config_stack: Vec<(Vec<&str>, &config::Value)> =
vec![(filter_prefix.to_vec(), &top_value)];
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.to_vec();
let mut key_path = path.clone();
key_path.push(k);
config_stack.push((key_path, v));
}
}
_ => {
config_vals.push(AnnotatedValue {
path: path.iter().map(|&s| s.to_owned()).collect_vec(),
path,
value: value.to_owned(),
source: source.to_owned(),
// Note: Value updated below.
Expand Down Expand Up @@ -674,7 +730,12 @@ mod tests {
env_overrides: empty_config,
arg_overrides: None,
};
assert_eq!(layered_configs.resolved_config_values(&[]).unwrap(), []);
assert_eq!(
layered_configs
.resolved_config_values(&ConfigNamePathBuf::root())
.unwrap(),
[]
);
}

#[test]
Expand Down Expand Up @@ -702,14 +763,30 @@ mod tests {
};
// Note: "email" is alphabetized, before "name" from same layer.
insta::assert_debug_snapshot!(
layered_configs.resolved_config_values(&[]).unwrap(),
layered_configs.resolved_config_values(&ConfigNamePathBuf::root()).unwrap(),
@r###"
[
AnnotatedValue {
path: [
"user",
"email",
],
path: ConfigNamePathBuf(
[
Key {
key: "user",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
Key {
key: "email",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
],
),
value: Value {
origin: None,
kind: String(
Expand All @@ -720,10 +797,26 @@ mod tests {
is_overridden: true,
},
AnnotatedValue {
path: [
"user",
"name",
],
path: ConfigNamePathBuf(
[
Key {
key: "user",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
Key {
key: "name",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
],
),
value: Value {
origin: None,
kind: String(
Expand All @@ -734,10 +827,26 @@ mod tests {
is_overridden: false,
},
AnnotatedValue {
path: [
"user",
"email",
],
path: ConfigNamePathBuf(
[
Key {
key: "user",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
Key {
key: "email",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
],
),
value: Value {
origin: None,
kind: String(
Expand Down Expand Up @@ -777,15 +886,31 @@ mod tests {
};
insta::assert_debug_snapshot!(
layered_configs
.resolved_config_values(&["test-table1"])
.resolved_config_values(&ConfigNamePathBuf::from_iter(["test-table1"]))
.unwrap(),
@r###"
[
AnnotatedValue {
path: [
"test-table1",
"foo",
],
path: ConfigNamePathBuf(
[
Key {
key: "test-table1",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
Key {
key: "foo",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
],
),
value: Value {
origin: None,
kind: String(
Expand All @@ -796,10 +921,26 @@ mod tests {
is_overridden: false,
},
AnnotatedValue {
path: [
"test-table1",
"bar",
],
path: ConfigNamePathBuf(
[
Key {
key: "test-table1",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
Key {
key: "bar",
repr: None,
decor: Decor {
prefix: "default",
suffix: "default",
},
},
],
),
value: Value {
origin: None,
kind: String(
Expand Down
Loading

0 comments on commit a127fd9

Please sign in to comment.