config: add workaround for config path expression parsing

As of config 0.13.4, the path Expression type is private, and there's no escape
syntax. This patch adds a fallback to nested HashMap<String, Value> lookup.

https://github.com/mehcode/config-rs/blob/v0.13.4/src/path/mod.rs#L10
https://github.com/mehcode/config-rs/blob/v0.13.4/src/path/parser.rs

Fixes #1723
This commit is contained in:
Yuya Nishihara 2024-05-22 15:17:50 +09:00
parent a127fd9c5d
commit 82b0e88a21
2 changed files with 159 additions and 15 deletions

View file

@ -58,6 +58,51 @@ impl ConfigNamePathBuf {
pub fn push(&mut self, key: impl Into<toml_edit::Key>) {
self.0.push(key.into());
}
/// Looks up value in the given `config`.
///
/// This is a workaround for the `config.get()` API, which doesn't support
/// literal path expression. If we implement our own config abstraction,
/// this method should be moved there.
pub fn lookup_value(
&self,
config: &config::Config,
) -> Result<config::Value, config::ConfigError> {
// Use config.get() if the TOML keys can be converted to config path
// syntax. This should be cheaper than cloning the whole config map.
let (key_prefix, components) = self.split_safe_prefix();
let value: config::Value = match &key_prefix {
Some(key) => config.get(key)?,
None => config.collect()?.into(),
};
components
.iter()
.try_fold(value, |value, key| {
let mut table = value.into_table().ok()?;
table.remove(key.get())
})
.ok_or_else(|| config::ConfigError::NotFound(self.to_string()))
}
/// Splits path to dotted literal expression and remainder.
///
/// The literal expression part doesn't contain meta characters other than
/// ".", therefore it can be passed in to `config.get()`.
/// https://github.com/mehcode/config-rs/issues/110
fn split_safe_prefix(&self) -> (Option<Cow<'_, str>>, &[toml_edit::Key]) {
// https://github.com/mehcode/config-rs/blob/v0.13.4/src/path/parser.rs#L15
let is_ident = |key: &str| {
key.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
};
let pos = self.0.iter().take_while(|&k| is_ident(k)).count();
let safe_key = match pos {
0 => None,
1 => Some(Cow::Borrowed(self.0[0].get())),
_ => Some(Cow::Owned(self.0[..pos].iter().join("."))),
};
(safe_key, &self.0[pos..])
}
}
impl<K: Into<toml_edit::Key>> FromIterator<K> for ConfigNamePathBuf {
@ -201,22 +246,9 @@ impl LayeredConfigs {
) -> Result<Vec<AnnotatedValue>, ConfigError> {
// Collect annotated values from each config.
let mut config_vals = vec![];
// TODO: don't reinterpret TOML path as config path expression
let prefix_key = match filter_prefix.as_ref() {
&[] => None,
_ => Some(filter_prefix.to_string()),
};
for (source, config) in self.sources() {
let top_value: config::Value = match prefix_key {
Some(ref key) => {
if let Some(val) = config.get(key).optional()? {
val
} else {
continue;
}
}
None => config.collect()?.into(),
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() {
@ -655,6 +687,43 @@ mod tests {
use super::*;
#[test]
fn test_split_safe_config_name_path() {
let parse = |s| ConfigNamePathBuf::from_str(s).unwrap();
let key = |s: &str| toml_edit::Key::new(s);
// Empty (or root) path isn't recognized by config::Config::get()
assert_eq!(
ConfigNamePathBuf::root().split_safe_prefix(),
(None, [].as_slice())
);
assert_eq!(
parse("Foo-bar_1").split_safe_prefix(),
(Some("Foo-bar_1".into()), [].as_slice())
);
assert_eq!(
parse("'foo()'").split_safe_prefix(),
(None, [key("foo()")].as_slice())
);
assert_eq!(
parse("foo.'bar()'").split_safe_prefix(),
(Some("foo".into()), [key("bar()")].as_slice())
);
assert_eq!(
parse("foo.'bar()'.baz").split_safe_prefix(),
(Some("foo".into()), [key("bar()"), key("baz")].as_slice())
);
assert_eq!(
parse("foo.bar").split_safe_prefix(),
(Some("foo.bar".into()), [].as_slice())
);
assert_eq!(
parse("foo.bar.'baz()'").split_safe_prefix(),
(Some("foo.bar".into()), [key("baz()")].as_slice())
);
}
#[test]
fn test_command_args() {
let config = config::Config::builder()

View file

@ -713,7 +713,82 @@ fn test_config_get() {
#[test]
fn test_config_path_syntax() {
let test_env = TestEnvironment::default();
test_env.add_config(
r#"
a.'b()' = 0
'b c'.d = 1
'b c'.e.'f[]' = 2
- = 3
_ = 4
'.' = 5
"#,
);
let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "a.'b()'"]);
insta::assert_snapshot!(stdout, @r###"
a.'b()'=0
"###);
let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "'b c'"]);
insta::assert_snapshot!(stdout, @r###"
'b c'.d=1
'b c'.e."f[]"=2
"###);
let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "'b c'.d"]);
insta::assert_snapshot!(stdout, @r###"
'b c'.d=1
"###);
let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "'b c'.e.'f[]'"]);
insta::assert_snapshot!(stdout, @r###"
'b c'.e.'f[]'=2
"###);
// Not a table
let (stdout, stderr) =
test_env.jj_cmd_ok(test_env.env_root(), &["config", "list", "a.'b()'.x"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Warning: No matching config key for a.'b()'.x
"###);
// "-" and "_" are valid TOML keys
let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "-"]);
insta::assert_snapshot!(stdout, @r###"
-=3
"###);
let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "_"]);
insta::assert_snapshot!(stdout, @r###"
_=4
"###);
// "." requires quoting
let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "'.'"]);
insta::assert_snapshot!(stdout, @r###"
'.'=5
"###);
let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["config", "list", "."]);
insta::assert_snapshot!(stderr, @r###"
error: invalid value '.' for '[NAME]': TOML parse error at line 1, column 1
|
1 | .
| ^
invalid key
For more information, try '--help'.
"###);
// Invalid TOML keys
let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["config", "list", "b c"]);
insta::assert_snapshot!(stderr, @r###"
error: invalid value 'b c' for '[NAME]': TOML parse error at line 1, column 3
|
1 | b c
| ^
For more information, try '--help'.
"###);
let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["config", "list", ""]);
insta::assert_snapshot!(stderr, @r###"
error: invalid value '' for '[NAME]': TOML parse error at line 1, column 1