diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e7690b017c..119e2499c10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). jj workspace can have its own Git HEAD. Existing repositories are migrated automatically. +* `jj fix` now supports `--summary`, `--stat` and `-p`/`--patch` flags to + display modified files, a histogram or a patch of changes per fixed commit. + ### Fixed bugs * A side of a conflict whose contents end with a carriage return no longer loses diff --git a/cli/src/commands/fix.rs b/cli/src/commands/fix.rs index 7f300b98f5c..a70780ff05e 100644 --- a/cli/src/commands/fix.rs +++ b/cli/src/commands/fix.rs @@ -29,6 +29,7 @@ use jj_lib::fileset::FilesetExpression; use jj_lib::fileset::FilesetParseContext; use jj_lib::fix::FileToFix; use jj_lib::fix::FixError; +use jj_lib::fix::FixSummary; use jj_lib::fix::LineRange; use jj_lib::fix::ParallelFileFixer; use jj_lib::fix::RegionsToFormat; @@ -46,12 +47,14 @@ use tracing::instrument; use crate::cli_util::CommandHelper; use crate::cli_util::RevisionArg; +use crate::cli_util::WorkspaceCommandTransaction; use crate::cli_util::print_unmatched_explicit_paths; use crate::command_error::CommandError; use crate::command_error::config_error; use crate::command_error::print_parse_diagnostics; use crate::complete; use crate::config::CommandNameAndArgs; +use crate::diff_util::DiffFormatArgs; use crate::ui::Ui; /// Update files with formatting fixes or other changes @@ -159,6 +162,7 @@ use crate::ui::Ui; /// configuration. #[derive(clap::Args, Clone, Debug)] #[command(verbatim_doc_comment)] +#[command(group(clap::ArgGroup::new("format").args(&["summary", "stat", "patch"])))] pub(crate) struct FixArgs { /// Fix files in the specified revision(s) and their descendants. If no /// revisions are specified, this defaults to the `revsets.fix` setting, or @@ -182,6 +186,21 @@ pub(crate) struct FixArgs { /// this option has no effect since the formatter always formats all lines. #[arg(long, short)] all_lines: bool, + + // TODO: `-s` is used by `--source` above, which means we can't reuse + // `DiffFormatArgs` with full diff formatting options and share it with + // `diff` and `op log`. + /// Display a summary of fixed files for each modified commit + #[arg(long)] + summary: bool, + + /// Display a histogram of changes for each modified commit + #[arg(long)] + stat: bool, + + /// Display a patch of changes for each modified commit + #[arg(long, short)] + patch: bool, } #[instrument(skip_all)] @@ -190,6 +209,11 @@ pub(crate) async fn cmd_fix( command: &CommandHelper, args: &FixArgs, ) -> Result<(), CommandError> { + // Reuse `DiffFormatArgs` for consistency across commands, even though we can't + // use it directly for command parsing. + let mut format_args = DiffFormatArgs::default(); + format_args.summary = args.summary; + format_args.stat = args.stat; let mut workspace_command = command.workspace_helper(ui).await?; let workspace_root = workspace_command.workspace_root().to_owned(); let path_converter = workspace_command.path_converter().to_owned(); @@ -253,14 +277,75 @@ pub(crate) async fn cmd_fix( &mut parallel_fixer, ) .await?; + print_fix_summary_and_status( + ui, + &tx, + &matcher, + &commits, + &summary, + &format_args, + args.patch, + ) + .await?; + tx.finish(ui, format!("fixed {} commits", summary.num_fixed_commits)) + .await +} + +async fn print_fix_summary_and_status( + ui: &mut Ui, + tx: &WorkspaceCommandTransaction<'_>, + matcher: &dyn Matcher, + commits: &[Commit], + summary: &FixSummary, + format_args: &DiffFormatArgs, + patch: bool, +) -> Result<(), CommandError> { + let Some(mut formatter) = ui.status_formatter() else { + return Ok(()); + }; writeln!( - ui.status(), + formatter, "Fixed {} commits of {} checked.", - summary.num_fixed_commits, - summary.num_checked_commits + summary.num_fixed_commits, summary.num_checked_commits )?; - tx.finish(ui, format!("fixed {} commits", summary.num_fixed_commits)) - .await + if summary.rewrites.is_empty() { + return Ok(()); + } + + let Some(diff_renderer) = tx + .base_workspace_helper() + .diff_renderer_for_log(format_args, patch)? + else { + return Ok(()); + }; + + let mut buffer = Vec::new(); + for old_commit in commits { + let Some(new_id) = summary.rewrites.get(old_commit.id()) else { + continue; + }; + let new_commit = tx.repo().store().get_commit(new_id)?; + + buffer.clear(); + diff_renderer + .show_inter_diff( + ui, + ui.new_formatter(&mut buffer).as_mut(), + std::slice::from_ref(old_commit), + &new_commit, + matcher, + ui.term_width(), + ) + .await?; + + if !buffer.is_empty() { + tx.write_commit_summary(formatter.as_mut(), &new_commit)?; + writeln!(formatter)?; + formatter.raw()?.write_all(&buffer)?; + } + } + + Ok(()) } /// Invokes all matching tools (if any) to file_to_fix. If the content is diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 98be4b4e2c0..404bbbda77f 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -100,7 +100,7 @@ use crate::templater::TemplateRenderer; use crate::text_util; use crate::ui::Ui; -#[derive(clap::Args, Clone, Debug)] +#[derive(clap::Args, Clone, Debug, Default)] #[command(next_help_heading = "Diff Formatting Options")] #[command(group(clap::ArgGroup::new("short-format").args(&["summary", "stat", "types", "name_only"])))] #[command(group(clap::ArgGroup::new("long-format").args(&["git", "color_words"])))] diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index d03d1c88b3b..2ebd541d720 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1450,6 +1450,9 @@ configuration. * `-a`, `--all-lines` — Format all lines instead of only modified lines. If the formatter doesn't support formatting only modified lines, then this option has no effect since the formatter always formats all lines. +* `--summary` — Display a summary of fixed files for each modified commit +* `--stat` — Display a histogram of changes for each modified commit +* `-p`, `--patch` — Display a patch of changes for each modified commit diff --git a/cli/tests/test_fix_command.rs b/cli/tests/test_fix_command.rs index 15ce32e66ac..3e77c5d83b0 100644 --- a/cli/tests/test_fix_command.rs +++ b/cli/tests/test_fix_command.rs @@ -2175,3 +2175,275 @@ fn test_fix_with_line_ranges_and_include_unchanged_files_all_lines() { let output = work_dir.run_jj(["file", "show", "empty.txt", "-r", "c2"]); insta::assert_snapshot!(output, @r""); } + +#[test] +fn test_summary() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin!("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--append", "fixed"] + patterns = ["all()"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + work_dir.write_file("file1", "foo\n"); + work_dir.write_file("file2", "bar\n"); + work_dir.run_jj(["describe", "-m", "work item 1"]).success(); + let output = work_dir.run_jj(["fix", "--summary"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Fixed 1 commits of 1 checked. + qpvuntsm 2e8af140 work item 1 + M file1 + M file2 + Working copy (@) now at: qpvuntsm 2e8af140 work item 1 + Parent commit (@-) : zzzzzzzz 00000000 (empty) (no description set) + Added 0 files, modified 2 files, removed 0 files + [EOF] + "###); +} + +#[test] +fn test_summary_multiple_commits() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin!("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--append", "fixed"] + patterns = ["all()"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + work_dir.write_file("file1", "foo\n"); + work_dir.run_jj(["describe", "-m", "work item 1"]).success(); + work_dir.run_jj(["new"]).success(); + work_dir.write_file("file2", "bar\n"); + work_dir.run_jj(["describe", "-m", "work item 2"]).success(); + let output = work_dir.run_jj(["fix", "--summary"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Fixed 2 commits of 2 checked. + kkmpptxz 63ab61b2 work item 2 + M file2 + qpvuntsm e0c80ab2 work item 1 + M file1 + Working copy (@) now at: kkmpptxz 63ab61b2 work item 2 + Parent commit (@-) : qpvuntsm e0c80ab2 work item 1 + Added 0 files, modified 2 files, removed 0 files + [EOF] + "###); +} + +#[test] +fn test_stat() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin!("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--uppercase"] + patterns = ["file1"] + + [fix.tools.tool-2] + command = ["{formatter}", "--append", "fixed"] + patterns = ["file2"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + work_dir.write_file("file1", "foo\nbar\n"); + work_dir.write_file("file2", "baz\n"); + work_dir.run_jj(["describe", "-m", "work item 1"]).success(); + let output = work_dir.run_jj(["fix", "--stat"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Fixed 1 commits of 1 checked. + qpvuntsm 17550c40 work item 1 + file1 | 4 ++-- + file2 | 1 + + 2 files changed, 3 insertions(+), 2 deletions(-) + Working copy (@) now at: qpvuntsm 17550c40 work item 1 + Parent commit (@-) : zzzzzzzz 00000000 (empty) (no description set) + Added 0 files, modified 2 files, removed 0 files + [EOF] + "###); +} + +#[test] +fn test_stat_multiple_commits() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin!("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--uppercase"] + patterns = ["file1"] + + [fix.tools.tool-2] + command = ["{formatter}", "--append", "fixed"] + patterns = ["file2"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + work_dir.write_file("file1", "foo\nbar\n"); + work_dir.run_jj(["describe", "-m", "work item 1"]).success(); + work_dir.run_jj(["new"]).success(); + work_dir.write_file("file2", "baz\n"); + work_dir.run_jj(["describe", "-m", "work item 2"]).success(); + let output = work_dir.run_jj(["fix", "--stat"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Fixed 2 commits of 2 checked. + kkmpptxz 8331a050 work item 2 + file2 | 1 + + 1 file changed, 1 insertion(+), 0 deletions(-) + qpvuntsm 3f596c8f work item 1 + file1 | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + Working copy (@) now at: kkmpptxz 8331a050 work item 2 + Parent commit (@-) : qpvuntsm 3f596c8f work item 1 + Added 0 files, modified 2 files, removed 0 files + [EOF] + "###); +} + +#[test] +fn test_stat_no_changes() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin!("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--uppercase"] + patterns = ["all()"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + work_dir.write_file("file1", "ALREADY UPPERCASE\n"); + work_dir.run_jj(["describe", "-m", "work item 1"]).success(); + let output = work_dir.run_jj(["fix", "--stat"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Fixed 0 commits of 1 checked. + Nothing changed. + [EOF] + "###); +} + +#[test] +fn test_patch() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin!("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--uppercase"] + patterns = ["file1"] + + [fix.tools.tool-2] + command = ["{formatter}", "--append", "fixed"] + patterns = ["file2"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + work_dir.write_file("file1", "foo\nbar\n"); + work_dir.write_file("file2", "baz\n"); + work_dir.run_jj(["describe", "-m", "work item 1"]).success(); + let output = work_dir.run_jj(["fix", "-p"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Fixed 1 commits of 1 checked. + qpvuntsm 17550c40 work item 1 + Modified regular file file1: + 1 : foo + 2 : bar + 1: FOO + 2: BAR + Modified regular file file2: + 1 1: baz + 2: fixed + Working copy (@) now at: qpvuntsm 17550c40 work item 1 + Parent commit (@-) : zzzzzzzz 00000000 (empty) (no description set) + Added 0 files, modified 2 files, removed 0 files + [EOF] + "###); +} + +#[test] +fn test_patch_multiple_commits() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin!("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--uppercase"] + patterns = ["file1"] + + [fix.tools.tool-2] + command = ["{formatter}", "--append", "fixed"] + patterns = ["file2"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + work_dir.write_file("file1", "foo\nbar\n"); + work_dir.run_jj(["describe", "-m", "work item 1"]).success(); + work_dir.run_jj(["new"]).success(); + work_dir.write_file("file2", "baz\n"); + work_dir.run_jj(["describe", "-m", "work item 2"]).success(); + let output = work_dir.run_jj(["fix", "-p"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Fixed 2 commits of 2 checked. + kkmpptxz 8331a050 work item 2 + Modified regular file file2: + 1 1: baz + 2: fixed + qpvuntsm 3f596c8f work item 1 + Modified regular file file1: + 1 : foo + 2 : bar + 1: FOO + 2: BAR + Working copy (@) now at: kkmpptxz 8331a050 work item 2 + Parent commit (@-) : qpvuntsm 3f596c8f work item 1 + Added 0 files, modified 2 files, removed 0 files + [EOF] + "###); +} diff --git a/docs/paid_contributors.md b/docs/paid_contributors.md index bf367a60ff3..5f23b4706f6 100644 --- a/docs/paid_contributors.md +++ b/docs/paid_contributors.md @@ -39,6 +39,7 @@ See [contribution docs](contributing.md#code-reviews) for details on this policy * honglooker * hooper * incognito124 +* jlndr * jonathantanmy * josephlou5 * kevincliao