From 24131d8ba6cb6d8c8838e745d174db5f46277a72 Mon Sep 17 00:00:00 2001 From: Benjamin Tan Date: Fri, 5 Jul 2024 17:46:05 +0800 Subject: [PATCH] describe: allow updating the description of multiple commits If multiple commits are provided, the description of each commit will be combined into a single file for editing. --- CHANGELOG.md | 2 + cli/src/command_error.rs | 7 + cli/src/commands/describe.rs | 197 ++++++++++++++++--- cli/src/description_util.rs | 298 ++++++++++++++++++++++++++++- cli/tests/cli-reference@.md.snap | 10 +- cli/tests/test_describe_command.rs | 256 ++++++++++++++++++++++++- 6 files changed, 724 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f42cfd4ef8..234d1f1e06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,6 +103,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * Added revset functions `author_date` and `committer_date`. +* `jj describe` can now update the description of multiple commits. + ### Fixed bugs * `jj status` will show different messages in a conflicted tree, depending diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 1c3f9a5936..8ebff95097 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -37,6 +37,7 @@ use jj_lib::workspace::WorkspaceInitError; use thiserror::Error; use crate::cli_util::short_operation_hash; +use crate::description_util::ParseBulkEditMessageError; use crate::diff_util::DiffRenderError; use crate::formatter::{FormatRecorder, Formatter}; use crate::merge_tools::{ConflictResolveError, DiffEditError, MergeToolConfigError}; @@ -541,6 +542,12 @@ impl From for CommandError { } } +impl From for CommandError { + fn from(err: ParseBulkEditMessageError) -> Self { + user_error(err) + } +} + fn find_source_parse_error_hint(err: &dyn error::Error) -> Option { let source = err.source()?; if let Some(source) = source.downcast_ref() { diff --git a/cli/src/commands/describe.rs b/cli/src/commands/describe.rs index e3dc66d5a7..1004cca767 100644 --- a/cli/src/commands/describe.rs +++ b/cli/src/commands/describe.rs @@ -12,33 +12,45 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashMap; use std::io::{self, Read}; +use itertools::Itertools; +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::description_util::{description_template, edit_description, join_message_paragraphs}; +use crate::command_error::{user_error, CommandError}; +use crate::description_util::{ + description_template, edit_description, edit_multiple_descriptions, join_message_paragraphs, + ParsedBulkEditMessage, +}; use crate::ui::Ui; /// Update the change description or other metadata /// -/// Starts an editor to let you edit the description of a change. The editor +/// Starts an editor to let you edit the description of changes. The editor /// will be $EDITOR, or `pico` if that's not defined (`Notepad` on Windows). #[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 @@ -65,39 +77,164 @@ 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 commits: Vec<_> = workspace_command + .parse_union_revsets(&args.revisions)? + .evaluate_to_commits()? + .try_collect()?; // in reverse topological order + if commits.is_empty() { + writeln!(ui.status(), "No revisions to describe.")?; + return Ok(()); + } + workspace_command.check_rewritable(commits.iter().ids())?; let mut tx = workspace_command.start_transaction(); - let mut commit_builder = tx - .mut_repo() - .rewrite_commit(command.settings(), &commit) - .detach(); - if args.reset_author { - commit_builder.set_author(commit_builder.committer().clone()); - } + let tx_description = if commits.len() == 1 { + format!("describe commit {}", commits[0].id().hex()) + } else { + format!( + "describe commit {} and {} more", + commits[0].id().hex(), + commits.len() - 1 + ) + }; - let description = if args.stdin { + let shared_description = if args.stdin { let mut buffer = String::new(); io::stdin().read_to_string(&mut buffer)?; - 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 { - if commit_builder.description().is_empty() { - commit_builder.set_description(command.settings().default_description()); + None + }; + + let commit_descriptions: Vec<(_, _)> = if args.no_edit || shared_description.is_some() { + commits + .iter() + .map(|commit| { + let new_description = shared_description + .as_deref() + .unwrap_or_else(|| commit.description()); + (commit, new_description.to_owned()) + }) + .collect() + } else { + let repo = tx.base_repo().clone(); + let temp_commits: Vec<(_, _)> = commits + .iter() + // Edit descriptions in topological order + .rev() + .map(|commit| -> Result<_, CommandError> { + let mut commit_builder = tx + .mut_repo() + .rewrite_commit(command.settings(), commit) + .detach(); + if commit_builder.description().is_empty() { + commit_builder.set_description(command.settings().default_description()); + } + if args.reset_author { + let new_author = commit_builder.committer().clone(); + commit_builder.set_author(new_author); + } + let temp_commit = commit_builder.write_hidden()?; + Ok((commit.id(), temp_commit)) + }) + .try_collect()?; + + if let [(_, temp_commit)] = &*temp_commits { + let template = description_template(&tx, "", temp_commit)?; + let description = edit_description(&repo, &template, command.settings())?; + + vec![(&commits[0], description)] + } else { + let ParsedBulkEditMessage { + descriptions, + missing, + duplicates, + unexpected, + } = edit_multiple_descriptions(&mut tx, &repo, &temp_commits, command.settings())?; + if !missing.is_empty() { + return Err(user_error(format!( + "The description for the following commits were not found in the edited \ + message: {}", + missing.join(", ") + ))); + } + if !duplicates.is_empty() { + return Err(user_error(format!( + "The following commits were found in the edited message multiple times: {}", + duplicates.join(", ") + ))); + } + if !unexpected.is_empty() { + return Err(user_error(format!( + "The following commits were not being edited, but were found in the edited \ + message: {}", + unexpected.join(", ") + ))); + } + + let commit_descriptions = commits + .iter() + .map(|commit| { + let description = descriptions.get(commit.id()).unwrap().to_owned(); + (commit, description) + }) + .collect(); + + commit_descriptions } - let temp_commit = commit_builder.write_hidden()?; - let template = description_template(&tx, "", &temp_commit)?; - edit_description(tx.base_repo(), &template, command.settings())? }; - commit_builder.set_description(description); - if commit_builder.description() != commit.description() || args.reset_author { - commit_builder.write(tx.mut_repo())?; + // Filter out unchanged commits to avoid rebasing descendants in + // `transform_descendants` below unnecessarily. + let commit_descriptions: HashMap<_, _> = commit_descriptions + .into_iter() + .filter_map(|(commit, new_description)| { + if *new_description == *commit.description() && !args.reset_author { + None + } else { + Some((commit.id(), new_description)) + } + }) + .collect(); + + let mut num_described = 0; + let mut num_rebased = 0; + // Even though `MutRepo::rewrite_commit` and `MutRepo::rebase_descendants` can + // handle rewriting of a commit even if it is a descendant of another commit + // being rewritten, using `MutRepo::transform_descendants` prevents us from + // rewriting the same commit multiple times, and adding additional entries + // in the predecessor chain. + 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, format!("describe commit {}", commit.id().hex()))?; + tx.finish(ui, tx_description)?; Ok(()) } diff --git a/cli/src/description_util.rs b/cli/src/description_util.rs index 3e6b14d7aa..43881df64b 100644 --- a/cli/src/description_util.rs +++ b/cli/src/description_util.rs @@ -1,16 +1,30 @@ +use std::collections::HashMap; use std::io::Write as _; use bstr::ByteVec as _; +use indexmap::IndexMap; use itertools::Itertools; +use jj_lib::backend::CommitId; use jj_lib::commit::Commit; use jj_lib::repo::ReadonlyRepo; use jj_lib::settings::UserSettings; +use thiserror::Error; -use crate::cli_util::{edit_temp_file, WorkspaceCommandTransaction}; +use crate::cli_util::{edit_temp_file, short_commit_hash, WorkspaceCommandTransaction}; use crate::command_error::CommandError; use crate::formatter::PlainTextFormatter; use crate::text_util; +/// Cleanup a description by normalizing line endings, and removing leading and +/// trailing blank lines. +fn cleanup_description(description: &str) -> String { + let description = description + .lines() + .filter(|line| !line.starts_with("JJ: ")) + .join("\n"); + text_util::complete_newline(description.trim_matches('\n')) +} + pub fn edit_description( repo: &ReadonlyRepo, description: &str, @@ -31,12 +45,126 @@ JJ: Lines starting with "JJ: " (like this one) will be removed. settings, )?; - // Normalize line ending, remove leading and trailing blank lines. - let description = description - .lines() - .filter(|line| !line.starts_with("JJ: ")) - .join("\n"); - Ok(text_util::complete_newline(description.trim_matches('\n'))) + Ok(cleanup_description(&description)) +} + +/// Edits the descriptions of the given commits in a single editor session. +pub fn edit_multiple_descriptions( + tx: &mut WorkspaceCommandTransaction, + repo: &ReadonlyRepo, + commits: &[(&CommitId, Commit)], + settings: &UserSettings, +) -> Result, CommandError> { + let mut commits_map = IndexMap::new(); + let mut bulk_message = String::new(); + + for (commit_id, temp_commit) in commits.iter() { + let commit_hash = short_commit_hash(commit_id); + bulk_message.push_str("JJ: describe "); + bulk_message.push_str(&commit_hash); + bulk_message.push_str(" -------\n"); + commits_map.insert(commit_hash, *commit_id); + let template = description_template(tx, "", temp_commit)?; + bulk_message.push_str(&template); + bulk_message.push('\n'); + } + bulk_message.push_str("JJ: Lines starting with \"JJ: \" (like this one) will be removed.\n"); + + let bulk_message = edit_temp_file( + "description", + ".jjdescription", + repo.repo_path(), + &bulk_message, + settings, + )?; + + Ok(parse_bulk_edit_message(&bulk_message, &commits_map)?) +} + +#[derive(Debug)] +pub struct ParsedBulkEditMessage { + /// The parsed, formatted descriptions. + pub descriptions: HashMap, + /// Commit IDs that were expected while parsing the edited messages, but + /// which were not found. + pub missing: Vec, + /// Commit IDs that were found multiple times while parsing the edited + /// messages. + pub duplicates: Vec, + /// Commit IDs that were found while parsing the edited messages, but which + /// were not originally being edited. + pub unexpected: Vec, +} + +#[derive(Debug, Error, PartialEq)] +pub enum ParseBulkEditMessageError { + #[error(r#"Found the following line without a commit header: "{0}""#)] + LineWithoutCommitHeader(String), +} + +/// Parse the bulk message of edited commit descriptions. +fn parse_bulk_edit_message( + message: &str, + commit_ids_map: &IndexMap, +) -> Result, ParseBulkEditMessageError> +where + T: Eq + std::hash::Hash + Clone, +{ + let mut descriptions = HashMap::new(); + let mut duplicates = Vec::new(); + let mut unexpected = Vec::new(); + + let mut messages: Vec<(&str, Vec<&str>)> = vec![]; + for line in message.lines() { + if let Some(commit_id_prefix) = line.strip_prefix("JJ: describe ") { + let commit_id_prefix = commit_id_prefix.trim_end_matches(" -------"); + messages.push((commit_id_prefix, vec![])); + } else if let Some((_, lines)) = messages.last_mut() { + lines.push(line); + } + // Do not allow lines without a commit header, except for empty lines or comments. + else if !line.trim().is_empty() && !line.starts_with("JJ: ") { + return Err(ParseBulkEditMessageError::LineWithoutCommitHeader( + line.to_owned(), + )); + }; + } + + for (commit_id_prefix, description_lines) in messages { + let commit_id = match commit_ids_map.get(commit_id_prefix) { + Some(&commit_id) => commit_id, + None => { + unexpected.push(commit_id_prefix.to_string()); + continue; + } + }; + if descriptions.contains_key(commit_id) { + duplicates.push(commit_id_prefix.to_string()); + continue; + } + descriptions.insert( + commit_id.clone(), + cleanup_description(&description_lines.join("\n")), + ); + } + + let missing: Vec<_> = commit_ids_map + .iter() + .filter_map(|(commit_id_prefix, commit_id)| { + if !descriptions.contains_key(*commit_id) { + Some(commit_id_prefix.to_string()) + } else { + None + } + }) + .collect(); + + Ok(ParsedBulkEditMessage { + descriptions, + missing, + duplicates, + unexpected, + }) } /// Combines the descriptions from the input commits. If only one is non-empty, @@ -116,3 +244,159 @@ pub fn description_template( // Template output is usually UTF-8, but it can contain file content. Ok(output.into_string_lossy()) } + +#[cfg(test)] +mod tests { + use indexmap::indexmap; + use indoc::indoc; + use maplit::hashmap; + + use super::parse_bulk_edit_message; + use crate::description_util::ParseBulkEditMessageError; + + #[test] + fn test_parse_complete_bulk_edit_message() { + let result = parse_bulk_edit_message( + indoc! {" + JJ: describe 1 ------- + Description 1 + + JJ: describe 2 ------- + Description 2 + "}, + &indexmap! { + "1".to_string() => &1, + "2".to_string() => &2, + }, + ) + .unwrap(); + assert_eq!( + result.descriptions, + hashmap! { + 1 => "Description 1\n".to_string(), + 2 => "Description 2\n".to_string(), + } + ); + assert!(result.missing.is_empty()); + assert!(result.duplicates.is_empty()); + assert!(result.unexpected.is_empty()); + } + + #[test] + fn test_parse_bulk_edit_message_with_missing_descriptions() { + let result = parse_bulk_edit_message( + indoc! {" + JJ: describe 1 ------- + Description 1 + "}, + &indexmap! { + "1".to_string() => &1, + "2".to_string() => &2, + }, + ) + .unwrap(); + assert_eq!( + result.descriptions, + hashmap! { + 1 => "Description 1\n".to_string(), + } + ); + assert_eq!(result.missing, vec!["2".to_string()]); + assert!(result.duplicates.is_empty()); + assert!(result.unexpected.is_empty()); + } + + #[test] + fn test_parse_bulk_edit_message_with_duplicate_descriptions() { + let result = parse_bulk_edit_message( + indoc! {" + JJ: describe 1 ------- + Description 1 + + JJ: describe 1 ------- + Description 1 (repeated) + "}, + &indexmap! { + "1".to_string() => &1, + }, + ) + .unwrap(); + assert_eq!( + result.descriptions, + hashmap! { + 1 => "Description 1\n".to_string(), + } + ); + assert!(result.missing.is_empty()); + assert_eq!(result.duplicates, vec!["1".to_string()]); + assert!(result.unexpected.is_empty()); + } + + #[test] + fn test_parse_bulk_edit_message_with_unexpected_descriptions() { + let result = parse_bulk_edit_message( + indoc! {" + JJ: describe 1 ------- + Description 1 + + JJ: describe 3 ------- + Description 3 (unexpected) + "}, + &indexmap! { + "1".to_string() => &1, + }, + ) + .unwrap(); + assert_eq!( + result.descriptions, + hashmap! { + 1 => "Description 1\n".to_string(), + } + ); + assert!(result.missing.is_empty()); + assert!(result.duplicates.is_empty()); + assert_eq!(result.unexpected, vec!["3".to_string()]); + } + + #[test] + fn test_parse_bulk_edit_message_with_no_header() { + let result = parse_bulk_edit_message( + indoc! {" + Description 1 + "}, + &indexmap! { + "1".to_string() => &1, + }, + ); + assert_eq!( + result.unwrap_err(), + ParseBulkEditMessageError::LineWithoutCommitHeader("Description 1".to_string()) + ); + } + + #[test] + fn test_parse_bulk_edit_message_with_comment_before_header() { + let result = parse_bulk_edit_message( + indoc! {" + JJ: Custom comment and empty lines below should be accepted + + + JJ: describe 1 ------- + Description 1 + "}, + &indexmap! { + "1".to_string() => &1, + }, + ) + .unwrap(); + assert_eq!( + result.descriptions, + hashmap! { + 1 => "Description 1\n".to_string(), + } + ); + assert!(result.missing.is_empty()); + assert!(result.duplicates.is_empty()); + assert!(result.unexpected.is_empty()); + } +} diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 20ceae6159..fd81686e87 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -571,20 +571,24 @@ Update config file to set the given option to a given value 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). +Starts an editor to let you edit the description of changes. 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:** * `-m`, `--message ` — The change description to use (don't open editor) + + If multiple revisions are specified, the same description will be used for all of them. * `--stdin` — Read the change description from stdin + + If multiple revisions are specified, the same description will be used for all of them. * `--no-edit` — Don't open an editor This is mainly useful in combination with e.g. `--reset-author`. diff --git a/cli/tests/test_describe_command.rs b/cli/tests/test_describe_command.rs index 380cff1c80..5532669d1e 100644 --- a/cli/tests/test_describe_command.rs +++ b/cli/tests/test_describe_command.rs @@ -12,7 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; + +use indoc::indoc; use crate::common::{get_stderr_string, TestEnvironment}; @@ -173,6 +175,200 @@ 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"]); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ c6349e79bbfd + ○ 65b6b74e0897 + ○ 230dd059e1b0 + ◆ 000000000000 + "###); + + // 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 41659b84 (empty) description from CLI + Parent commit : rlvkpnrz 8d650510 (empty) (no description set) + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ 41659b846096 description from CLI + ○ 8d650510daad + ○ a42f5755e688 description from CLI + ◆ 000000000000 + "###); + + // Check that the text file gets initialized with the current description of + // each commit and doesn't update commits if no changes are made. + // Commit descriptions are edited in topological order + std::fs::write(&edit_script, "dump editor0").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: describe 8d650510daad ------- + + JJ: describe 41659b846096 ------- + description from CLI + + JJ: Lines starting with "JJ: " (like this one) will be removed. + "###); + + // Set the description of multiple commits in the editor + std::fs::write( + &edit_script, + indoc! {" + write + JJ: describe 8d650510daad ------- + description from editor of @- + + further commit message of @- + + JJ: describe 41659b846096 ------- + description from editor of @ + + further commit message of @ + + JJ: Lines starting with \"JJ: \" (like this one) will be removed. + "}, + ) + .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 f203494a (empty) description from editor of @ + Parent commit : rlvkpnrz 0d76a92c (empty) description from editor of @- + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ f203494a4507 description from editor of @ + │ + │ further commit message of @ + ○ 0d76a92ca7cc description from editor of @- + │ + │ further commit message of @- + ○ a42f5755e688 description from CLI + ◆ 000000000000 + "###); + + // Fails if the edited message has a commit with multiple descriptions + std::fs::write( + &edit_script, + indoc! {" + write + JJ: describe 0d76a92ca7cc ------- + first description from editor of @- + + further commit message of @- + + JJ: describe 0d76a92ca7cc ------- + second description from editor of @- + + further commit message of @- + + JJ: describe f203494a4507 ------- + updated description from editor of @ + + further commit message of @ + + JJ: Lines starting with \"JJ: \" (like this one) will be removed. + "}, + ) + .unwrap(); + let stderr = test_env.jj_cmd_failure(&repo_path, &["describe", "@", "@-"]); + insta::assert_snapshot!(stderr, @r###" + Error: The following commits were found in the edited message multiple times: 0d76a92ca7cc + "###); + + // Fails if the edited message has unexpected commit IDs + std::fs::write( + &edit_script, + indoc! {" + write + JJ: describe 000000000000 ------- + unexpected commit ID + + JJ: describe 0d76a92ca7cc ------- + description from editor of @- + + further commit message of @- + + JJ: describe f203494a4507 ------- + description from editor of @ + + further commit message of @ + + JJ: Lines starting with \"JJ: \" (like this one) will be removed. + "}, + ) + .unwrap(); + let stderr = test_env.jj_cmd_failure(&repo_path, &["describe", "@", "@-"]); + insta::assert_snapshot!(stderr, @r###" + Error: The following commits were not being edited, but were found in the edited message: 000000000000 + "###); + + // Fails if the edited message has missing commit messages + std::fs::write( + &edit_script, + indoc! {" + write + JJ: describe f203494a4507 ------- + description from editor of @ + + further commit message of @ + + JJ: Lines starting with \"JJ: \" (like this one) will be removed. + "}, + ) + .unwrap(); + let stderr = test_env.jj_cmd_failure(&repo_path, &["describe", "@", "@-"]); + insta::assert_snapshot!(stderr, @r###" + Error: The description for the following commits were not found in the edited message: 0d76a92ca7cc + "###); + + // Fails if the edited message has a line which does not have any preceding + // `JJ: describe` headers + std::fs::write( + &edit_script, + indoc! {" + write + description from editor of @- + + JJ: describe f203494a4507 ------- + description from editor of @ + + JJ: Lines starting with \"JJ: \" (like this one) will be removed. + "}, + ) + .unwrap(); + let stderr = test_env.jj_cmd_failure(&repo_path, &["describe", "@", "@-"]); + insta::assert_snapshot!(stderr, @r###" + Error: Found the following line without a commit header: "description from editor of @-" + "###); + + // Fails if the editor fails + std::fs::write(&edit_script, "fail").unwrap(); + let stderr = test_env.jj_cmd_failure(&repo_path, &["describe", "@", "@-"]); + assert!(stderr.contains("exited with an error")); +} + #[test] fn test_multiple_message_args() { let test_env = TestEnvironment::default(); @@ -295,19 +491,30 @@ 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:07.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: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 (the committer is always reset) + // Reset the author for the latest commit (the committer is always reset) test_env.jj_cmd_ok( &repo_path, &[ @@ -320,8 +527,40 @@ 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: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 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""#, + "--no-edit", + "--reset-author", + ], + ); + insta::assert_snapshot!(get_signatures(), @r###" + @ 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 + │ 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 ~ "###); } @@ -343,3 +582,8 @@ fn test_describe_avoids_unc() { // over 260 chars. assert_eq!(edited_path, dunce::simplified(&edited_path)); } + +fn get_log_output(test_env: &TestEnvironment, repo_path: &Path) -> String { + let template = r#"commit_id.short() ++ " " ++ description"#; + test_env.jj_cmd_success(repo_path, &["log", "-T", template]) +}