diff --git a/CHANGELOG.md b/CHANGELOG.md index bfd9ce82a5..6ab17a7055 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Upgraded `scm-record` from v0.2.0 to v0.3.0. See release notes at https://github.com/arxanas/scm-record/releases/tag/v0.3.0 +* `jj describe` can now update the description of multiple commits. + ### Fixed bugs * Previously, `jj git push` only made sure that the branch is in the expected diff --git a/cli/src/commands/describe.rs b/cli/src/commands/describe.rs index a944eee50b..621e5d815f 100644 --- a/cli/src/commands/describe.rs +++ b/cli/src/commands/describe.rs @@ -12,13 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::io::{self, Read, Write}; +use std::io::{self, Read}; +use indexmap::IndexMap; +use itertools::Itertools; +use jj_lib::backend::CommitId; +use jj_lib::commit::CommitIteratorExt; use jj_lib::object_id::ObjectId; use tracing::instrument; use crate::cli_util::{CommandHelper, RevisionArg}; -use crate::command_error::CommandError; +use crate::command_error::{user_error, CommandError}; use crate::description_util::{ description_template_for_describe, edit_description, join_message_paragraphs, }; @@ -31,16 +35,22 @@ use crate::ui::Ui; #[derive(clap::Args, Clone, Debug)] #[command(visible_aliases = &["desc"])] pub(crate) struct DescribeArgs { - /// The revision whose description to edit + /// The revision(s) whose description to edit #[arg(default_value = "@")] - revision: RevisionArg, + revisions: Vec, /// Ignored (but lets you pass `-r` for consistency with other commands) - #[arg(short = 'r', hide = true)] - unused_revision: bool, + #[arg(short = 'r', hide = true, action = clap::ArgAction::Count)] + unused_revision: u8, /// The change description to use (don't open editor) + /// + /// If multiple revisions are specified, the same description will be used + /// for all of them. #[arg(long = "message", short, value_name = "MESSAGE")] message_paragraphs: Vec, /// Read the change description from stdin + /// + /// If multiple revisions are specified, the same description will be used + /// for all of them. #[arg(long)] stdin: bool, /// Don't open an editor @@ -67,35 +77,96 @@ pub(crate) fn cmd_describe( args: &DescribeArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; - workspace_command.check_rewritable([commit.id()])?; - let description = if args.stdin { + let commits = workspace_command + .resolve_some_revsets_default_single(&args.revisions)? + .into_iter() + .collect_vec(); + if commits.is_empty() { + return Err(user_error("Empty revision set")); + } + workspace_command.check_rewritable(commits.iter().ids())?; + let shared_description = if args.stdin { let mut buffer = String::new(); io::stdin().read_to_string(&mut buffer).unwrap(); - buffer + Some(buffer) } else if !args.message_paragraphs.is_empty() { - join_message_paragraphs(&args.message_paragraphs) - } else if args.no_edit { - commit.description().to_owned() + Some(join_message_paragraphs(&args.message_paragraphs)) } else { - let template = - description_template_for_describe(ui, command.settings(), &workspace_command, &commit)?; - edit_description(workspace_command.repo(), &template, command.settings())? + None }; - if description == *commit.description() && !args.reset_author { - writeln!(ui.status(), "Nothing changed.")?; + + // TODO: Allow editing description of multiple commits in a single editor? + let commit_descriptions: IndexMap<_, _> = commits + .iter() + .rev() + .map( + |commit| -> Result, CommandError> { + let original_description = commit.description(); + let new_description = if args.no_edit { + commit.description().to_owned() + } else if let Some(shared_description) = &shared_description { + shared_description.to_owned() + } else { + let template = description_template_for_describe( + ui, + command.settings(), + &workspace_command, + commit, + )?; + edit_description(workspace_command.repo(), &template, command.settings())? + }; + if new_description == *original_description && !args.reset_author { + Ok(None) + } else { + Ok(Some((commit.id(), new_description.to_owned()))) + } + }, + ) + .filter_map_ok(|x| x) + .try_collect()?; + + let mut tx = workspace_command.start_transaction(); + let tx_description = if commits.len() == 1 { + format!("describe commit {}", commits[0].id().hex()) } else { - let mut tx = workspace_command.start_transaction(); - let mut commit_builder = tx - .mut_repo() - .rewrite_commit(command.settings(), &commit) - .set_description(description); - if args.reset_author { - let new_author = commit_builder.committer().clone(); - commit_builder = commit_builder.set_author(new_author); - } - commit_builder.write()?; - tx.finish(ui, format!("describe commit {}", commit.id().hex()))?; + format!( + "describe commit {} and {} more", + commits[0].id().hex(), + commits.len() - 1 + ) + }; + + let mut num_described = 0; + let mut num_rebased = 0; + tx.mut_repo().transform_descendants( + command.settings(), + commit_descriptions + .keys() + .map(|&id| id.clone()) + .collect_vec(), + |rewriter| { + let old_commit_id = rewriter.old_commit().id().clone(); + let mut commit_builder = rewriter.rebase(command.settings())?; + if let Some(description) = commit_descriptions.get(&old_commit_id) { + commit_builder = commit_builder.set_description(description); + if args.reset_author { + let new_author = commit_builder.committer().clone(); + commit_builder = commit_builder.set_author(new_author); + } + num_described += 1; + } else { + num_rebased += 1; + } + commit_builder.write()?; + Ok(()) + }, + )?; + if num_described > 1 { + writeln!(ui.status(), "Updated {} commits", num_described)?; + } + if num_rebased > 0 { + writeln!(ui.status(), "Rebased {} descendant commits", num_rebased)?; } + tx.finish(ui, tx_description)?; Ok(()) } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 9df2ab8a16..9a17da870c 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -600,20 +600,17 @@ Update the change description or other metadata Starts an editor to let you edit the description of a change. The editor will be $EDITOR, or `pico` if that's not defined (`Notepad` on Windows). -**Usage:** `jj describe [OPTIONS] [REVISION]` +**Usage:** `jj describe [OPTIONS] [REVISIONS]...` ###### **Arguments:** -* `` — The revision whose description to edit +* `` — The revision(s) whose description to edit Default value: `@` ###### **Options:** * `-r` — Ignored (but lets you pass `-r` for consistency with other commands) - - Possible values: `true`, `false` - * `-m`, `--message ` — The change description to use (don't open editor) * `--stdin` — Read the change description from stdin diff --git a/cli/tests/test_describe_command.rs b/cli/tests/test_describe_command.rs index e05a772117..9e0decf30a 100644 --- a/cli/tests/test_describe_command.rs +++ b/cli/tests/test_describe_command.rs @@ -171,6 +171,171 @@ fn test_describe() { assert!(get_stderr_string(&assert).contains("bad-jj-editor-from-jj-editor-env")); } +#[test] +fn test_describe_multiple_commits() { + let mut 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"); + + let edit_script = test_env.set_up_fake_editor(); + + // Initial setup + test_env.jj_cmd_ok(&repo_path, &["new"]); + test_env.jj_cmd_ok(&repo_path, &["new"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-Tdescription"]); + insta::assert_snapshot!(stdout, @r###" + @ + ◉ + ◉ + ◉ + "###); + + // Set the description of multiple commits using `-m` flag + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["describe", "@", "@--", "-m", "description from CLI"], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Updated 2 commits + Rebased 1 descendant commits + Working copy now at: kkmpptxz 3b1c88bc (empty) description from CLI + Parent commit : rlvkpnrz 4026cb14 (empty) (no description set) + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-Tdescription"]); + insta::assert_snapshot!(stdout, @r###" + @ description from CLI + ◉ + ◉ description from CLI + ◉ + "###); + + // Check that the text file gets initialized with the current description of + // each commit and make no changes + std::fs::write( + &edit_script, + ["dump editor0", "next invocation\n", "dump editor1"].join("\0"), + ) + .unwrap(); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["describe", "@", "@-"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Nothing changed. + "###); + insta::assert_snapshot!( + std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###" + + JJ: Lines starting with "JJ: " (like this one) will be removed. + "###); + insta::assert_snapshot!( + std::fs::read_to_string(test_env.env_root().join("editor1")).unwrap(), @r###" + description from CLI + + JJ: Lines starting with "JJ: " (like this one) will be removed. + "###); + + // Set the description of multiple commits in editor + std::fs::write( + &edit_script, + [ + // Commit descriptions are edited in topological order + "write\ndescription from editor of @-", + "next invocation\n", + "write\ndescription from editor of @", + ] + .join("\0"), + ) + .unwrap(); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["describe", "@", "@-"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Updated 2 commits + Working copy now at: kkmpptxz 982a3cfd (empty) description from editor of @ + Parent commit : rlvkpnrz 15019c9d (empty) description from editor of @- + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-Tdescription"]); + insta::assert_snapshot!(stdout, @r###" + @ description from editor of @ + ◉ description from editor of @- + ◉ description from CLI + ◉ + "###); + + // Only update a commit if it's description has changed + std::fs::write( + &edit_script, + [ + "write\ndescription from editor of @-", + "next invocation\n", + "write\nnew description from editor of @", + ] + .join("\0"), + ) + .unwrap(); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["describe", "@", "@-"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: kkmpptxz 633b6c46 (empty) new description from editor of @ + Parent commit : rlvkpnrz 15019c9d (empty) description from editor of @- + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-Tdescription"]); + insta::assert_snapshot!(stdout, @r###" + @ new description from editor of @ + ◉ description from editor of @- + ◉ description from CLI + ◉ + "###); + + // Fails if the editor fails for one of the description edits + std::fs::write( + &edit_script, + [ + "write\ndescription from editor of @-", + "next invocation\n", + "fail", + ] + .join("\0"), + ) + .unwrap(); + let stderr = test_env.jj_cmd_failure(&repo_path, &["describe", "@", "@-"]); + assert!(stderr.contains("exited with an error")); + + // Fail if a revset resolves to more than 1 commit without `all:` prefix + let stderr = test_env.jj_cmd_failure( + &repo_path, + &["describe", "..", "-m", "description from cli"], + ); + insta::assert_snapshot!(stderr, @r###" + Error: Revset ".." resolved to more than one revision + Hint: The revset ".." resolved to these revisions: + kkmpptxz 633b6c46 (empty) new description from editor of @ + rlvkpnrz 15019c9d (empty) description from editor of @- + qpvuntsm 65507340 (empty) description from CLI + Hint: Prefix the expression with 'all:' to allow any number of revisions (i.e. 'all:..'). + "###); + + // Updates description if a revset resolves to more than 1 commit with `all:` + // prefix + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["describe", "all:..", "-m", "description from cli"], + ); + insta::assert_snapshot!(stdout, @r###" + "###); + insta::assert_snapshot!(stderr, @r###" + Updated 3 commits + Working copy now at: kkmpptxz a41d9849 (empty) description from cli + Parent commit : rlvkpnrz 1aa228a6 (empty) description from cli + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-Tdescription"]); + insta::assert_snapshot!(stdout, @r###" + @ description from cli + ◉ description from cli + ◉ description from cli + ◉ + "###); +} + #[test] fn test_multiple_message_args() { let test_env = TestEnvironment::default(); @@ -293,23 +458,60 @@ fn test_describe_author() { &repo_path, &[ "log", - "-r@", + "-r..", "-T", r#"format_signature(author) ++ "\n" ++ format_signature(committer)"#, ], ) }; + + // Initial setup + test_env.jj_cmd_ok(&repo_path, &["new"]); + test_env.jj_cmd_ok(&repo_path, &["new"]); + test_env.jj_cmd_ok(&repo_path, &["new"]); + insta::assert_snapshot!(get_signatures(), @r###" + @ Test User test.user@example.com 2001-02-03 04:05:10.000 +07:00 + │ Test User test.user@example.com 2001-02-03 04:05:10.000 +07:00 + ◉ Test User test.user@example.com 2001-02-03 04:05:09.000 +07:00 + │ Test User test.user@example.com 2001-02-03 04:05:09.000 +07:00 + ◉ Test User test.user@example.com 2001-02-03 04:05:08.000 +07:00 + │ Test User test.user@example.com 2001-02-03 04:05:08.000 +07:00 + ◉ Test User test.user@example.com 2001-02-03 04:05:07.000 +07:00 + │ Test User test.user@example.com 2001-02-03 04:05:07.000 +07:00 + ~ + "###); + + // Reset the author for the latest commit (the committer is always reset) + test_env.jj_cmd_ok( + &repo_path, + &[ + "describe", + "--config-toml", + r#"user.name = "Ove Ridder" + user.email = "ove.ridder@example.com""#, + "--no-edit", + "--reset-author", + ], + ); insta::assert_snapshot!(get_signatures(), @r###" - @ Test User test.user@example.com 2001-02-03 04:05:07.000 +07:00 + @ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:12.000 +07:00 + │ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:12.000 +07:00 + ◉ Test User test.user@example.com 2001-02-03 04:05:09.000 +07:00 + │ Test User test.user@example.com 2001-02-03 04:05:09.000 +07:00 + ◉ Test User test.user@example.com 2001-02-03 04:05:08.000 +07:00 + │ Test User test.user@example.com 2001-02-03 04:05:08.000 +07:00 + ◉ Test User test.user@example.com 2001-02-03 04:05:07.000 +07:00 │ Test User test.user@example.com 2001-02-03 04:05:07.000 +07:00 ~ "###); - // Reset the author (the committer is always reset) + // Reset the author for multiple commits (the committer is always reset) test_env.jj_cmd_ok( &repo_path, &[ "describe", + "@---", + "@-", "--config-toml", r#"user.name = "Ove Ridder" user.email = "ove.ridder@example.com""#, @@ -318,8 +520,14 @@ fn test_describe_author() { ], ); insta::assert_snapshot!(get_signatures(), @r###" - @ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:09.000 +07:00 - │ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:09.000 +07:00 + @ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:12.000 +07:00 + │ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:14.000 +07:00 + ◉ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:14.000 +07:00 + │ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:14.000 +07:00 + ◉ Test User test.user@example.com 2001-02-03 04:05:08.000 +07:00 + │ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:14.000 +07:00 + ◉ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:14.000 +07:00 + │ Ove Ridder ove.ridder@example.com 2001-02-03 04:05:14.000 +07:00 ~ "###); }