From 6a4e64a2556d48b701ab81fbae00045f050910e4 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

Follows up on #3725
---
 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"]}}]
     "###);
 }