diff --git a/CHANGELOG.md b/CHANGELOG.md index 048d3b289..825313b18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * The default log format now uses the committer timestamp instead of the author timestamp. +* `jj log --summary --patch` now shows both summary and diff outputs. + ### Fixed bugs * When sharing the working copy with a Git repo, we used to forget to export diff --git a/src/commands.rs b/src/commands.rs index e8382d0d9..21d4dca9d 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1304,7 +1304,7 @@ fn cmd_diff(ui: &mut Ui, command: &CommandHelper, args: &DiffArgs) -> Result<(), &from_tree, &to_tree, matcher.as_ref(), - diff_util::diff_format_for(ui, &args.format), + &diff_util::diff_formats_for(ui, &args.format), )?; Ok(()) } @@ -1344,7 +1344,7 @@ fn cmd_show(ui: &mut Ui, command: &CommandHelper, args: &ShowArgs) -> Result<(), &workspace_command, &commit, &EverythingMatcher, - diff_util::diff_format_for(ui, &args.format), + &diff_util::diff_formats_for(ui, &args.format), )?; Ok(()) } @@ -1514,7 +1514,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C }; let store = repo.store(); - let diff_format = diff_util::diff_format_for_log(ui, &args.diff_format, args.patch); + let diff_formats = diff_util::diff_formats_for_log(ui, &args.diff_format, args.patch); let template_string = match &args.template { Some(value) => value.to_string(), @@ -1581,14 +1581,14 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C if !buffer.ends_with(b"\n") { buffer.push(b'\n'); } - if let Some(diff_format) = diff_format { + if !diff_formats.is_empty() { let mut formatter = ui.new_formatter(&mut buffer); diff_util::show_patch( formatter.as_mut(), &workspace_command, &commit, matcher.as_ref(), - diff_format, + &diff_formats, )?; } let node_symbol = if is_checkout { b"@" } else { b"o" }; @@ -1608,13 +1608,13 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C for index_entry in iter { let commit = store.get_commit(&index_entry.commit_id())?; template.format(&commit, formatter)?; - if let Some(diff_format) = diff_format { + if !diff_formats.is_empty() { diff_util::show_patch( formatter, &workspace_command, &commit, matcher.as_ref(), - diff_format, + &diff_formats, )?; } } @@ -1654,7 +1654,7 @@ fn cmd_obslog(ui: &mut Ui, command: &CommandHelper, args: &ObslogArgs) -> Result .view() .get_wc_commit_id(&workspace_id); - let diff_format = diff_util::diff_format_for_log(ui, &args.diff_format, args.patch); + let diff_formats = diff_util::diff_formats_for_log(ui, &args.diff_format, args.patch); let template_string = match &args.template { Some(value) => value.to_string(), @@ -1691,13 +1691,13 @@ fn cmd_obslog(ui: &mut Ui, command: &CommandHelper, args: &ObslogArgs) -> Result if !buffer.ends_with(b"\n") { buffer.push(b'\n'); } - if let Some(diff_format) = diff_format { + if !diff_formats.is_empty() { let mut formatter = ui.new_formatter(&mut buffer); show_predecessor_patch( formatter.as_mut(), &workspace_command, &commit, - diff_format, + &diff_formats, )?; } let node_symbol = if Some(commit.id()) == wc_commit_id { @@ -1710,8 +1710,8 @@ fn cmd_obslog(ui: &mut Ui, command: &CommandHelper, args: &ObslogArgs) -> Result } else { for commit in commits { template.format(&commit, formatter)?; - if let Some(diff_format) = diff_format { - show_predecessor_patch(formatter, &workspace_command, &commit, diff_format)?; + if !diff_formats.is_empty() { + show_predecessor_patch(formatter, &workspace_command, &commit, &diff_formats)?; } } } @@ -1723,7 +1723,7 @@ fn show_predecessor_patch( formatter: &mut dyn Formatter, workspace_command: &WorkspaceCommandHelper, commit: &Commit, - diff_format: DiffFormat, + diff_formats: &[DiffFormat], ) -> Result<(), CommandError> { let predecessors = commit.predecessors(); let predecessor = match predecessors.first() { @@ -1737,7 +1737,7 @@ fn show_predecessor_patch( &predecessor_tree, &commit.tree(), &EverythingMatcher, - diff_format, + diff_formats, ) } @@ -1759,7 +1759,7 @@ fn cmd_interdiff( &from_tree, &to.tree(), matcher.as_ref(), - diff_util::diff_format_for(ui, &args.format), + &diff_util::diff_formats_for(ui, &args.format), ) } @@ -2457,7 +2457,7 @@ fn description_template_for_cmd_split( from_tree, to_tree, &EverythingMatcher, - DiffFormat::Summary, + &[DiffFormat::Summary], )?; let diff_summary = std::str::from_utf8(&diff_summary_bytes).expect( "Summary diffs and repo paths must always be valid UTF8.", diff --git a/src/diff_util.rs b/src/diff_util.rs index f6ae18b87..2eee87e65 100644 --- a/src/diff_util.rs +++ b/src/diff_util.rs @@ -17,7 +17,6 @@ use std::io; use std::ops::Range; use std::sync::Arc; -use clap::ArgGroup; use itertools::Itertools; use jujutsu_lib::backend::TreeValue; use jujutsu_lib::commit::Commit; @@ -34,7 +33,6 @@ use crate::formatter::{Formatter, PlainTextFormatter}; use crate::ui::Ui; #[derive(clap::Args, Clone, Debug)] -#[command(group(ArgGroup::new("format").args(&["summary", "git", "color_words"])))] pub struct DiffFormatArgs { /// For each path, show only whether it was modified, added, or removed #[arg(long, short)] @@ -54,20 +52,37 @@ pub enum DiffFormat { ColorWords, } -pub fn diff_format_for(ui: &Ui, args: &DiffFormatArgs) -> DiffFormat { - if args.summary { - DiffFormat::Summary - } else if args.git { - DiffFormat::Git - } else if args.color_words { - DiffFormat::ColorWords +/// Returns a list of requested diff formats, which will never be empty. +pub fn diff_formats_for(ui: &Ui, args: &DiffFormatArgs) -> Vec { + let formats = diff_formats_from_args(args); + if formats.is_empty() { + vec![default_diff_format(ui)] } else { - default_diff_format(ui) + formats } } -pub fn diff_format_for_log(ui: &Ui, args: &DiffFormatArgs, patch: bool) -> Option { - (patch || args.git || args.color_words || args.summary).then(|| diff_format_for(ui, args)) +/// Returns a list of requested diff formats for log-like commands, which may be +/// empty. +pub fn diff_formats_for_log(ui: &Ui, args: &DiffFormatArgs, patch: bool) -> Vec { + let mut formats = diff_formats_from_args(args); + // --patch implies default if no format other than --summary is specified + if patch && matches!(formats.as_slice(), [] | [DiffFormat::Summary]) { + formats.push(default_diff_format(ui)); + formats.dedup(); + } + formats +} + +fn diff_formats_from_args(args: &DiffFormatArgs) -> Vec { + [ + (args.summary, DiffFormat::Summary), + (args.git, DiffFormat::Git), + (args.color_words, DiffFormat::ColorWords), + ] + .into_iter() + .filter_map(|(arg, format)| arg.then(|| format)) + .collect() } fn default_diff_format(ui: &Ui) -> DiffFormat { @@ -85,18 +100,20 @@ pub fn show_diff( from_tree: &Tree, to_tree: &Tree, matcher: &dyn Matcher, - format: DiffFormat, + formats: &[DiffFormat], ) -> Result<(), CommandError> { - let tree_diff = from_tree.diff(to_tree, matcher); - match format { - DiffFormat::Summary => { - show_diff_summary(formatter, workspace_command, tree_diff)?; - } - DiffFormat::Git => { - show_git_diff(formatter, workspace_command, tree_diff)?; - } - DiffFormat::ColorWords => { - show_color_words_diff(formatter, workspace_command, tree_diff)?; + for format in formats { + let tree_diff = from_tree.diff(to_tree, matcher); + match format { + DiffFormat::Summary => { + show_diff_summary(formatter, workspace_command, tree_diff)?; + } + DiffFormat::Git => { + show_git_diff(formatter, workspace_command, tree_diff)?; + } + DiffFormat::ColorWords => { + show_color_words_diff(formatter, workspace_command, tree_diff)?; + } } } Ok(()) @@ -107,7 +124,7 @@ pub fn show_patch( workspace_command: &WorkspaceCommandHelper, commit: &Commit, matcher: &dyn Matcher, - format: DiffFormat, + formats: &[DiffFormat], ) -> Result<(), CommandError> { let parents = commit.parents(); let from_tree = rewrite::merge_commit_trees(workspace_command.repo().as_repo_ref(), &parents); @@ -118,7 +135,7 @@ pub fn show_patch( &from_tree, &to_tree, matcher, - format, + formats, ) } @@ -127,7 +144,7 @@ pub fn diff_as_bytes( from_tree: &Tree, to_tree: &Tree, matcher: &dyn Matcher, - format: DiffFormat, + formats: &[DiffFormat], ) -> Result, CommandError> { let mut diff_bytes: Vec = vec![]; let mut formatter = PlainTextFormatter::new(&mut diff_bytes); @@ -137,7 +154,7 @@ pub fn diff_as_bytes( from_tree, to_tree, matcher, - format, + formats, )?; Ok(diff_bytes) } diff --git a/tests/test_diff_command.rs b/tests/test_diff_command.rs index 406d9731d..729ead3d3 100644 --- a/tests/test_diff_command.rs +++ b/tests/test_diff_command.rs @@ -71,6 +71,34 @@ fn test_diff_basic() { @@ -1,0 +1,1 @@ +foo "###); + + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "--git"]); + insta::assert_snapshot!(stdout, @r###" + R file1 + M file2 + A file3 + diff --git a/file1 b/file1 + deleted file mode 100644 + index 257cc5642c..0000000000 + --- a/file1 + +++ /dev/null + @@ -1,1 +1,0 @@ + -foo + diff --git a/file2 b/file2 + index 257cc5642c...3bd1f0e297 100644 + --- a/file2 + +++ b/file2 + @@ -1,1 +1,2 @@ + foo + +bar + diff --git a/file3 b/file3 + new file mode 100644 + index 0000000000..257cc5642c + --- /dev/null + +++ b/file3 + @@ -1,0 +1,1 @@ + +foo + "###); } #[test] diff --git a/tests/test_log_command.rs b/tests/test_log_command.rs index c99386ce1..2c9c88fbf 100644 --- a/tests/test_log_command.rs +++ b/tests/test_log_command.rs @@ -68,6 +68,82 @@ fn test_log_with_or_without_diff() { (no description set) "###); + // `-p` for default diff output, `-s` for summary + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "description", "-p", "-s"]); + insta::assert_snapshot!(stdout, @r###" + @ a new commit + | M file1 + | Modified regular file file1: + | 1 1: foo + | 2: bar + o add a file + | A file1 + | Added regular file file1: + | 1: foo + o (no description set) + "###); + + // `-s` for summary, `--git` for git diff (which implies `-p`) + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "description", "-s", "--git"]); + insta::assert_snapshot!(stdout, @r###" + @ a new commit + | M file1 + | diff --git a/file1 b/file1 + | index 257cc5642c...3bd1f0e297 100644 + | --- a/file1 + | +++ b/file1 + | @@ -1,1 +1,2 @@ + | foo + | +bar + o add a file + | A file1 + | diff --git a/file1 b/file1 + | new file mode 100644 + | index 0000000000..257cc5642c + | --- /dev/null + | +++ b/file1 + | @@ -1,0 +1,1 @@ + | +foo + o (no description set) + "###); + + // `-p` enables default "summary" output, so `-s` is noop + let stdout = test_env.jj_cmd_success( + &repo_path, + &[ + "log", + "-T", + "description", + "-p", + "-s", + "--config-toml=diff.format='summary'", + ], + ); + insta::assert_snapshot!(stdout, @r###" + @ a new commit + | M file1 + o add a file + | A file1 + o (no description set) + "###); + + // `-p` enables default "color-words" diff output, so `--color-words` is noop + let stdout = test_env.jj_cmd_success( + &repo_path, + &["log", "-T", "description", "-p", "--color-words"], + ); + insta::assert_snapshot!(stdout, @r###" + @ a new commit + | Modified regular file file1: + | 1 1: foo + | 2: bar + o add a file + | Added regular file file1: + | 1: foo + o (no description set) + "###); + + // `--git` enables git diff, so `-p` is noop let stdout = test_env.jj_cmd_success( &repo_path, &["log", "-T", "description", "--no-graph", "-p", "--git"], @@ -92,7 +168,45 @@ fn test_log_with_or_without_diff() { (no description set) "###); - // `-s` implies `-p`, with or without graph + // Both formats enabled if `--git` and `--color-words` are explicitly specified + let stdout = test_env.jj_cmd_success( + &repo_path, + &[ + "log", + "-T", + "description", + "--no-graph", + "-p", + "--git", + "--color-words", + ], + ); + insta::assert_snapshot!(stdout, @r###" + a new commit + diff --git a/file1 b/file1 + index 257cc5642c...3bd1f0e297 100644 + --- a/file1 + +++ b/file1 + @@ -1,1 +1,2 @@ + foo + +bar + Modified regular file file1: + 1 1: foo + 2: bar + add a file + diff --git a/file1 b/file1 + new file mode 100644 + index 0000000000..257cc5642c + --- /dev/null + +++ b/file1 + @@ -1,0 +1,1 @@ + +foo + Added regular file file1: + 1: foo + (no description set) + "###); + + // `-s` with or without graph let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "description", "-s"]); insta::assert_snapshot!(stdout, @r###" @ a new commit