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

cli/src/config.rs

@@ -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()

cli/tests/test_config_command.rs

@@ -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