From 45c3acc34ac88a6bf78a7ba080494ca221ae4102 Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Tue, 21 May 2024 14:32:32 -0700
Subject: [PATCH] cli `config list`: serialize toml values more properly
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Follows up on #3725

Apart from the bugfix, this also makes `jj config list` output

```
template-aliases.builtin_log_node="""
coalesce(
  if(!self, label(\"elided\", \"~\")),
  label(
    separate(\" \",
      if(current_working_copy, \"working_copy\"),
      if(immutable, \"immutable\"),
      if(conflict, \"conflict\"),
    ),
    coalesce(
      if(current_working_copy, \"@\"),
      if(immutable, \"◆\"),
      if(conflict, \"×\"),
      \"○\",
    )
  )
)
"""
```

instead of

```
template-aliases.builtin_log_node="coalesce(\n  if(!self, label(\"elided\", \"~\")),\n  label(\n    separate(\" \",\n      if(current_working_copy, \"working_copy\"),\n      if(immutable, \"immutable\"),\n      if(conflict, \"conflict\"),\n    ),\n    coalesce(\n      if(current_working_copy, \"@\"),\n      if(immutable, \"◆\"),\n      if(conflict, \"×\"),\n      \"○\",\n    )\n  )\n)\n"
```.

`toml_edit` also seems to use single-quoted strings sometimes.
---
 cli/src/commands/config.rs       | 20 +++++++++++++-------
 cli/tests/test_alias.rs          |  2 +-
 cli/tests/test_config_command.rs |  4 +++-
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/cli/src/commands/config.rs b/cli/src/commands/config.rs
index 7ed7f59de5..c1cf864ad8 100644
--- a/cli/src/commands/config.rs
+++ b/cli/src/commands/config.rs
@@ -169,26 +169,27 @@ pub(crate) fn cmd_config(
     }
 }
 
-fn toml_escape_key(key: String) -> String {
+fn toml_escape_key(key: &str) -> String {
     toml_edit::Key::from(key).to_string()
 }
 
-// TODO: Use a proper TOML library to serialize instead.
+// TODO: If `config::Value` implemented Serialize, this would be much easier.
 pub fn toml_serialize_value(value: &config::Value) -> String {
     match &value.kind {
         config::ValueKind::Table(table) => format!(
             "{{{}}}",
-            // TODO: Remove sorting when config crate maintains deterministic ordering.
+            // TODO: `config` crate now has a `preserve_order` crate feature. We could use it
+            // instead of sorting.
             table
                 .iter()
                 .sorted_by_key(|(k, _)| *k)
-                .map(|(k, v)| format!("{k}={}", toml_serialize_value(v)))
+                .map(|(k, v)| format!("{}={}", toml_escape_key(k), toml_serialize_value(v)))
                 .join(", ")
         ),
         config::ValueKind::Array(vals) => {
             format!("[{}]", vals.iter().map(toml_serialize_value).join(", "))
         }
-        config::ValueKind::String(val) => format!("{val:?}"),
+        config::ValueKind::String(val) => toml_edit::value(val).to_string(),
         _ => value.to_string(),
     }
 }
@@ -200,8 +201,13 @@ fn config_template_language() -> GenericTemplateLanguage<'static, AnnotatedValue
     let mut language = L::new();
     // "name" instead of "path" to avoid confusion with the source file path
     language.add_keyword("name", |self_property| {
-        let out_property = self_property
-            .map(|annotated| annotated.path.into_iter().map(toml_escape_key).join("."));
+        let out_property = self_property.map(|annotated| {
+            annotated
+                .path
+                .into_iter()
+                .map(|s| toml_escape_key(&s))
+                .join(".")
+        });
         Ok(L::wrap_string(out_property))
     });
     language.add_keyword("value", |self_property| {
diff --git a/cli/tests/test_alias.rs b/cli/tests/test_alias.rs
index 85b3d058f8..d0916a2b2d 100644
--- a/cli/tests/test_alias.rs
+++ b/cli/tests/test_alias.rs
@@ -334,6 +334,6 @@ fn test_alias_in_repo_config() {
         ],
     );
     insta::assert_snapshot!(stdout, @r###"
-    aliases.l=["log", "-r@", "--no-graph", "-T\"user alias\\n\""]
+    aliases.l=["log", "-r@", "--no-graph", '-T"user alias\n"']
     "###);
 }
diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs
index b8413c89bb..20e0a0ac90 100644
--- a/cli/tests/test_config_command.rs
+++ b/cli/tests/test_config_command.rs
@@ -104,11 +104,13 @@ fn test_config_list_inline_table() {
         x = 1
         [[test-table]]
         y = ["z"]
+        [[test-table]]
+        z."this=key is silly" = ["qq"]
     "#,
     );
     let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "test-table"]);
     insta::assert_snapshot!(stdout, @r###"
-    test-table=[{x=1}, {y=["z"]}]
+    test-table=[{x=1}, {y=["z"]}, {z={"this=key is silly"=["qq"]}}]
     "###);
 }