diff --git a/CHANGELOG.md b/CHANGELOG.md index 7558615926..e622527fc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj branch` can accept any number of branches to update, rather than just one. +* Aliases can now call other aliases. + ### Fixed bugs * When rebasing a conflict where one side modified a file and the other side @@ -114,6 +116,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * You now get a proper error message instead of a crash when `$EDITOR` doesn't exist or exits with an error. +* Global arguments, such as `--at-op=`, can now be passed before + an alias. + * Fixed relative path to the current directory in output to be `.` instead of empty string. diff --git a/src/commands.rs b/src/commands.rs index 12a00a3851..962b0fe38f 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1042,7 +1042,11 @@ fn update_working_copy( /// /// To get started, see the tutorial at https://github.com/martinvonz/jj/blob/main/docs/tutorial.md. #[derive(clap::Parser, Clone, Debug)] -#[clap(author = "Martin von Zweigbergk ", version)] +#[clap( + name = "jj", + author = "Martin von Zweigbergk ", + version +)] #[clap(mut_arg("help", |arg| { arg .help("Print help information, more help with --help than with -h") @@ -1102,6 +1106,27 @@ struct GlobalArgs { at_operation: String, } +impl GlobalArgs { + fn merge_from(&mut self, earlier_global_args: &GlobalArgs) { + // Destructure `earlier_global_args`, so we're forced to update this code if new + // fields are added in the future. + let GlobalArgs { + repository, + no_commit_working_copy, + at_operation, + } = earlier_global_args; + if self.repository.is_none() { + self.repository = repository.clone(); + } + if !self.no_commit_working_copy { + self.no_commit_working_copy = *no_commit_working_copy; + } + if self.at_operation == "@" { + self.at_operation = at_operation.clone() + } + } +} + #[derive(Subcommand, Clone, Debug)] enum Commands { Init(InitArgs), @@ -1139,6 +1164,9 @@ enum Commands { Git(GitArgs), Bench(BenchArgs), Debug(DebugArgs), + /// An alias or an unknown command + #[clap(external_subcommand)] + Alias(Vec), } /// Create a new repo in the given directory @@ -5034,34 +5062,53 @@ fn string_list_from_config(value: config::Value) -> Option> { } } -fn resolve_alias(settings: &UserSettings, args: Vec) -> Result, CommandError> { - if args.len() >= 2 { - let command_name = args[1].clone(); - match settings - .config() - .get::(&format!("alias.{}", command_name)) - { - Ok(value) => { - if let Some(alias_definition) = string_list_from_config(value) { - let mut resolved_args = vec![args[0].clone()]; - resolved_args.extend(alias_definition); - resolved_args.extend_from_slice(&args[2..]); - Ok(resolved_args) - } else { - Err(CommandError::UserError(format!( - r#"Alias definition for "{}" must be a string list"#, - command_name, - ))) - } +fn parse_args(settings: &UserSettings, string_args: &[String]) -> Result { + let mut resolved_aliases = HashSet::new(); + let mut string_args = string_args.to_vec(); + let mut earlier_global_args = None; + loop { + let mut args: Args = clap::Parser::parse_from(&string_args); + if let Some(earlier_global_args) = &earlier_global_args { + args.global_args.merge_from(earlier_global_args); + } + if let Commands::Alias(alias_args) = &args.command { + let alias_name = &alias_args[0]; + if resolved_aliases.contains(alias_name) { + return Err(CommandError::UserError(format!( + r#"Recursive alias definition involving "{alias_name}""# + ))); } - Err(config::ConfigError::NotFound(_)) => { - // The command is not an alias - Ok(args) + match settings + .config() + .get::(&format!("alias.{}", alias_name)) + { + Ok(value) => { + if let Some(alias_definition) = string_list_from_config(value) { + let executable_name = string_args[0].clone(); + string_args = vec![executable_name]; + string_args.extend(alias_definition); + string_args.extend_from_slice(&alias_args[1..]); + resolved_aliases.insert(alias_name.clone()); + earlier_global_args = Some(args.global_args.clone()); + } else { + return Err(CommandError::UserError(format!( + r#"Alias definition for "{alias_name}" must be a string list"# + ))); + } + } + Err(config::ConfigError::NotFound(_)) => { + let mut app = Args::command(); + app.error(clap::ErrorKind::ArgumentNotFound, format!( + r#"Found argument '{alias_name}' which wasn't expected, or isn't valid in this context"# + )).exit(); + } + Err(err) => { + return Err(CommandError::from(err)); + } } - Err(err) => Err(CommandError::from(err)), + } else { + return Ok(args); } - } else { - Ok(args) } } @@ -5080,9 +5127,8 @@ where } } - let string_args = resolve_alias(ui.settings(), string_args)?; + let args = parse_args(ui.settings(), &string_args)?; let app = Args::command(); - let args: Args = clap::Parser::parse_from(&string_args); let command_helper = CommandHelper::new(app, string_args, args.global_args.clone()); match &args.command { Commands::Init(sub_args) => cmd_init(ui, &command_helper, sub_args), @@ -5119,6 +5165,7 @@ where Commands::Git(sub_args) => cmd_git(ui, &command_helper, sub_args), Commands::Bench(sub_args) => cmd_bench(ui, &command_helper, sub_args), Commands::Debug(sub_args) => cmd_debug(ui, &command_helper, sub_args), + Commands::Alias(_) => panic!("Unresolved alias"), } } diff --git a/tests/test_alias.rs b/tests/test_alias.rs index d9b4b0ba6b..0dc6b053fc 100644 --- a/tests/test_alias.rs +++ b/tests/test_alias.rs @@ -16,6 +16,143 @@ use crate::common::TestEnvironment; pub mod common; +#[test] +fn test_alias_basic() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.add_config( + br#"[alias] + b = ["log", "-r", "@", "-T", "branches"] + "#, + ); + test_env.jj_cmd_success(&repo_path, &["branch", "my-branch"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["b"]); + insta::assert_snapshot!(stdout, @r###" + @ my-branch + ~ + "###); +} + +#[test] +fn test_alias_calls_unknown_command() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.add_config( + br#"[alias] + foo = ["nonexistent"] + "#, + ); + // Should get an error about the unknown command + test_env.jj_cmd_cli_error(&repo_path, &["foo"]); +} + +#[test] +fn test_alias_cannot_override_builtin() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.add_config( + br#"[alias] + log = ["rebase"] + "#, + ); + // Alias should be ignored + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "root"]); + insta::assert_snapshot!(stdout, @r###" + o 000000000000 000000000000 1970-01-01 00:00:00.000 +00:00 + + "###); +} + +#[test] +fn test_alias_recursive() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.add_config( + br#"[alias] + foo = ["foo"] + bar = ["baz"] + baz = ["bar"] + "#, + ); + // Alias should not cause infinite recursion or hang + let stderr = test_env.jj_cmd_failure(&repo_path, &["foo"]); + insta::assert_snapshot!(stderr, @r###" + Error: Recursive alias definition involving "foo" + "###); + // Also test with mutual recursion + let stderr = test_env.jj_cmd_failure(&repo_path, &["bar"]); + insta::assert_snapshot!(stderr, @r###" + Error: Recursive alias definition involving "bar" + "###); +} + +#[test] +fn test_alias_global_args_before_and_after() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + test_env.add_config( + br#"[alias] + l = ["log", "-T", "commit_id"] + "#, + ); + // Test the setup + let stdout = test_env.jj_cmd_success(&repo_path, &["l"]); + insta::assert_snapshot!(stdout, @r###" + @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 + o 0000000000000000000000000000000000000000 + "###); + + // Can pass global args before + let stdout = test_env.jj_cmd_success(&repo_path, &["l", "--at-op", "@-"]); + insta::assert_snapshot!(stdout, @r###" + o 0000000000000000000000000000000000000000 + "###); + // Can pass global args after + let stdout = test_env.jj_cmd_success(&repo_path, &["--at-op", "@-", "l"]); + insta::assert_snapshot!(stdout, @r###" + o 0000000000000000000000000000000000000000 + "###); + // Test passing global args both before and after + // TODO: These should ideally fail just like it does when there's no alias + // involved. + let stdout = test_env.jj_cmd_success(&repo_path, &["--at-op", "abc123", "l", "--at-op", "@-"]); + insta::assert_snapshot!(stdout, @r###" + o 0000000000000000000000000000000000000000 + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["-R", "../nonexistent", "l", "-R", "."]); + insta::assert_snapshot!(stdout, @r###" + @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 + o 0000000000000000000000000000000000000000 + "###); +} + +#[test] +fn test_alias_global_args_in_definition() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + test_env.add_config( + br#"[alias] + l = ["log", "-T", "commit_id", "--at-op", "@-"] + "#, + ); + + // The global argument in the alias is respected + let stdout = test_env.jj_cmd_success(&repo_path, &["l"]); + insta::assert_snapshot!(stdout, @r###" + o 0000000000000000000000000000000000000000 + "###); +} + #[test] fn test_alias_invalid_definition() { let test_env = TestEnvironment::default();