From c4a6eec8a037ceaa571d681706774d7c45ace2e2 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 22 May 2024 18:31:42 +0900 Subject: [PATCH] config: migrate "config get"/"set" to TOML-based name argument parsing --- CHANGELOG.md | 4 ++++ cli/src/commands/config.rs | 12 +++++------ cli/src/config.rs | 14 +++++++----- cli/tests/test_config_command.rs | 37 +++++++++++++++++++++++++++++--- docs/contributing.md | 2 +- 5 files changed, 54 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f126599d2..920ca1ede6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj config list` now uses multi-line strings and single-quoted strings in the output when appropriate. +* `jj config get`/`list`/`set` now parse `name` argument as [TOML + key](https://toml.io/en/v1.0.0#keys). Quote meta characters as needed. + Example: `jj config get "revset-aliases.'trunk()'"` + ### Deprecations - Attempting to alias a built-in command now gives a warning, rather than being silently ignored. diff --git a/cli/src/commands/config.rs b/cli/src/commands/config.rs index d1dac27c73..f49103b68e 100644 --- a/cli/src/commands/config.rs +++ b/cli/src/commands/config.rs @@ -116,14 +116,14 @@ pub(crate) struct ConfigListArgs { #[command(verbatim_doc_comment)] pub(crate) struct ConfigGetArgs { #[arg(required = true)] - name: String, + name: ConfigNamePathBuf, } /// Update config file to set the given option to a given value. #[derive(clap::Args, Clone, Debug)] pub(crate) struct ConfigSetArgs { #[arg(required = true)] - name: String, + name: ConfigNamePathBuf, #[arg(required = true)] value: String, #[command(flatten)] @@ -277,10 +277,10 @@ pub(crate) fn cmd_config_get( command: &CommandHelper, args: &ConfigGetArgs, ) -> Result<(), CommandError> { - let value = command - .settings() - .config() - .get_string(&args.name) + let value = args + .name + .lookup_value(command.settings().config()) + .and_then(|value| value.into_string()) .map_err(|err| match err { config::ConfigError::Type { origin, diff --git a/cli/src/config.rs b/cli/src/config.rs index 87986a3ad1..29b7406125 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -17,7 +17,7 @@ use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::process::Command; use std::str::FromStr; -use std::{env, fmt}; +use std::{env, fmt, slice}; use config::Source; use itertools::Itertools; @@ -54,6 +54,11 @@ impl ConfigNamePathBuf { self.0.is_empty() } + /// Returns iterator of path components (or keys.) + pub fn components(&self) -> slice::Iter<'_, toml_edit::Key> { + self.0.iter() + } + /// Appends the given `key` component. pub fn push(&mut self, key: impl Into) { self.0.push(key.into()); @@ -525,7 +530,7 @@ fn read_config_path(config_path: &Path) -> Result Result<(), CommandError> { @@ -555,9 +560,8 @@ pub fn write_config_value_to_file( _ => toml_edit::value(value_str), }; let mut target_table = doc.as_table_mut(); - let mut key_parts_iter = key.split('.'); - // Note: split guarantees at least one item. - let last_key_part = key_parts_iter.next_back().unwrap(); + let mut key_parts_iter = key.components(); + let last_key_part = key_parts_iter.next_back().expect("key must not be empty"); for key_part in key_parts_iter { target_table = target_table .entry(key_part) diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index 6eb31bef67..2232615590 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -420,7 +420,7 @@ fn test_config_layer_workspace() { } #[test] -fn test_config_set_missing_opts() { +fn test_config_set_bad_opts() { let test_env = TestEnvironment::default(); let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["config", "set"]); insta::assert_snapshot!(stderr, @r###" @@ -431,6 +431,19 @@ fn test_config_set_missing_opts() { Usage: jj config set <--user|--repo> + For more information, try '--help'. + "###); + + let stderr = + test_env.jj_cmd_cli_error(test_env.env_root(), &["config", "set", "--user", "", "x"]); + insta::assert_snapshot!(stderr, @r###" + error: invalid value '' for '': TOML parse error at line 1, column 1 + | + 1 | + | ^ + invalid key + + For more information, try '--help'. "###); } @@ -452,6 +465,10 @@ fn test_config_set_for_user() { &repo_path, &["config", "set", "--user", "test-table.foo", "true"], ); + test_env.jj_cmd_ok( + &repo_path, + &["config", "set", "--user", "test-table.'bar()'", "0"], + ); // Ensure test-key successfully written to user config. let user_config_toml = std::fs::read_to_string(&user_config_path) @@ -461,6 +478,7 @@ fn test_config_set_for_user() { [test-table] foo = true + "bar()" = 0 "###); } @@ -741,6 +759,10 @@ fn test_config_path_syntax() { insta::assert_snapshot!(stdout, @r###" 'b c'.e.'f[]'=2 "###); + let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "get", "'b c'.e.'f[]'"]); + insta::assert_snapshot!(stdout, @r###" + 2 + "###); // Not a table let (stdout, stderr) = @@ -749,6 +771,11 @@ fn test_config_path_syntax() { insta::assert_snapshot!(stderr, @r###" Warning: No matching config key for a.'b()'.x "###); + let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["config", "get", "a.'b()'.x"]); + insta::assert_snapshot!(stderr, @r###" + Config error: configuration property "a.'b()'.x" not found + For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md. + "###); // "-" and "_" are valid TOML keys let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "-"]); @@ -765,9 +792,13 @@ fn test_config_path_syntax() { insta::assert_snapshot!(stdout, @r###" '.'=5 "###); - let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["config", "list", "."]); + let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "get", "'.'"]); + insta::assert_snapshot!(stdout, @r###" + 5 + "###); + let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["config", "get", "."]); insta::assert_snapshot!(stderr, @r###" - error: invalid value '.' for '[NAME]': TOML parse error at line 1, column 1 + error: invalid value '.' for '': TOML parse error at line 1, column 1 | 1 | . | ^ diff --git a/docs/contributing.md b/docs/contributing.md index f767d35d4a..ed92bd6f19 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -115,7 +115,7 @@ You will probably also want to make the `gh-pages` branch immutable (and thereby hidden from the default `jj log` output) by running the following in your repo: ```shell -jj config set --repo "revset-aliases.immutable_heads()" 'remote_branches(exact:"main") | remote_branches(exact:"gh-pages")' +jj config set --repo "revset-aliases.'immutable_heads()'" 'remote_branches(exact:"main") | remote_branches(exact:"gh-pages")' ``` ### Summary