config: migrate underlying data structure to toml_edit

This patch does not change the handling of inline tables yet. Both inline and
non-inline tables are merged as before. OTOH, .set_value() is strict about table
types because it should refuse to overwrite a table whereas an inline table
should be overwritten as a value. This matches "jj config set"/"unset"
semantics. rules_from_config() in formatter.rs uses .as_inline_table(), which is
valid because toml_edit::Value type never contains non-inline table.

Since toml_edit::Value doesn't implement PartialEq, stacking tests now use
insta::assert_snapshot!().
This commit is contained in:
Yuya Nishihara 2024-11-29 14:44:28 +09:00
parent 45c80c2a5f
commit e2be4fa1ac
15 changed files with 489 additions and 346 deletions

View file

@ -12,6 +12,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
### Breaking changes
* Configuration variables are no longer "stringly" typed. For example, `true` is
not converted to a string `"true"`, and vice versa.
* The following configuration variables are now parsed strictly:
`ui.progress-indicator`, `ui.quiet`

View file

@ -2694,22 +2694,19 @@ fn load_template_aliases(
Ok(Some(table)) => table,
Ok(None) => continue,
Err(item) => {
// TODO: rewrite error construction after migrating to toml_edit
let error = item.clone().into_table().unwrap_err();
return Err(ConfigGetError::Type {
name: table_name.to_string(),
error: error.into(),
error: format!("Expected a table, but is {}", item.type_name()).into(),
source_path: layer.path.clone(),
}
.into());
}
};
// TODO: remove sorting after migrating to toml_edit
for (decl, value) in table.iter().sorted_by_key(|&(decl, _)| decl) {
let r = value
.clone()
.into_string()
.map_err(|e| e.to_string())
for (decl, item) in table.iter().sorted_by_key(|&(decl, _)| decl) {
let r = item
.as_str()
.ok_or_else(|| format!("Expected a string, but is {}", item.type_name()))
.and_then(|v| aliases_map.insert(decl, v).map_err(|e| e.to_string()));
if let Err(s) = r {
writeln!(

View file

@ -13,12 +13,10 @@
// limitations under the License.
use std::io::Write as _;
use std::path::PathBuf;
use clap_complete::ArgValueCandidates;
use jj_lib::config::ConfigError;
use jj_lib::config::ConfigGetError;
use jj_lib::config::ConfigNamePathBuf;
use jj_lib::config::ConfigValue;
use tracing::instrument;
use crate::cli_util::CommandHelper;
@ -48,21 +46,24 @@ pub fn cmd_config_get(
command: &CommandHelper,
args: &ConfigGetArgs,
) -> Result<(), CommandError> {
let value = command.settings().get_value(&args.name)?;
let stringified = value.into_string().map_err(|err| -> CommandError {
match err {
ConfigError::Type {
origin, unexpected, ..
} => ConfigGetError::Type {
name: args.name.to_string(),
error: format!("Expected a value convertible to a string, but is {unexpected}")
.into(),
source_path: origin.map(PathBuf::from),
let stringified = command
.settings()
.get_value_with(&args.name, |value| match value {
// Remove extra formatting from a string value
ConfigValue::String(v) => Ok(v.into_value()),
// Print other values in TOML syntax (but whitespace trimmed)
ConfigValue::Integer(_)
| ConfigValue::Float(_)
| ConfigValue::Boolean(_)
| ConfigValue::Datetime(_) => Ok(value.decorated("", "").to_string()),
// TODO: maybe okay to just print array or table in TOML syntax?
ConfigValue::Array(_) => {
Err("Expected a value convertible to a string, but is an array")
}
.into(),
err => err.into(),
}
})?;
ConfigValue::InlineTable(_) => {
Err("Expected a value convertible to a string, but is a table")
}
})?;
writeln!(ui.stdout(), "{stringified}")?;
Ok(())
}

View file

@ -22,7 +22,6 @@ 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;
use crate::template_builder::TemplateLanguage as _;
@ -122,8 +121,9 @@ fn config_template_language() -> GenericTemplateLanguage<'static, AnnotatedValue
});
language.add_keyword("value", |self_property| {
// TODO: would be nice if we can provide raw dynamically-typed value
// .decorated("", "") to trim leading/trailing whitespace
let out_property =
self_property.and_then(|annotated| Ok(to_toml_value(&annotated.value)?.to_string()));
self_property.map(|annotated| annotated.value.decorated("", "").to_string());
Ok(L::wrap_string(out_property))
});
language.add_keyword("overridden", |self_property| {

View file

@ -52,7 +52,6 @@ use crate::command_error::config_error;
use crate::command_error::print_parse_diagnostics;
use crate::command_error::CommandError;
use crate::complete;
use crate::config::to_toml_value;
use crate::config::CommandNameAndArgs;
use crate::ui::Ui;
@ -470,7 +469,10 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
command = {}
patterns = ["all()"]
"###,
to_toml_value(&settings.get_value("fix.tool-command").unwrap()).unwrap()
settings
.get_value("fix.tool-command")
.unwrap()
.decorated("", "") // trim whitespace
)?;
}
let tools: Vec<ToolConfig> = settings

View file

@ -51,32 +51,6 @@ pub fn parse_toml_value_or_bare_string(value_str: &str) -> toml_edit::Value {
}
}
pub fn to_toml_value(value: &ConfigValue) -> Result<toml_edit::Value, ConfigError> {
fn type_error<T: fmt::Display>(message: T) -> ConfigError {
ConfigError::Message(message.to_string())
}
// It's unlikely that the config object contained unsupported values, but
// there's no guarantee. For example, values coming from environment
// variables might be big int.
match value.kind {
config::ValueKind::Nil => Err(type_error(format!("Unexpected value: {value}"))),
config::ValueKind::Boolean(v) => Ok(v.into()),
config::ValueKind::I64(v) => Ok(v.into()),
config::ValueKind::I128(v) => Ok(i64::try_from(v).map_err(type_error)?.into()),
config::ValueKind::U64(v) => Ok(i64::try_from(v).map_err(type_error)?.into()),
config::ValueKind::U128(v) => Ok(i64::try_from(v).map_err(type_error)?.into()),
config::ValueKind::Float(v) => Ok(v.into()),
config::ValueKind::String(ref v) => Ok(v.into()),
// TODO: Remove sorting when config crate maintains deterministic ordering.
config::ValueKind::Table(ref table) => table
.iter()
.sorted_by_key(|(k, _)| *k)
.map(|(k, v)| Ok((k, to_toml_value(v)?)))
.collect(),
config::ValueKind::Array(ref array) => array.iter().map(to_toml_value).collect(),
}
}
#[derive(Error, Debug)]
pub enum ConfigEnvError {
#[error(transparent)]
@ -118,24 +92,30 @@ pub fn resolved_config_values(
};
let mut config_stack = vec![(filter_prefix.clone(), top_item)];
while let Some((name, item)) = config_stack.pop() {
match &item.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: item.to_owned(),
source: layer.source,
// Note: Value updated below.
is_overridden: false,
});
// TODO: item.as_table() to print inline table as a value?
if let Some(table) = item.as_table_like() {
// table.iter() does not implement DoubleEndedIterator as of
// toml_edit 0.22.22.
let frame = config_stack.len();
// TODO: Remove sorting
for (k, v) in table.iter().sorted_by_key(|(k, _)| *k) {
let mut sub_name = name.clone();
sub_name.push(k);
config_stack.push((sub_name, v));
}
config_stack[frame..].reverse();
} else {
let value = item
.clone()
.into_value()
.expect("Item::None should not exist in table");
config_vals.push(AnnotatedValue {
name,
value,
source: layer.source,
// Note: Value updated below.
is_overridden: false,
});
}
}
}
@ -798,12 +778,13 @@ mod tests {
},
],
),
value: Value {
origin: None,
kind: String(
"base@user.email",
),
},
value: String(
Formatted {
value: "base@user.email",
repr: "default",
decor: Decor { .. },
},
),
source: EnvBase,
is_overridden: true,
},
@ -824,12 +805,13 @@ mod tests {
},
],
),
value: Value {
origin: None,
kind: String(
"base-user-name",
),
},
value: String(
Formatted {
value: "base-user-name",
repr: "default",
decor: Decor { .. },
},
),
source: EnvBase,
is_overridden: false,
},
@ -850,12 +832,13 @@ mod tests {
},
],
),
value: Value {
origin: None,
kind: String(
"repo@user.email",
),
},
value: String(
Formatted {
value: "repo@user.email",
repr: "default",
decor: Decor { .. },
},
),
source: Repo,
is_overridden: false,
},
@ -897,12 +880,13 @@ mod tests {
},
],
),
value: Value {
origin: None,
kind: String(
"user-FOO",
),
},
value: String(
Formatted {
value: "user-FOO",
repr: "default",
decor: Decor { .. },
},
),
source: User,
is_overridden: false,
},
@ -923,12 +907,13 @@ mod tests {
},
],
),
value: Value {
origin: None,
kind: String(
"repo-BAR",
),
},
value: String(
Formatted {
value: "repo-BAR",
repr: "default",
decor: Decor { .. },
},
),
source: Repo,
is_overridden: false,
},

View file

@ -417,44 +417,41 @@ fn rules_from_config(config: &StackedConfig) -> Result<Rules, ConfigGetError> {
.split_whitespace()
.map(ToString::to_string)
.collect_vec();
// TODO: report invalid type?
let value = config.get_value(["colors", key])?;
match value.kind {
config::ValueKind::String(color_name) => {
let style = Style {
fg_color: Some(color_for_name_or_hex(&color_name).map_err(to_config_err)?),
bg_color: None,
bold: None,
underlined: None,
};
result.push((labels, style));
if let Some(color_name) = value.as_str() {
let style = Style {
fg_color: Some(color_for_name_or_hex(color_name).map_err(to_config_err)?),
bg_color: None,
bold: None,
underlined: None,
};
result.push((labels, style));
} else if let Some(style_table) = value.as_inline_table() {
let mut style = Style::default();
if let Some(item) = style_table.get("fg") {
if let Some(color_name) = item.as_str() {
style.fg_color =
Some(color_for_name_or_hex(color_name).map_err(to_config_err)?);
}
}
config::ValueKind::Table(style_table) => {
let mut style = Style::default();
if let Some(value) = style_table.get("fg") {
if let config::ValueKind::String(color_name) = &value.kind {
style.fg_color =
Some(color_for_name_or_hex(color_name).map_err(to_config_err)?);
}
if let Some(item) = style_table.get("bg") {
if let Some(color_name) = item.as_str() {
style.bg_color =
Some(color_for_name_or_hex(color_name).map_err(to_config_err)?);
}
if let Some(value) = style_table.get("bg") {
if let config::ValueKind::String(color_name) = &value.kind {
style.bg_color =
Some(color_for_name_or_hex(color_name).map_err(to_config_err)?);
}
}
if let Some(value) = style_table.get("bold") {
if let config::ValueKind::Boolean(value) = &value.kind {
style.bold = Some(*value);
}
}
if let Some(value) = style_table.get("underline") {
if let config::ValueKind::Boolean(value) = &value.kind {
style.underlined = Some(*value);
}
}
result.push((labels, style));
}
_ => {}
if let Some(item) = style_table.get("bold") {
if let Some(value) = item.as_bool() {
style.bold = Some(value);
}
}
if let Some(item) = style_table.get("underline") {
if let Some(value) = item.as_bool() {
style.underlined = Some(value);
}
}
result.push((labels, style));
}
}
Ok(result)

View file

@ -176,26 +176,22 @@ pub fn load_revset_aliases(
Ok(Some(table)) => table,
Ok(None) => continue,
Err(item) => {
// TODO: rewrite error construction after migrating to toml_edit
let error = item.clone().into_table().unwrap_err();
return Err(ConfigGetError::Type {
name: table_name.to_string(),
error: error.into(),
error: format!("Expected a table, but is {}", item.type_name()).into(),
source_path: layer.path.clone(),
}
.into());
}
};
// TODO: remove sorting after migrating to toml_edit
for (decl, value) in table.iter().sorted_by_key(|&(decl, _)| decl) {
for (decl, item) in table.iter().sorted_by_key(|&(decl, _)| decl) {
warn_user_redefined_builtin(ui, layer.source, decl)?;
let r = value
.clone()
.into_string()
.map_err(|e| e.to_string())
let r = item
.as_str()
.ok_or_else(|| format!("Expected a string, but is {}", item.type_name()))
.and_then(|v| aliases_map.insert(decl, v).map_err(|e| e.to_string()));
if let Err(s) = r {
writeln!(
ui.warning_default(),

View file

@ -230,20 +230,22 @@ fn test_alias_invalid_definition() {
test_env.add_config(
r#"[aliases]
non-list = 5
non-string-list = [[]]
non-string-list = [0]
"#,
);
let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["non-list"]);
insta::assert_snapshot!(stderr.replace('\\', "/"), @r"
Config error: Invalid type or value for aliases.non-list
Caused by: invalid type: integer `5`, expected a sequence
Hint: Check the config file: $TEST_ENV/config/config0002.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
");
let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["non-string-list"]);
insta::assert_snapshot!(stderr.replace('\\', "/"), @r"
insta::assert_snapshot!(stderr, @r"
Config error: Invalid type or value for aliases.non-string-list
Caused by: invalid type: sequence, expected a string for key `[0]` in config/config0002.toml
Caused by: invalid type: integer `0`, expected a string
Hint: Check the config file: $TEST_ENV/config/config0002.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
");
@ -318,7 +320,5 @@ fn test_alias_in_repo_config() {
repo2_path.to_str().unwrap(),
],
);
insta::assert_snapshot!(stdout, @r###"
aliases.l = ["log", "-r@", "--no-graph", '-T"user alias\n"']
"###);
insta::assert_snapshot!(stdout, @r#"aliases.l = ['log', '-r@', '--no-graph', '-T"user alias\n"']"#);
}

View file

@ -153,12 +153,12 @@ bar
);
let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "multiline"]);
insta::assert_snapshot!(stdout, @r###"
multiline = """
insta::assert_snapshot!(stdout, @r"
multiline = '''
foo
bar
"""
"###);
'''
");
let stdout = test_env.jj_cmd_success(
test_env.env_root(),
@ -170,13 +170,13 @@ bar
"--config-toml=multiline='single'",
],
);
insta::assert_snapshot!(stdout, @r###"
# multiline = """
insta::assert_snapshot!(stdout, @r"
# multiline = '''
# foo
# bar
# """
multiline = "single"
"###);
# '''
multiline = 'single'
");
}
#[test]
@ -836,17 +836,18 @@ fn test_config_get() {
"###);
let stdout = test_env.jj_cmd_failure(test_env.env_root(), &["config", "get", "table.list"]);
insta::assert_snapshot!(stdout.replace('\\', "/"), @r"
insta::assert_snapshot!(stdout, @r"
Config error: Invalid type or value for table.list
Caused by: Expected a value convertible to a string, but is sequence
Hint: Check the config file: config/config0002.toml
Caused by: Expected a value convertible to a string, but is an array
Hint: Check the config file: $TEST_ENV/config/config0002.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
");
let stdout = test_env.jj_cmd_failure(test_env.env_root(), &["config", "get", "table"]);
insta::assert_snapshot!(stdout, @r"
Config error: Invalid type or value for table
Caused by: Expected a value convertible to a string, but is map
Caused by: Expected a value convertible to a string, but is a table
Hint: Check the config file: $TEST_ENV/config/config0003.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
");
@ -975,7 +976,8 @@ fn test_config_show_paths() {
let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["st"]);
insta::assert_snapshot!(stderr, @r"
Config error: Invalid type or value for ui.paginate
Caused by: enum PaginationChoice does not have variant constructor :builtin
Caused by: unknown variant `:builtin`, expected `never` or `auto`
Hint: Check the config file: $TEST_ENV/config/config.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
");

View file

@ -45,11 +45,7 @@ fn init_with_fake_formatter(args: &[&str]) -> (TestEnvironment, PathBuf, impl Fn
// When the test runs on Windows, backslashes in the path complicate things by
// changing the double quotes to single quotes in the serialized TOML.
snapshot.replace(
&if cfg!(windows) {
format!(r#"'{}'"#, formatter_path.to_str().unwrap())
} else {
format!(r#""{}""#, formatter_path.to_str().unwrap())
},
&format!("'{}'", formatter_path.to_str().unwrap()),
"<redacted formatter path>",
)
})
@ -174,16 +170,16 @@ fn test_config_multiple_tools_with_same_name() {
std::fs::write(repo_path.join("bar"), "Bar\n").unwrap();
let stderr = test_env.jj_cmd_failure(&repo_path, &["fix"]);
#[cfg(unix)]
insta::assert_snapshot!(stderr, @r###"
Config error: redefinition of table `fix.tools.my-tool` for key `fix.tools.my-tool` at line 6 column 9 in ../config/config0002.toml
insta::assert_snapshot!(stderr, @r"
Config error: TOML parse error at line 6, column 9
|
6 | [fix.tools.my-tool]
| ^
invalid table header
duplicate key `my-tool` in table `fix.tools`
in $TEST_ENV/config/config0002.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
"###);
#[cfg(windows)]
insta::assert_snapshot!(stderr, @r###"
Config error: redefinition of table `fix.tools.my-tool` for key `fix.tools.my-tool` at line 6 column 9 in ..\config\config0002.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
"###);
");
test_env.set_config_path("/dev/null".into());
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]);
@ -259,6 +255,7 @@ fn test_config_tables_all_commands_missing() {
insta::assert_snapshot!(stderr.replace('\\', "/"), @r"
Config error: Invalid type or value for fix.tools.my-tool-missing-command-1
Caused by: missing field `command`
Hint: Check the config file: $TEST_ENV/config/config0002.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
");
@ -293,6 +290,7 @@ fn test_config_tables_some_commands_missing() {
insta::assert_snapshot!(stderr.replace('\\', "/"), @r"
Config error: Invalid type or value for fix.tools.my-tool-missing-command
Caused by: missing field `command`
Hint: Check the config file: $TEST_ENV/config/config0002.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
");
@ -428,16 +426,16 @@ fn test_fix_empty_commit() {
let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
"#);
}
#[test]
@ -449,18 +447,18 @@ fn test_fix_leaf_commit() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
Working copy now at: rlvkpnrz 85ce8924 (no description set)
Parent commit : qpvuntsm b2ca2bc5 (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@-"]);
insta::assert_snapshot!(content, @"unaffected");
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
@ -482,18 +480,18 @@ fn test_fix_parent_commit() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "parent"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 3 commits of 3 checked.
Working copy now at: mzvwutvl d30c8ae2 child2 | (no description set)
Parent commit : qpvuntsm 70a4dae2 parent | (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "parent"]);
insta::assert_snapshot!(content, @"PARENT");
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "child1"]);
@ -516,15 +514,15 @@ fn test_fix_sibling_commit() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "child1"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "parent"]);
insta::assert_snapshot!(content, @"parent");
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "child1"]);
@ -561,18 +559,18 @@ fn test_default_revset() {
test_env.add_config(r#"revset-aliases."immutable_heads()" = "trunk2""#);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 3 commits of 3 checked.
Working copy now at: yostqsxw dabc47b2 bar2 | (no description set)
Parent commit : yqosqzyt 984b5924 bar1 | (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "trunk1"]);
insta::assert_snapshot!(content, @"trunk1");
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "trunk2"]);
@ -604,15 +602,15 @@ fn test_custom_default_revset() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "foo"]);
insta::assert_snapshot!(content, @"foo");
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "bar"]);
@ -634,7 +632,7 @@ fn test_fix_immutable_commit() {
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Error: Commit e4b41a3ce243 is immutable
@ -654,16 +652,16 @@ fn test_fix_empty_file() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"");
}
@ -676,18 +674,18 @@ fn test_fix_some_paths() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@", "file1"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm 54a90d2b (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file1"]);
insta::assert_snapshot!(content, @r###"
FOO
@ -703,35 +701,35 @@ fn test_fix_cyclic() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--reverse"]
command = [<redacted formatter path>, '--reverse']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm bf5e6a5a (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"tnetnoc\n");
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--reverse"]
command = [<redacted formatter path>, '--reverse']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm 0e2d20d6 (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"content\n");
}
@ -760,18 +758,18 @@ fn test_deduplication() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase", "--tee", "$path-fixlog"]
command = [<redacted formatter path>, '--uppercase', '--tee', '$path-fixlog']
patterns = ["all()"]
Fixed 4 commits of 4 checked.
Working copy now at: yqosqzyt cf770245 d | (no description set)
Parent commit : mzvwutvl 370615a5 c | (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "a"]);
insta::assert_snapshot!(content, @"FOO\n");
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "b"]);
@ -806,16 +804,16 @@ fn test_executed_but_nothing_changed() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--tee", "$path-copy"]
command = [<redacted formatter path>, '--tee', '$path-copy']
patterns = ["all()"]
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"content\n");
let copy_content = std::fs::read_to_string(repo_path.join("file-copy").as_os_str()).unwrap();
@ -829,16 +827,16 @@ fn test_failure() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--fail"]
command = [<redacted formatter path>, '--fail']
patterns = ["all()"]
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"content");
}
@ -853,18 +851,18 @@ fn test_stderr_success() {
// of passing it through directly.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--stderr", "error", "--stdout", "new content"]
command = [<redacted formatter path>, '--stderr', 'error', '--stdout', 'new content']
patterns = ["all()"]
errorFixed 1 commits of 1 checked.
Working copy now at: qpvuntsm 487808ba (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"new content");
}
@ -877,16 +875,16 @@ fn test_stderr_failure() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--stderr", "error", "--stdout", "new content", "--fail"]
command = [<redacted formatter path>, '--stderr', 'error', '--stdout', 'new content', '--fail']
patterns = ["all()"]
errorFixed 0 commits of 1 checked.
Nothing changed.
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"old content");
}
@ -923,18 +921,18 @@ fn test_fix_file_types() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm 6836a9e4 (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"CONTENT");
}
@ -951,18 +949,18 @@ fn test_fix_executable() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm fee78e99 (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"CONTENT");
let executable = std::fs::metadata(&path).unwrap().permissions().mode() & 0o111;
@ -985,16 +983,16 @@ fn test_fix_trivial_merge_commit() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file_a", "-r", "@"]);
insta::assert_snapshot!(content, @"content a");
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file_b", "-r", "@"]);
@ -1023,11 +1021,11 @@ fn test_fix_adding_merge_commit() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 1 commits of 1 checked.
@ -1035,7 +1033,7 @@ fn test_fix_adding_merge_commit() {
Parent commit : qpvuntsm 6e64e7a7 a | (no description set)
Parent commit : kkmpptxz c536f264 b | (no description set)
Added 0 files, modified 4 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file_a", "-r", "@"]);
insta::assert_snapshot!(content, @"CHANGE A");
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file_b", "-r", "@"]);
@ -1060,11 +1058,11 @@ fn test_fix_both_sides_of_conflict() {
// fixed if we didn't fix the parents also.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a", "-s", "b"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 3 commits of 3 checked.
@ -1074,7 +1072,7 @@ fn test_fix_both_sides_of_conflict() {
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "a"]);
insta::assert_snapshot!(content, @r###"
CONTENT A
@ -1110,11 +1108,11 @@ fn test_fix_resolve_conflict() {
// fixed if we didn't fix the parents also.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a", "-s", "b"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(redact(&stderr), @r###"
insta::assert_snapshot!(redact(&stderr), @r#"
Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version.
Hint: Replace it with the following:
[fix.tools.legacy-tool-command]
command = [<redacted formatter path>, "--uppercase"]
command = [<redacted formatter path>, '--uppercase']
patterns = ["all()"]
Fixed 3 commits of 3 checked.
@ -1122,7 +1120,7 @@ fn test_fix_resolve_conflict() {
Parent commit : qpvuntsm dd2721f1 a | (no description set)
Parent commit : kkmpptxz 07c27a8e b | (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
"#);
let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]);
insta::assert_snapshot!(content, @r###"
CONTENT

View file

@ -586,10 +586,16 @@ fn test_invalid_config() {
test_env.add_config("[section]key = value-missing-quotes");
let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["init", "repo"]);
insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
Config error: expected newline, found an identifier at line 1 column 10 in config/config0002.toml
insta::assert_snapshot!(stderr, @r"
Config error: TOML parse error at line 1, column 10
|
1 | [section]key = value-missing-quotes
| ^
invalid table header
expected newline, `#`
in $TEST_ENV/config/config0002.toml
For help, see https://martinvonz.github.io/jj/latest/config/.
"###);
");
}
#[test]
@ -606,6 +612,7 @@ fn test_invalid_config_value() {
insta::assert_snapshot!(stderr, @r"
Config error: Invalid type or value for snapshot.auto-track
Caused by: invalid type: sequence, expected a string
For help, see https://martinvonz.github.io/jj/latest/config/.
");
}

View file

@ -1326,7 +1326,8 @@ fn test_graph_styles() {
);
insta::assert_snapshot!(stderr, @r"
Config error: Invalid type or value for ui.graph.style
Caused by: enum GraphStyle does not have variant constructor unknown
Caused by: unknown variant `unknown`, expected one of `ascii`, `ascii-large`, `curved`, `square`
For help, see https://martinvonz.github.io/jj/latest/config/.
");
}

View file

@ -17,7 +17,7 @@
use std::borrow::Borrow;
use std::convert::Infallible;
use std::fmt;
use std::mem;
use std::fs;
use std::ops::Range;
use std::path::Path;
use std::path::PathBuf;
@ -25,15 +25,20 @@ use std::slice;
use std::str::FromStr;
use itertools::Itertools as _;
use serde::de::IntoDeserializer as _;
use serde::Deserialize;
use thiserror::Error;
use toml_edit::DocumentMut;
use toml_edit::ImDocument;
use crate::file_util::IoResultExt as _;
/// Config value or table node.
pub type ConfigItem = toml_edit::Item;
/// Table of config key and value pairs.
pub type ConfigTable = config::Map<String, config::Value>;
pub type ConfigTable = toml_edit::Table;
/// Generic config value.
pub type ConfigValue = config::Value;
pub type ConfigValue = toml_edit::Value;
/// Error that can occur when accessing configuration.
// TODO: will be replaced with our custom error type
@ -63,8 +68,20 @@ pub enum ConfigGetError {
/// Error that can occur when updating config variable.
#[derive(Debug, Error)]
#[error(transparent)]
pub struct ConfigUpdateError(pub ConfigError);
pub enum ConfigUpdateError {
/// Non-table value exists at parent path, which shouldn't be removed.
#[error("Would overwrite non-table value with parent table {name}")]
WouldOverwriteValue {
/// Dotted config name path.
name: String,
},
/// Table exists at the path, which shouldn't be overwritten by a value.
#[error("Would overwrite entire table {name}")]
WouldOverwriteTable {
/// Dotted config name path.
name: String,
},
}
/// Extension methods for `Result<T, ConfigGetError>`.
pub trait ConfigGetResultExt<T> {
@ -234,17 +251,17 @@ pub struct ConfigLayer {
/// Source file path of this layer if any.
pub path: Option<PathBuf>,
/// Configuration variables.
pub data: config::Config,
pub data: DocumentMut,
}
impl ConfigLayer {
/// Creates new layer with empty data.
pub fn empty(source: ConfigSource) -> Self {
Self::with_data(source, config::Config::default())
Self::with_data(source, DocumentMut::new())
}
/// Creates new layer with the configuration variables `data`.
pub fn with_data(source: ConfigSource, data: config::Config) -> Self {
pub fn with_data(source: ConfigSource, data: DocumentMut) -> Self {
ConfigLayer {
source,
path: None,
@ -254,27 +271,26 @@ impl ConfigLayer {
/// Parses TOML document `text` into new layer.
pub fn parse(source: ConfigSource, text: &str) -> Result<Self, ConfigError> {
let data = config::Config::builder()
.add_source(config::File::from_str(text, config::FileFormat::Toml))
.build()?;
Ok(Self::with_data(source, data))
let data = ImDocument::parse(text).map_err(|err| ConfigError::FileParse {
uri: None,
cause: err.into(),
})?;
Ok(Self::with_data(source, data.into_mut()))
}
fn load_from_file(source: ConfigSource, path: PathBuf) -> Result<Self, ConfigError> {
// TODO: will be replaced with toml_edit::DocumentMut or ImDocument
let data = config::Config::builder()
.add_source(
config::File::from(path.clone())
// TODO: The path should exist, but the config crate refuses
// to read a special file (e.g. /dev/null) as TOML.
.required(false)
.format(config::FileFormat::Toml),
)
.build()?;
let text = fs::read_to_string(&path).map_err(|err| ConfigError::FileParse {
uri: Some(path.to_string_lossy().into_owned()),
cause: err.into(),
})?;
let data = ImDocument::parse(text).map_err(|err| ConfigError::FileParse {
uri: Some(path.to_string_lossy().into_owned()),
cause: err.into(),
})?;
Ok(ConfigLayer {
source,
path: Some(path),
data,
data: data.into_mut(),
})
}
@ -303,14 +319,16 @@ impl ConfigLayer {
/// Looks up sub table by the `name` path. Returns `Some(table)` if a table
/// was found at the path. Returns `Err(item)` if middle or leaf node wasn't
/// a table.
// TODO: Perhaps, an inline table will be processed as a value, and this
// function will return &ConfigTable.
pub fn look_up_table(
&self,
name: impl ToConfigNamePath,
) -> Result<Option<&ConfigTable>, &config::Value> {
) -> Result<Option<&dyn toml_edit::TableLike>, &ConfigItem> {
match self.look_up_item(name) {
Ok(Some(item)) => match &item.kind {
config::ValueKind::Table(table) => Ok(Some(table)),
_ => Err(item),
Ok(Some(item)) => match item.as_table_like() {
Some(table) => Ok(Some(table)),
None => Err(item),
},
Ok(None) => Ok(None),
Err(item) => Err(item),
@ -322,35 +340,53 @@ impl ConfigLayer {
pub fn look_up_item(
&self,
name: impl ToConfigNamePath,
) -> Result<Option<&ConfigValue>, &ConfigValue> {
look_up_item(&self.data.cache, name.into_name_path().borrow())
) -> Result<Option<&ConfigItem>, &ConfigItem> {
look_up_item(self.data.as_item(), name.into_name_path().borrow())
}
/// Sets new `value` to the `name` path.
/// Sets `new_value` to the `name` path. Returns old value if any.
///
/// This function errors out if attempted to overwrite a non-table middle
/// node or a leaf table (in the same way as file/directory operation.)
pub fn set_value(
&mut self,
name: &str, // TODO: impl ToConfigNamePath
value: impl Into<ConfigValue>,
) -> Result<(), ConfigUpdateError> {
self.data = config::Config::builder()
.add_source(mem::take(&mut self.data))
.set_override(name, value)
.map_err(ConfigUpdateError)?
.build()
.map_err(ConfigUpdateError)?;
Ok(())
name: impl ToConfigNamePath,
new_value: impl Into<ConfigValue>,
) -> Result<Option<ConfigValue>, ConfigUpdateError> {
let name = name.into_name_path();
let name = name.borrow();
let (parent_table, leaf_key) = ensure_parent_table(self.data.as_table_mut(), name)
.map_err(|keys| ConfigUpdateError::WouldOverwriteValue {
name: keys.join("."),
})?;
match parent_table.entry(leaf_key) {
toml_edit::Entry::Occupied(mut entry) => {
if !entry.get().is_value() {
return Err(ConfigUpdateError::WouldOverwriteTable {
name: name.to_string(),
});
}
let old_item = entry.insert(toml_edit::value(new_value));
Ok(Some(old_item.into_value().unwrap()))
}
toml_edit::Entry::Vacant(entry) => {
entry.insert(toml_edit::value(new_value));
Ok(None)
}
}
}
}
/// Looks up item from the `root_item`. Returns `Some(item)` if an item found at
/// the path. Returns `Err(item)` if middle node wasn't a table.
fn look_up_item<'a>(
root_item: &'a ConfigValue,
root_item: &'a ConfigItem,
name: &ConfigNamePathBuf,
) -> Result<Option<&'a ConfigValue>, &'a ConfigValue> {
) -> Result<Option<&'a ConfigItem>, &'a ConfigItem> {
let mut cur_item = root_item;
for key in name.components().map(toml_edit::Key::get) {
let config::ValueKind::Table(table) = &cur_item.kind else {
for key in name.components() {
// TODO: Should inline tables be addressed by dotted name path?
let Some(table) = cur_item.as_table_like() else {
return Err(cur_item);
};
cur_item = match table.get(key) {
@ -361,6 +397,21 @@ fn look_up_item<'a>(
Ok(Some(cur_item))
}
/// Inserts tables down to the parent of the `name` path. Returns `Err(keys)` if
/// middle node exists at the prefix name `keys` and wasn't a table.
fn ensure_parent_table<'a, 'b>(
root_table: &'a mut ConfigTable,
name: &'b ConfigNamePathBuf,
) -> Result<(&'a mut ConfigTable, &'b toml_edit::Key), &'b [toml_edit::Key]> {
let mut keys = name.components();
let leaf_key = keys.next_back().ok_or(&name.0[..])?;
let parent_table = keys.enumerate().try_fold(root_table, |table, (i, key)| {
let sub_item = table.entry(key).or_insert_with(toml_edit::table);
sub_item.as_table_mut().ok_or(&name.0[..=i])
})?;
Ok((parent_table, leaf_key))
}
/// Stack of configuration layers which can be merged as needed.
#[derive(Clone, Debug)]
pub struct StackedConfig {
@ -454,12 +505,12 @@ impl StackedConfig {
&self,
name: impl ToConfigNamePath,
) -> Result<T, ConfigGetError> {
self.get_item_with(name, T::deserialize)
self.get_value_with(name, |value| T::deserialize(value.into_deserializer()))
}
/// Looks up value from all layers, merges sub fields as needed.
pub fn get_value(&self, name: impl ToConfigNamePath) -> Result<ConfigValue, ConfigGetError> {
self.get_item_with::<_, Infallible>(name, Ok)
self.get_value_with::<_, Infallible>(name, Ok)
}
/// Looks up value from all layers, merges sub fields as needed, then
@ -469,8 +520,15 @@ impl StackedConfig {
name: impl ToConfigNamePath,
convert: impl FnOnce(ConfigValue) -> Result<T, E>,
) -> Result<T, ConfigGetError> {
// TODO: Item will be a different type than Value
self.get_item_with(name, convert)
self.get_item_with(name, |item| {
// Item variants other than Item::None can be converted to a Value,
// and Item::None is not a valid TOML type. See also the following
// thread: https://github.com/toml-rs/toml/issues/299
let value = item
.into_value()
.expect("Item::None should not exist in loaded tables");
convert(value)
})
}
/// Looks up sub table from all layers, merges fields as needed.
@ -478,13 +536,18 @@ impl StackedConfig {
/// Use `table_keys(prefix)` and `get([prefix, key])` instead if table
/// values have to be converted to non-generic value type.
pub fn get_table(&self, name: impl ToConfigNamePath) -> Result<ConfigTable, ConfigGetError> {
self.get(name)
self.get_item_with(name, |item| {
// TODO: Should inline table be converted to non-inline table?
// .into_table() does this conversion.
item.into_table()
.map_err(|item| format!("Expected a table, but is {}", item.type_name()))
})
}
fn get_item_with<T, E: Into<Box<dyn std::error::Error + Send + Sync>>>(
&self,
name: impl ToConfigNamePath,
convert: impl FnOnce(ConfigValue) -> Result<T, E>,
convert: impl FnOnce(ConfigItem) -> Result<T, E>,
) -> Result<T, ConfigGetError> {
let name = name.into_name_path();
let name = name.borrow();
@ -513,8 +576,7 @@ impl StackedConfig {
to_merge
.into_iter()
.rev()
// TODO: remove sorting after migrating to toml_edit
.flat_map(|table| table.keys().sorted_unstable().map(|k| k.as_ref()))
.flat_map(|table| table.iter().map(|(k, _)| k))
.unique()
}
}
@ -524,7 +586,7 @@ impl StackedConfig {
fn get_merged_item(
layers: &[ConfigLayer],
name: &ConfigNamePathBuf,
) -> Option<(ConfigValue, usize)> {
) -> Option<(ConfigItem, usize)> {
let mut to_merge = Vec::new();
for (index, layer) in layers.iter().enumerate().rev() {
let item = match layer.look_up_item(name) {
@ -532,7 +594,8 @@ fn get_merged_item(
Ok(None) => continue, // parent is a table, but no value found
Err(_) => break, // parent is not a table, shadows lower layers
};
if matches!(item.kind, config::ValueKind::Table(_)) {
// TODO: handle non-inline and inline tables differently?
if item.is_table_like() {
to_merge.push((item, index));
} else if to_merge.is_empty() {
return Some((item.clone(), index)); // no need to allocate vec
@ -557,7 +620,7 @@ fn get_merged_item(
fn get_tables_to_merge<'a>(
layers: &'a [ConfigLayer],
name: &ConfigNamePathBuf,
) -> Vec<&'a ConfigTable> {
) -> Vec<&'a dyn toml_edit::TableLike> {
let mut to_merge = Vec::new();
for layer in layers.iter().rev() {
match layer.look_up_table(name) {
@ -570,22 +633,26 @@ fn get_tables_to_merge<'a>(
}
/// Merges `upper_item` fields into `lower_item` recursively.
fn merge_items(lower_item: &mut ConfigValue, upper_item: &ConfigValue) {
// TODO: If we switch to toml_edit, inline table will probably be treated as
// a value, not a table to be merged. For example, { fg = "red" } won't
// inherit other color parameters from the lower table.
let (config::ValueKind::Table(lower_table), config::ValueKind::Table(upper_table)) =
(&mut lower_item.kind, &upper_item.kind)
fn merge_items(lower_item: &mut ConfigItem, upper_item: &ConfigItem) {
// TODO: Inline table will probably be treated as a value, not a table to be
// merged. For example, { fg = "red" } won't inherit other color parameters
// from the lower table.
let (Some(lower_table), Some(upper_table)) =
(lower_item.as_table_like_mut(), upper_item.as_table_like())
else {
// Not a table, the upper item wins.
*lower_item = upper_item.clone();
return;
};
for (key, upper) in upper_table {
lower_table
.entry(key.clone())
.and_modify(|lower| merge_items(lower, upper))
.or_insert_with(|| upper.clone());
for (key, upper) in upper_table.iter() {
match lower_table.entry(key) {
toml_edit::Entry::Occupied(entry) => {
merge_items(entry.into_mut(), upper);
}
toml_edit::Entry::Vacant(entry) => {
entry.insert(upper.clone());
}
};
}
}
@ -597,9 +664,61 @@ mod tests {
use super::*;
#[test]
fn test_config_layer_set_value() {
let mut layer = ConfigLayer::empty(ConfigSource::User);
// Cannot overwrite the root table
assert_matches!(
layer.set_value(ConfigNamePathBuf::root(), 0),
Err(ConfigUpdateError::WouldOverwriteValue { name }) if name.is_empty()
);
// Insert some values
layer.set_value("foo", 1).unwrap();
layer.set_value("bar.baz.blah", "2").unwrap();
layer
.set_value("bar.qux", ConfigValue::from_iter([("inline", "table")]))
.unwrap();
insta::assert_snapshot!(layer.data, @r#"
foo = 1
[bar]
qux = { inline = "table" }
[bar.baz]
blah = "2"
"#);
// Can overwrite value
layer
.set_value("foo", ConfigValue::from_iter(["new", "foo"]))
.unwrap();
// Can overwrite inline table
layer.set_value("bar.qux", "new bar.qux").unwrap();
// Cannot overwrite table
assert_matches!(
layer.set_value("bar", 0),
Err(ConfigUpdateError::WouldOverwriteTable { name }) if name == "bar"
);
// Cannot overwrite value by table
assert_matches!(
layer.set_value("bar.baz.blah.blah", 0),
Err(ConfigUpdateError::WouldOverwriteValue { name }) if name == "bar.baz.blah"
);
insta::assert_snapshot!(layer.data, @r#"
foo = ["new", "foo"]
[bar]
qux = "new bar.qux"
[bar.baz]
blah = "2"
"#);
}
#[test]
fn test_stacked_config_layer_order() {
let empty_data = || config::Config::builder().build().unwrap();
let empty_data = || DocumentMut::new();
let layer_sources = |config: &StackedConfig| {
config
.layers()
@ -686,10 +805,6 @@ mod tests {
ConfigLayer::parse(ConfigSource::User, text).unwrap()
}
fn parse_to_table(text: &str) -> ConfigTable {
new_user_layer(text).data.cache.into_table().unwrap()
}
#[test]
fn test_stacked_config_get_simple_value() {
let mut config = StackedConfig::empty();
@ -726,6 +841,57 @@ mod tests {
);
}
#[test]
fn test_stacked_config_get_table_as_value() {
let mut config = StackedConfig::empty();
config.add_layer(new_user_layer(indoc! {"
a.b = { c = 'a.b.c #0' }
"}));
config.add_layer(new_user_layer(indoc! {"
a.d = ['a.d #1']
"}));
// Table can be converted to a value (so it can be deserialized to a
// structured value.)
insta::assert_snapshot!(
config.get_value("a").unwrap(),
@"{ b = { c = 'a.b.c #0' }, d = ['a.d #1'] }");
}
#[test]
fn test_stacked_config_get_inline_table() {
let mut config = StackedConfig::empty();
config.add_layer(new_user_layer(indoc! {"
a.b = { c = 'a.b.c #0' }
"}));
config.add_layer(new_user_layer(indoc! {"
a.b = { d = 'a.b.d #1' }
"}));
// Inline tables are merged. This might change later.
insta::assert_snapshot!(
config.get_value("a.b").unwrap(),
@" { c = 'a.b.c #0' , d = 'a.b.d #1' }");
}
#[test]
fn test_stacked_config_get_inline_non_inline_table() {
let mut config = StackedConfig::empty();
config.add_layer(new_user_layer(indoc! {"
a.b = { c = 'a.b.c #0' }
"}));
config.add_layer(new_user_layer(indoc! {"
a.b.d = 'a.b.d #1'
"}));
insta::assert_snapshot!(
config.get_value("a.b").unwrap(),
@" { c = 'a.b.c #0' , d = 'a.b.d #1'}");
insta::assert_snapshot!(
config.get_table("a").unwrap(),
@"b = { c = 'a.b.c #0' , d = 'a.b.d #1'}");
}
#[test]
fn test_stacked_config_get_value_shadowing_table() {
let mut config = StackedConfig::empty();
@ -755,11 +921,7 @@ mod tests {
config.add_layer(new_user_layer(indoc! {"
a.b.c = 'a.b.c #1'
"}));
let expected = parse_to_table(indoc! {"
c = 'a.b.c #1'
"});
assert_eq!(config.get_table("a.b").unwrap(), expected);
insta::assert_snapshot!(config.get_table("a.b").unwrap(), @"c = 'a.b.c #1'");
}
#[test]
@ -775,14 +937,13 @@ mod tests {
a.a.c = 'a.a.c #1'
a.c = 'a.c #1'
"}));
let expected = parse_to_table(indoc! {"
a.a = 'a.a.a #0'
a.b = 'a.a.b #1'
a.c = 'a.a.c #1'
b = 'a.b #0'
c = 'a.c #1'
"});
assert_eq!(config.get_table("a").unwrap(), expected);
insta::assert_snapshot!(config.get_table("a").unwrap(), @r"
a.a = 'a.a.a #0'
a.b = 'a.a.b #1'
a.c = 'a.a.c #1'
b = 'a.b #0'
c = 'a.c #1'
");
assert_eq!(config.table_keys("a").collect_vec(), vec!["a", "b", "c"]);
assert_eq!(config.table_keys("a.a").collect_vec(), vec!["a", "b", "c"]);
assert_eq!(config.table_keys("a.b").collect_vec(), vec![""; 0]);
@ -804,10 +965,7 @@ mod tests {
config.add_layer(new_user_layer(indoc! {"
a.a.b = 'a.a.b #2'
"}));
let expected = parse_to_table(indoc! {"
a.b = 'a.a.b #2'
"});
assert_eq!(config.get_table("a").unwrap(), expected);
insta::assert_snapshot!(config.get_table("a").unwrap(), @"a.b = 'a.a.b #2'");
assert_eq!(config.table_keys("a").collect_vec(), vec!["a"]);
assert_eq!(config.table_keys("a.a").collect_vec(), vec!["b"]);
}
@ -827,11 +985,10 @@ mod tests {
config.add_layer(new_user_layer(indoc! {"
a.a.b = 'a.a.b #2'
"}));
let expected = parse_to_table(indoc! {"
a.b = 'a.a.b #2'
b = 'a.b #0'
"});
assert_eq!(config.get_table("a").unwrap(), expected);
insta::assert_snapshot!(config.get_table("a").unwrap(), @r"
a.b = 'a.a.b #2'
b = 'a.b #0'
");
assert_eq!(config.table_keys("a").collect_vec(), vec!["a", "b"]);
assert_eq!(config.table_keys("a.a").collect_vec(), vec!["b"]);
}
@ -850,11 +1007,8 @@ mod tests {
config.add_layer(new_user_layer(indoc! {"
a.a.b = 'a.a.b #2'
"}));
let expected = parse_to_table(indoc! {"
b = 'a.a.b #2'
"});
// a is not under a.a, but it should still shadow lower layers
assert_eq!(config.get_table("a.a").unwrap(), expected);
insta::assert_snapshot!(config.get_table("a.a").unwrap(), @"b = 'a.a.b #2'");
assert_eq!(config.table_keys("a.a").collect_vec(), vec!["b"]);
}
}

View file

@ -374,10 +374,10 @@ impl TryFrom<ConfigValue> for HumanByteSize {
type Error = &'static str;
fn try_from(value: ConfigValue) -> Result<Self, Self::Error> {
if let Ok(n) = value.clone().into_int() {
if let Some(n) = value.as_integer() {
let n = u64::try_from(n).map_err(|_| "Integer out of range")?;
Ok(HumanByteSize(n))
} else if let Ok(s) = value.into_string() {
} else if let Some(s) = value.as_str() {
s.parse()
} else {
Err("Expected a positive integer or a string in '<number><unit>' form")