From 4716ba82655b2f513a8e6d69287d8c274c214523 Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Tue, 2 Apr 2024 11:19:12 -0400 Subject: [PATCH] Add a --use-destination-message option to `jj squash` if `--use-destination-message/-u` is passed to `jj squash`, the resulting revision will use the description of the destination revision and the description(s) of the source revision(s) will be discarded. --- CHANGELOG.md | 4 ++ cli/src/commands/move.rs | 4 +- cli/src/commands/prev.rs | 2 +- cli/src/commands/squash.rs | 53 +++++++++++++++++++--- cli/tests/cli-reference@.md.snap | 4 ++ cli/tests/test_squash_command.rs | 75 ++++++++++++++++++++++++++++++++ 6 files changed, 132 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6feac50316..71bdd59c6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * New command `jj parallelize` that rebases a set of revisions into siblings. +* `jj squash` now accepts a `--use-destination-message/-u` option uses the + description of the destination revision and discards the descriptions of the + source revisions. + ### Fixed bugs ## [0.16.0] - 2024-04-03 diff --git a/cli/src/commands/move.rs b/cli/src/commands/move.rs index c67ad628b0..e955ac942a 100644 --- a/cli/src/commands/move.rs +++ b/cli/src/commands/move.rs @@ -16,7 +16,7 @@ use clap::ArgGroup; use jj_lib::object_id::ObjectId; use tracing::instrument; -use super::squash::move_diff; +use super::squash::{move_diff, SquashedDescription}; use crate::cli_util::{CommandHelper, RevisionArg}; use crate::command_error::{user_error, CommandError}; use crate::ui::Ui; @@ -95,7 +95,7 @@ pub(crate) fn cmd_move( &destination, matcher.as_ref(), &diff_selector, - None, + SquashedDescription::Combine, false, &args.paths, )?; diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs index 7e756e7ae2..6766dd5697 100644 --- a/cli/src/commands/prev.rs +++ b/cli/src/commands/prev.rs @@ -91,7 +91,7 @@ pub(crate) fn cmd_prev( ancestor_expression } else { // Jujutsu will always create a new commit for prev, even where Mercurial cannot - // and fails. The decision and all discussion around it are available + // and fails. The decision and all discussin around it are available // here: https://github.com/martinvonz/jj/pull/1200#discussion_r1298623933 // // If users ever request erroring out, add `.ancestors()` to the revset below. diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index 0141b75dc1..852c15cf3f 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -23,7 +23,7 @@ use jj_lib::settings::UserSettings; use tracing::instrument; use crate::cli_util::{CommandHelper, DiffSelector, RevisionArg, WorkspaceCommandTransaction}; -use crate::command_error::{user_error, CommandError}; +use crate::command_error::{cli_error, user_error, CommandError}; use crate::description_util::{combine_messages, join_message_paragraphs}; use crate::ui::Ui; @@ -62,6 +62,10 @@ pub(crate) struct SquashArgs { /// The description to use for squashed revision (don't open editor) #[arg(long = "message", short, value_name = "MESSAGE")] message_paragraphs: Vec, + /// Use the description of the destination revision and discard the + /// description(s) of the source revision(s) + #[arg(long, short, conflicts_with = "message_paragraphs")] + use_destination_message: bool, /// Interactively choose which parts to squash #[arg(long, short)] interactive: bool, @@ -116,8 +120,6 @@ pub(crate) fn cmd_squash( workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?; let mut tx = workspace_command.start_transaction(); let tx_description = format!("squash commits into {}", destination.id().hex()); - let description = (!args.message_paragraphs.is_empty()) - .then(|| join_message_paragraphs(&args.message_paragraphs)); move_diff( ui, &mut tx, @@ -126,7 +128,7 @@ pub(crate) fn cmd_squash( &destination, matcher.as_ref(), &diff_selector, - description, + SquashedDescription::from_args(args)?, args.revision.is_none() && args.from.is_empty() && args.into.is_none(), &args.paths, )?; @@ -134,6 +136,40 @@ pub(crate) fn cmd_squash( Ok(()) } +// TODO(#2882): Remove public visibility once `jj move` is deleted. +pub(crate) enum SquashedDescription { + // Use this exact description. + Exact(String), + // Use the destination's description and discard the descriptions of the + // source revisions. + UseDestination, + // Combine the descriptions of the source and destination revisions. + Combine, +} + +// TODO(#2882): Remove public visibility once `jj move` is deleted. +impl SquashedDescription { + pub(crate) fn from_args(args: &SquashArgs) -> Result { + // These options are incompatible. Clap is configured to prevent this, but we + // should still check. + if args.use_destination_message && !args.message_paragraphs.is_empty() { + return Err(cli_error( + "the argument '--message ' cannot be used with \ + '--use-destination-message'", + )); + } + + if !args.message_paragraphs.is_empty() { + let desc = join_message_paragraphs(&args.message_paragraphs); + Ok(SquashedDescription::Exact(desc)) + } else if args.use_destination_message { + Ok(SquashedDescription::UseDestination) + } else { + Ok(SquashedDescription::Combine) + } + } +} + #[allow(clippy::too_many_arguments)] pub fn move_diff( ui: &mut Ui, @@ -143,7 +179,7 @@ pub fn move_diff( destination: &Commit, matcher: &dyn Matcher, diff_selector: &DiffSelector, - description: Option, + description: SquashedDescription, no_rev_arg: bool, path_arg: &[String], ) -> Result<(), CommandError> { @@ -233,8 +269,11 @@ from the source will be moved into the destination. destination_tree = destination_tree.merge(&tree1, &tree2)?; } let description = match description { - Some(description) => description, - None => combine_messages(tx.base_repo(), &abandoned_commits, destination, settings)?, + SquashedDescription::Exact(description) => description, + SquashedDescription::UseDestination => destination.description().to_owned(), + SquashedDescription::Combine => { + combine_messages(tx.base_repo(), &abandoned_commits, destination, settings)? + } }; let mut predecessors = vec![destination.id().clone()]; predecessors.extend(sources.iter().map(|source| source.id().clone())); diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 39638eaa51..69f49cac23 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1761,6 +1761,10 @@ If a working-copy commit gets abandoned, it will be given a new, empty commit. T * `--from ` — Revision(s) to squash from (default: @) * `--into ` — Revision to squash into (default: @) * `-m`, `--message ` — The description to use for squashed revision (don't open editor) +* `-u`, `--use-destination-message` — Use the description of the destination revision and discard the description(s) of the source revision(s) + + Possible values: `true`, `false` + * `-i`, `--interactive` — Interactively choose which parts to squash Possible values: `true`, `false` diff --git a/cli/tests/test_squash_command.rs b/cli/tests/test_squash_command.rs index 9161ca9bec..d640f199f7 100644 --- a/cli/tests/test_squash_command.rs +++ b/cli/tests/test_squash_command.rs @@ -977,9 +977,84 @@ fn test_squash_empty() { "###); } +#[test] +fn test_squash_use_destination_message() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["commit", "-m=a"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m=b"]); + test_env.jj_cmd_ok(&repo_path, &["describe", "-m=c"]); + // Test the setup + insta::assert_snapshot!(get_log_output_with_description(&test_env, &repo_path), @r###" + @ 71f7c810d8ed c + ◉ 10dd87c3b4e2 b + ◉ 4c5b3042d9e0 a + ◉ 000000000000 + "###); + + // Squash the current revision using the short name for the option. + test_env.jj_cmd_ok(&repo_path, &["squash", "-u"]); + insta::assert_snapshot!(get_log_output_with_description(&test_env, &repo_path), @r###" + @ 10e30ce4a910 + ◉ 1c21278b775f b + ◉ 4c5b3042d9e0 a + ◉ 000000000000 + "###); + + // Undo and squash again, but this time squash both "b" and "c" into "a". + test_env.jj_cmd_ok(&repo_path, &["undo"]); + test_env.jj_cmd_ok( + &repo_path, + &[ + "squash", + "--use-destination-message", + "--from", + "description(b)::", + "--into", + "description(a)", + ], + ); + insta::assert_snapshot!(get_log_output_with_description(&test_env, &repo_path), @r###" + @ da1507508bdf + ◉ f1387f804776 a + ◉ 000000000000 + "###); +} + +// The --use-destination-message and --message options are incompatible. +#[test] +fn test_squash_use_destination_message_and_message_mutual_exclusion() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m=a"]); + test_env.jj_cmd_ok(&repo_path, &["describe", "-m=b"]); + insta::assert_snapshot!(test_env.jj_cmd_cli_error( + &repo_path, + &[ + "squash", + "--message=123", + "--use-destination-message", + ], + ), @r###" + error: the argument '--message ' cannot be used with '--use-destination-message' + + Usage: jj squash --message [PATHS]... + + For more information, try '--help'. + "###); +} + fn get_description(test_env: &TestEnvironment, repo_path: &Path, rev: &str) -> String { test_env.jj_cmd_success( repo_path, &["log", "--no-graph", "-T", "description", "-r", rev], ) } + +fn get_log_output_with_description(test_env: &TestEnvironment, repo_path: &Path) -> String { + let template = r#"separate(" ", commit_id.short(), description)"#; + test_env.jj_cmd_success(repo_path, &["log", "-T", template]) +}