From 5bd726f77dc221c0eba8d715ed32b30e426eb66d Mon Sep 17 00:00:00 2001 From: Oscar Bonilla <6f6231@gmail.com> Date: Sun, 13 Aug 2023 12:30:57 -0700 Subject: [PATCH] Add jj diffs --stat option --- CHANGELOG.md | 2 + cli/src/diff_util.rs | 114 ++++++++++++++++++++++++++++++++- cli/tests/test_diff_command.rs | 68 ++++++++++++++++++++ 3 files changed, 183 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb2b6d3a0..722a2abbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +* `jj diff --stat` has been implemented. It shows a histogram of the changes, + same as `git diff --stat`. Fixes [#2066](https://github.com/martinvonz/jj/issues/2066) ### Breaking changes diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index c6b259f18..da0629390 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::cmp::max; use std::collections::VecDeque; use std::io; use std::ops::Range; @@ -36,12 +37,15 @@ use crate::merge_tools::{self, ExternalMergeTool}; use crate::ui::Ui; #[derive(clap::Args, Clone, Debug)] -#[command(group(clap::ArgGroup::new("short-format").args(&["summary", "types"])))] +#[command(group(clap::ArgGroup::new("short-format").args(&["summary", "stat", "types"])))] #[command(group(clap::ArgGroup::new("long-format").args(&["git", "color_words", "tool"])))] pub struct DiffFormatArgs { /// For each path, show only whether it was modified, added, or removed #[arg(long, short)] pub summary: bool, + // Show a histogram of the changes + #[arg(long)] + pub stat: bool, /// For each path, show only its type before and after /// /// The diff is shown as two letters. The first letter indicates the type @@ -65,6 +69,7 @@ pub struct DiffFormatArgs { #[derive(Clone, Debug, Eq, PartialEq)] pub enum DiffFormat { Summary, + Stat, Types, Git, ColorWords, @@ -109,6 +114,7 @@ fn diff_formats_from_args( (args.types, DiffFormat::Types), (args.git, DiffFormat::Git), (args.color_words, DiffFormat::ColorWords), + (args.stat, DiffFormat::Stat), ] .into_iter() .filter_map(|(arg, format)| arg.then_some(format)) @@ -141,6 +147,7 @@ fn default_diff_format(settings: &UserSettings) -> Result Ok(DiffFormat::Types), "git" => Ok(DiffFormat::Git), "color-words" => Ok(DiffFormat::ColorWords), + "stat" => Ok(DiffFormat::Stat), _ => Err(config::ConfigError::Message(format!( "invalid diff format: {name}" ))), @@ -162,6 +169,10 @@ pub fn show_diff( let tree_diff = from_tree.diff(to_tree, matcher); show_diff_summary(formatter, workspace_command, tree_diff)?; } + DiffFormat::Stat => { + let tree_diff = from_tree.diff(to_tree, matcher); + show_diff_stat(ui, formatter, workspace_command, tree_diff)?; + } DiffFormat::Types => { let tree_diff = from_tree.diff(to_tree, matcher); show_types(formatter, workspace_command, tree_diff)?; @@ -749,6 +760,107 @@ pub fn show_diff_summary( }) } +struct DiffStat { + path: String, + added: usize, + removed: usize, +} + +fn get_diff_stat(path: String, left_content: &[u8], right_content: &[u8]) -> DiffStat { + let hunks = unified_diff_hunks(left_content, right_content, 0); + let mut added = 0; + let mut removed = 0; + for hunk in hunks { + for (line_type, _content) in hunk.lines { + match line_type { + DiffLineType::Context => {} + DiffLineType::Removed => removed += 1, + DiffLineType::Added => added += 1, + } + } + } + DiffStat { + path, + added, + removed, + } +} + +pub fn show_diff_stat( + ui: &Ui, + formatter: &mut dyn Formatter, + workspace_command: &WorkspaceCommandHelper, + tree_diff: TreeDiffIterator, +) -> Result<(), CommandError> { + let mut stats: Vec = vec![]; + let mut max_path_length = 0; + let mut max_diffs = 0; + for (repo_path, diff) in tree_diff { + let path = workspace_command.format_file_path(&repo_path); + let mut left_content: Vec = vec![]; + let mut right_content: Vec = vec![]; + match diff { + tree::Diff::Modified(left, right) => { + left_content = diff_content(workspace_command.repo(), &repo_path, &left)?; + right_content = diff_content(workspace_command.repo(), &repo_path, &right)?; + } + tree::Diff::Added(right) => { + right_content = diff_content(workspace_command.repo(), &repo_path, &right)?; + } + tree::Diff::Removed(left) => { + left_content = diff_content(workspace_command.repo(), &repo_path, &left)?; + } + } + max_path_length = max(max_path_length, path.len()); + let stat = get_diff_stat(path, &left_content, &right_content); + max_diffs = max(max_diffs, stat.added + stat.removed); + stats.push(stat); + } + + let display_width = usize::from(ui.term_width().unwrap_or(80)) - 4; // padding + let max_bar_length = + display_width - max_path_length - " | ".len() - max_diffs.to_string().len() - 1; + let factor = if max_diffs < max_bar_length { + 1.0 + } else { + max_bar_length as f64 / max_diffs as f64 + }; + let number_padding = max_diffs.to_string().len(); + + formatter.with_label("diff", |formatter| { + let mut total_added = 0; + let mut total_removed = 0; + for stat in &stats { + total_added += stat.added; + total_removed += stat.removed; + let bar_added = (stat.added as f64 * factor).ceil() as usize; + let bar_removed = (stat.removed as f64 * factor).ceil() as usize; + // pad to max_path_length + write!( + formatter, + "{:number_padding$}{}", + stat.path, + stat.added + stat.removed, + if bar_added + bar_removed > 0 { " " } else { "" }, + )?; + write!(formatter.labeled("added"), "{}", "+".repeat(bar_added))?; + writeln!(formatter.labeled("removed"), "{}", "-".repeat(bar_removed))?; + } + writeln!( + formatter.labeled("stat-summary"), + "{} file{} changed, {} insertion{}(+), {} deletion{}(-)", + stats.len(), + if stats.len() == 1 { "" } else { "s" }, + total_added, + if total_added == 1 { "" } else { "s" }, + total_removed, + if total_removed == 1 { "" } else { "s" }, + )?; + Ok(()) + })?; + Ok(()) +} + pub fn show_types( formatter: &mut dyn Formatter, workspace_command: &WorkspaceCommandHelper, diff --git a/cli/tests/test_diff_command.rs b/cli/tests/test_diff_command.rs index 7e4acafbd..fadef3e28 100644 --- a/cli/tests/test_diff_command.rs +++ b/cli/tests/test_diff_command.rs @@ -106,6 +106,14 @@ fn test_diff_basic() { @@ -1,0 +1,1 @@ +foo "###); + + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]); + insta::assert_snapshot!(stdout, @r###" + file1 | 1 - + file2 | 1 + + file3 | 1 + + 3 files changed, 2 insertions(+), 1 deletion(-) + "###); } #[test] @@ -128,6 +136,12 @@ fn test_diff_empty() { Removed regular file file1: (empty) "###); + + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]); + insta::assert_snapshot!(stdout, @r###" + file1 | 0 + 1 file changed, 0 insertions(+), 0 deletions(-) + "###); } #[test] @@ -337,6 +351,24 @@ fn test_diff_relative_paths() { -foo1 +bar1 "###); + + let stdout = test_env.jj_cmd_success(&repo_path.join("dir1"), &["diff", "--stat"]); + #[cfg(unix)] + insta::assert_snapshot!(stdout, @r###" + file2 | 2 +- + subdir1/file3 | 2 +- + ../dir2/file4 | 2 +- + ../file1 | 2 +- + 4 files changed, 4 insertions(+), 4 deletions(-) + "###); + #[cfg(windows)] + insta::assert_snapshot!(stdout, @r###" + file2 | 2 +- + subdir1\file3 | 2 +- + ..\dir2\file4 | 2 +- + ..\file1 | 2 +- + 4 files changed, 4 insertions(+), 4 deletions(-) + "###); } #[test] @@ -384,6 +416,13 @@ fn test_diff_missing_newline() { +foo \ No newline at end of file "###); + + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]); + insta::assert_snapshot!(stdout, @r###" + file1 | 3 ++- + file2 | 3 +-- + 2 files changed, 3 insertions(+), 3 deletions(-) + "###); } #[test] @@ -715,3 +754,32 @@ fn test_diff_external_tool() { Tool exited with a non-zero code (run with --verbose to see the exact invocation). Exit code: 1. "###); } + +#[test] +fn test_diff_stat() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + std::fs::write(repo_path.join("file1"), "foo\n").unwrap(); + + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]); + insta::assert_snapshot!(stdout, @r###" + file1 | 1 + + 1 file changed, 1 insertion(+), 0 deletions(-) + "###); + + test_env.jj_cmd_success(&repo_path, &["new"]); + + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]); + insta::assert_snapshot!(stdout, @"0 files changed, 0 insertions(+), 0 deletions(-)"); + + std::fs::write(repo_path.join("file1"), "foo\nbar\n").unwrap(); + test_env.jj_cmd_success(&repo_path, &["new"]); + std::fs::write(repo_path.join("file1"), "bar\n").unwrap(); + + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]); + insta::assert_snapshot!(stdout, @r###" + file1 | 1 - + 1 file changed, 0 insertions(+), 1 deletion(-) + "###); +}