cli: unify alias & default-command parsing

cli/src/cli_util.rs

@@ -138,6 +138,7 @@ use jj_lib::workspace::Workspace;
 use jj_lib::workspace::WorkspaceLoadError;
 use jj_lib::workspace::WorkspaceLoader;
 use jj_lib::workspace::WorkspaceLoaderFactory;
+use serde::de;
 use tracing::instrument;
 use tracing_chrome::ChromeLayerBuilder;
 use tracing_subscriber::prelude::*;
@@ -3320,8 +3321,8 @@ fn expand_cmdline_default(
 
     // Resolve default command
     if matches.subcommand().is_none() {
-        let default_args = match get_string_or_array(config, "ui.default-command").optional()? {
+        let default_args = match config.get::<CmdAlias>("ui.default-command").optional()? {
-            Some(opt) => opt,
+            Some(opt) => opt.0,
             None => {
                 writeln!(
                     ui.hint_default(),
@@ -3343,16 +3344,6 @@ fn expand_cmdline_default(
     Ok(())
 }
 
-fn get_string_or_array(
-    config: &StackedConfig,
-    key: &'static str,
-) -> Result<Vec<String>, ConfigGetError> {
-    config
-        .get(key)
-        .map(|string| vec![string])
-        .or_else(|_| config.get::<Vec<String>>(key))
-}
-
 /// Expand any aliases in the supplied command line.
 fn expand_cmdline_aliases(
     ui: &Ui,
@@ -3407,7 +3398,7 @@ fn expand_cmdline_aliases(
                     )));
                 }
 
-                let alias_definition = config.get::<Vec<String>>(["aliases", command_name])?;
+                let alias_definition = config.get::<CmdAlias>(["aliases", command_name])?.0;
 
                 assert!(cmdline.ends_with(&alias_args));
                 cmdline.truncate(cmdline.len() - 1 - alias_args.len());
@@ -3424,6 +3415,50 @@ fn expand_cmdline_aliases(
     }
 }
 
+/// A `Vec<String>` that can also be deserialized as a space-delimited string.
+struct CmdAlias(pub Vec<String>);
+
+impl<'de> de::Deserialize<'de> for CmdAlias {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: de::Deserializer<'de>,
+    {
+        struct Visitor;
+        impl<'de> de::Visitor<'de> for Visitor {
+            type Value = Vec<String>;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+                formatter.write_str("a string or string sequence")
+            }
+
+            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+            where
+                E: de::Error,
+            {
+                Ok(vec![v.to_owned()])
+            }
+
+            fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
+            where
+                A: de::SeqAccess<'de>,
+            {
+                let mut args = Vec::new();
+                if let Some(size_hint) = seq.size_hint() {
+                    args.reserve_exact(size_hint);
+                }
+
+                while let Some(element) = seq.next_element()? {
+                    args.push(element);
+                }
+
+                Ok(args)
+            }
+        }
+
+        deserializer.deserialize_any(Visitor).map(CmdAlias)
+    }
+}
+
 /// Parse args that must be interpreted early, e.g. before printing help.
 fn parse_early_args(
     app: &Command,

cli/src/config-schema.json

@@ -181,17 +181,20 @@
             "properties": {
                 "fsmonitor": {
                     "type": "string",
-                    "enum": ["none", "watchman"],
+                    "enum": [
+                        "none",
+                        "watchman"
+                    ],
                     "description": "Whether to use an external filesystem monitor, useful for large repos"
                 },
                 "watchman": {
                     "type": "object",
                     "properties": {
-                      "register_snapshot_trigger": {
+                        "register_snapshot_trigger": {
-                        "type": "boolean",
+                            "type": "boolean",
-                        "default": false,
+                            "default": false,
-                        "description": "Whether to use triggers to monitor for changes in the background."
+                            "description": "Whether to use triggers to monitor for changes in the background."
-                      }
+                        }
                     }
                 }
             }
@@ -226,14 +229,14 @@
                     "pattern": "^#[0-9a-fA-F]{6}$"
                 },
                 "colors": {
-                  "oneOf": [
+                    "oneOf": [
-                    {
+                        {
-                        "$ref": "#/properties/colors/definitions/colorNames"
+                            "$ref": "#/properties/colors/definitions/colorNames"
-                    },
+                        },
-                    {
+                        {
-                        "$ref": "#/properties/colors/definitions/hexColor"
+                            "$ref": "#/properties/colors/definitions/hexColor"
-                    }
+                        }
-                  ]
+                    ]
                 },
                 "basicFormatterLabels": {
                     "enum": [
@@ -381,12 +384,12 @@
                         }
                     },
                     "diff-invocation-mode": {
-                      "description": "Invoke the tool with directories or individual files",
+                        "description": "Invoke the tool with directories or individual files",
-                      "enum": [
+                        "enum": [
-                        "dir",
+                            "dir",
-                        "file-by-file"
+                            "file-by-file"
-                      ],
+                        ],
-                      "default": "dir"
+                        "default": "dir"
                     },
                     "edit-args": {
                         "type": "array",
@@ -473,10 +476,17 @@
             "type": "object",
             "description": "Custom subcommand aliases to be supported by the jj command",
             "additionalProperties": {
-                "type": "array",
+                "anyOf": [
-                "items": {
+                    {
-                    "type": "string"
+                        "type": "array",
-                }
+                        "items": {
+                            "type": "string"
+                        }
+                    },
+                    {
+                        "type": "string"
+                    }
+                ]
             }
         },
         "snapshot": {
@@ -529,7 +539,11 @@
             "properties": {
                 "backend": {
                     "type": "string",
-                    "enum": ["gpg", "none", "ssh"],
+                    "enum": [
+                        "gpg",
+                        "none",
+                        "ssh"
+                    ],
                     "description": "The backend to use for signing commits. The string `none` disables signing.",
                     "default": "none"
                 },
@@ -581,7 +595,7 @@
                 }
             }
         },
-    "fix": {
+        "fix": {
             "type": "object",
             "description": "Settings for jj fix",
             "properties": {
@@ -619,4 +633,4 @@
             }
         }
     }
-}
+}

cli/tests/test_alias.rs

@@ -34,6 +34,21 @@ fn test_alias_basic() {
     "###);
 }
 
+#[test]
+fn test_alias_string() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    test_env.add_config(r#"aliases.l = "log""#);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["l", "-r", "@", "-T", "description"]);
+    insta::assert_snapshot!(stdout, @r"
+    @
+    │
+    ~
+    ");
+}
+
 #[test]
 fn test_alias_bad_name() {
     let test_env = TestEnvironment::default();
@@ -224,23 +239,25 @@ fn test_alias_global_args_in_definition() {
 }
 
 #[test]
-fn test_alias_invalid_definition() {
+fn test_alias_non_list() {
     let test_env = TestEnvironment::default();
 
-    test_env.add_config(
+    test_env.add_config(r#"aliases.non-list = 5"#);
-        r#"[aliases]
-    non-list = 5
-    non-string-list = [0]
-    "#,
-    );
     let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["non-list"]);
     insta::assert_snapshot!(stderr.replace('\\', "/"), @r"
     Config error: Invalid type or value for aliases.non-list
-    Caused by: invalid type: integer `5`, expected a sequence
+    Caused by: invalid type: integer `5`, expected a string or string sequence
 
     Hint: Check the config file: $TEST_ENV/config/config0002.toml
     For help, see https://jj-vcs.github.io/jj/latest/config/.
     ");
+}
+
+#[test]
+fn test_alias_non_string_list() {
+    let test_env = TestEnvironment::default();
+
+    test_env.add_config(r#"aliases.non-string-list = [0]"#);
     let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["non-string-list"]);
     insta::assert_snapshot!(stderr, @r"
     Config error: Invalid type or value for aliases.non-string-list

cli/tests/test_util_command.rs

@@ -31,8 +31,6 @@ fn test_util_config_schema() {
             "description": "User configuration for Jujutsu VCS. See https://jj-vcs.github.io/jj/latest/config/ for details",
             "properties": {
                 [...]
-            "fix": {
-                [...]
             }
         }
         "###);

docs/config.md

@@ -582,9 +582,12 @@ You can define aliases for commands, including their arguments. For example:
 
 ```toml
 [aliases]
-# `jj l` shows commits on the working-copy commit's (anonymous) bookmark
+# `jj l` is a simple alias for `jj my-log`
+l = "my-log"
+
+# `jj my-log` shows commits on the working-copy commit's (anonymous) bookmark
 # compared to the `main` bookmark
-l = ["log", "-r", "(main..@):: | (main..@)-"]
+my-log = ["log", "-r", "(main..@):: | (main..@)-"]
 ```
 
 This alias syntax can only run a single jj command. However, you may want to