From ef8038f60fb5072cd562b6996d42766651c187f1 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Tue, 21 May 2024 20:10:22 +0900
Subject: [PATCH] cli: config: leverage toml_edit::Value to serialize values

I use ValueKind::Ty(ref v) here because (*v).into() looked rather noisy.

Fixes #3374
---
 CHANGELOG.md                     |  3 +++
 cli/src/commands/config.rs       | 44 +++++++++++++++++++-------------
 cli/tests/test_alias.rs          |  2 +-
 cli/tests/test_config_command.rs | 24 ++++++++++++++++-
 4 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0e27a4c3a..7f126599d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * `jj split` will now refuse to split an empty commit.
 
+* `jj config list` now uses multi-line strings and single-quoted strings in the
+  output when appropriate.
+
 ### 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 aeb8f614b..94ddb2829 100644
--- a/cli/src/commands/config.rs
+++ b/cli/src/commands/config.rs
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::fmt;
 use std::io::Write;
 use std::path::{Path, PathBuf};
 
@@ -172,23 +173,29 @@ fn toml_escape_key(key: String) -> String {
     toml_edit::Key::from(key).to_string()
 }
 
-// TODO: Use a proper TOML library to serialize instead.
-fn serialize_config_value(value: &config::Value) -> String {
-    match &value.kind {
-        config::ValueKind::Table(table) => format!(
-            "{{{}}}",
-            // TODO: Remove sorting when config crate maintains deterministic ordering.
-            table
-                .iter()
-                .sorted_by_key(|(k, _)| *k)
-                .map(|(k, v)| format!("{k}={}", serialize_config_value(v)))
-                .join(", ")
-        ),
-        config::ValueKind::Array(vals) => {
-            format!("[{}]", vals.iter().map(serialize_config_value).join(", "))
-        }
-        config::ValueKind::String(val) => format!("{val:?}"),
-        _ => value.to_string(),
+fn to_toml_value(value: &config::Value) -> Result<toml_edit::Value, config::ConfigError> {
+    fn type_error<T: fmt::Display>(message: T) -> config::ConfigError {
+        config::ConfigError::Message(message.to_string())
+    }
+    // It's unlikely that the config object contained unsupported values, but
+    // there's no guarantee. For example, values coming from environment
+    // variables might be big int.
+    match value.kind {
+        config::ValueKind::Nil => Err(type_error(format!("Unexpected value: {value}"))),
+        config::ValueKind::Boolean(v) => Ok(v.into()),
+        config::ValueKind::I64(v) => Ok(v.into()),
+        config::ValueKind::I128(v) => Ok(i64::try_from(v).map_err(type_error)?.into()),
+        config::ValueKind::U64(v) => Ok(i64::try_from(v).map_err(type_error)?.into()),
+        config::ValueKind::U128(v) => Ok(i64::try_from(v).map_err(type_error)?.into()),
+        config::ValueKind::Float(v) => Ok(v.into()),
+        config::ValueKind::String(ref v) => Ok(v.into()),
+        // TODO: Remove sorting when config crate maintains deterministic ordering.
+        config::ValueKind::Table(ref table) => table
+            .iter()
+            .sorted_by_key(|(k, _)| *k)
+            .map(|(k, v)| Ok((k, to_toml_value(v)?)))
+            .collect(),
+        config::ValueKind::Array(ref array) => array.iter().map(to_toml_value).collect(),
     }
 }
 
@@ -286,7 +293,8 @@ fn config_template_language() -> GenericTemplateLanguage<'static, AnnotatedValue
     });
     language.add_keyword("value", |self_property| {
         // TODO: would be nice if we can provide raw dynamically-typed value
-        let out_property = self_property.map(|annotated| serialize_config_value(&annotated.value));
+        let out_property =
+            self_property.and_then(|annotated| Ok(to_toml_value(&annotated.value)?.to_string()));
         Ok(L::wrap_string(out_property))
     });
     language.add_keyword("overridden", |self_property| {
diff --git a/cli/tests/test_alias.rs b/cli/tests/test_alias.rs
index 85b3d058f..d0916a2b2 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 b8413c89b..94ef5e591 100644
--- a/cli/tests/test_config_command.rs
+++ b/cli/tests/test_config_command.rs
@@ -104,11 +104,12 @@ fn test_config_list_inline_table() {
         x = 1
         [[test-table]]
         y = ["z"]
+        z."key=with whitespace" = []
     "#,
     );
     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 = { "key=with whitespace" = [] } }]
     "###);
 }
 
@@ -136,6 +137,27 @@ fn test_config_list_all() {
     "###);
 }
 
+#[test]
+fn test_config_list_multiline_string() {
+    let test_env = TestEnvironment::default();
+    test_env.add_config(
+        r#"
+    multiline = '''
+foo
+bar
+'''
+    "#,
+    );
+
+    let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "multiline"]);
+    insta::assert_snapshot!(stdout, @r###"
+    multiline="""
+    foo
+    bar
+    """
+    "###);
+}
+
 #[test]
 fn test_config_list_layer() {
     let mut test_env = TestEnvironment::default();