config: migrate "config get"/"set" to TOML-based name argument parsing

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.

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,

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<toml_edit::Key>) {
         self.0.push(key.into());
@@ -525,7 +530,7 @@ fn read_config_path(config_path: &Path) -> Result<config::Config, config::Config
 }
 
 pub fn write_config_value_to_file(
-    key: &str,
+    key: &ConfigNamePathBuf,
     value_str: &str,
     path: &Path,
 ) -> 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)

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> <NAME> <VALUE>
 
+    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 '<NAME>': 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 '<NAME>': TOML parse error at line 1, column 1
       |
     1 | .
       | ^

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