config get: add support for displaying arrays and tables #7394
Conversation
AngelEzquerra
force-pushed
the
config_get_complex_values
branch
from
e564d73 to
a72d131
Compare
cli/src/commands/config/get.rs
Outdated
| | ConfigValue::Boolean(_) | ||
| | ConfigValue::Datetime(_) | ||
| | ConfigValue::Array(_) => Ok(value.decorated("", "").to_string()), | ||
| ConfigValue::InlineTable(v) => Ok(format_inline_table_as_toml(&v)), |
There was a problem hiding this comment.
I think inline table should be printed as value. It would be weird if colors.error were flattened, for example. To list non-value table items, the user should use jj config list.
There was a problem hiding this comment.
What do you mean by "value". Do you mean printing the table as a JSON object?
There was a problem hiding this comment.
Yes, as a TOML inline table.
https://toml.io/en/v1.0.0#inline-table
BTW, we no longer have to use .get_value_with() because it doesn't fail conditionally.
let value = command.settings().get_value(..)?;
if let Some(s) = value.as_str() {
writeln!(ui.stdout(), "{s}")?;
} else {
writeln!(ui.stdout(), "{}", value.decorated("", ""))?;
}There was a problem hiding this comment.
My first stab at this printed inline tables "as is" but I found that it was very unreadable, with the table being printed in a single line. Is that what you propose? Or would do you want to somehow pretty print the inline table into multiple lines? I thought it would make more sense to print them as a proper TOML table instead...
There was a problem hiding this comment.
Yes, just print it as a TOML inline table. It would be weird if jj config get returned multiple values depending on the type.
Strictly speaking, it's also odd that a string value is printed without quotes, which cannot be parsed as a TOML value. We might have to add -T/--template option to print the value in user-specified format.
There was a problem hiding this comment.
OK, I've changed it so that it just printing inline tables as you suggested.
AngelEzquerra
force-pushed
the
config_get_complex_values
branch
from
a72d131 to
e141f39
Compare
Up until now, trying to get a config value that was an array or a table would fail with an error indicating that only values that can be converted to a string can be displayed. This change fixes that issue by converting arrays and tables into TOML format.
AngelEzquerra
force-pushed
the
config_get_complex_values
branch
from
e141f39 to
781cb7e
Compare
| * `jj config get` now supports displaying array and table config values. | ||
|
|
There was a problem hiding this comment.
nit: move this to "Unreleased" and remove excessive blank line.
| let stringified = | ||
| command | ||
| .settings() | ||
| .get_value_with(&args.name, |value| -> Result<String, &str> { | ||
| match value { |
There was a problem hiding this comment.
nit: since inner function no longer fails, we don't have to use .get_value_with().
match command.settings().get_value(&args.name)? {
...
}
Up until now, trying to get a config value that was an array or a table would fail with an error indicating that only values that can be converted to a string can be displayed. This change fixes that issue by converting arrays and tables into TOML format.
Checklist
If applicable:
CHANGELOG.mdREADME.md,docs/,demos/)cli/src/config-schema.json)