Skip to content

Commit

Permalink
diff: add "diff" label globally by outer show_diff/patch() functions
Browse files Browse the repository at this point in the history
It's not so important, but this removes duplicated "diff" labels from template
output. Perhaps, this also fixes "diff access-denied" label in file-by-file
external diffs.

The inner show_*() functions no longer add "diff" labels, but that's okay
because all CLI callers (except for the templater) use DiffRenderer.
  • Loading branch information
yuja committed Aug 1, 2024
1 parent 07b0b06 commit d0f6f42
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 103 deletions.
150 changes: 75 additions & 75 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,20 @@ impl<'a> DiffRenderer<'a> {
to_tree: &MergedTree,
matcher: &dyn Matcher,
width: usize,
) -> Result<(), DiffRenderError> {
formatter.with_label("diff", |formatter| {
self.show_diff_inner(ui, formatter, from_tree, to_tree, matcher, width)
})
}

fn show_diff_inner(
&self,
ui: &Ui,
formatter: &mut dyn Formatter,
from_tree: &MergedTree,
to_tree: &MergedTree,
matcher: &dyn Matcher,
width: usize,
) -> Result<(), DiffRenderError> {
let store = self.repo.store();
let path_converter = self.path_converter;
Expand Down Expand Up @@ -533,7 +547,6 @@ pub fn show_color_words_diff(
path_converter: &RepoPathUiConverter,
num_context_lines: usize,
) -> Result<(), DiffRenderError> {
formatter.push_label("diff")?;
let mut diff_stream = materialized_diff_stream(store, tree_diff);
async {
while let Some((path, diff)) = diff_stream.next().await {
Expand Down Expand Up @@ -653,11 +666,9 @@ pub fn show_color_words_diff(
}
}
}
Ok::<(), DiffRenderError>(())
Ok(())
}
.block_on()?;
formatter.pop_label()?;
Ok(())
.block_on()
}

pub fn show_file_by_file_diff(
Expand Down Expand Up @@ -1001,8 +1012,6 @@ pub fn show_git_diff(
tree_diff: TreeDiffStream,
num_context_lines: usize,
) -> Result<(), DiffRenderError> {
formatter.push_label("diff")?;

let mut diff_stream = materialized_diff_stream(store, tree_diff);
async {
while let Some((path, diff)) = diff_stream.next().await {
Expand Down Expand Up @@ -1071,11 +1080,9 @@ pub fn show_git_diff(
)?;
}
}
Ok::<(), DiffRenderError>(())
Ok(())
}
.block_on()?;
formatter.pop_label()?;
Ok(())
.block_on()
}

#[instrument(skip_all)]
Expand All @@ -1084,24 +1091,22 @@ pub fn show_diff_summary(
mut tree_diff: TreeDiffStream,
path_converter: &RepoPathUiConverter,
) -> io::Result<()> {
formatter.with_label("diff", |formatter| -> io::Result<()> {
async {
while let Some((repo_path, diff)) = tree_diff.next().await {
let (before, after) = diff.unwrap();
let ui_path = path_converter.format_file_path(&repo_path);
if before.is_present() && after.is_present() {
writeln!(formatter.labeled("modified"), "M {ui_path}")?;
} else if before.is_absent() {
writeln!(formatter.labeled("added"), "A {ui_path}")?;
} else {
// `R` could be interpreted as "renamed"
writeln!(formatter.labeled("removed"), "D {ui_path}")?;
}
async {
while let Some((repo_path, diff)) = tree_diff.next().await {
let (before, after) = diff.unwrap();
let ui_path = path_converter.format_file_path(&repo_path);
if before.is_present() && after.is_present() {
writeln!(formatter.labeled("modified"), "M {ui_path}")?;
} else if before.is_absent() {
writeln!(formatter.labeled("added"), "A {ui_path}")?;
} else {
// `R` could be interpreted as "renamed"
writeln!(formatter.labeled("removed"), "D {ui_path}")?;
}
Ok(())
}
.block_on()
})
Ok(())
}
.block_on()
}

struct DiffStat {
Expand Down Expand Up @@ -1178,40 +1183,37 @@ pub fn show_diff_stat(
max_bar_length as f64 / max_diffs as f64
};

formatter.with_label("diff", |formatter| {
let mut total_added = 0;
let mut total_removed = 0;
let total_files = stats.len();
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;
// replace start of path with ellipsis if the path is too long
let (path, path_width) = text_util::elide_start(&stat.path, "...", max_path_width);
let path_pad_width = max_path_width - path_width;
write!(
formatter,
"{path}{:path_pad_width$} | {:>number_padding$}{}",
"", // pad to max_path_width
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{}(-)",
total_files,
if total_files == 1 { "" } else { "s" },
total_added,
if total_added == 1 { "" } else { "s" },
total_removed,
if total_removed == 1 { "" } else { "s" },
let mut total_added = 0;
let mut total_removed = 0;
let total_files = stats.len();
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;
// replace start of path with ellipsis if the path is too long
let (path, path_width) = text_util::elide_start(&stat.path, "...", max_path_width);
let path_pad_width = max_path_width - path_width;
write!(
formatter,
"{path}{:path_pad_width$} | {:>number_padding$}{}",
"", // pad to max_path_width
stat.added + stat.removed,
if bar_added + bar_removed > 0 { " " } else { "" },
)?;
io::Result::Ok(())
})?;
write!(formatter.labeled("added"), "{}", "+".repeat(bar_added))?;
writeln!(formatter.labeled("removed"), "{}", "-".repeat(bar_removed))?;
}
writeln!(
formatter.labeled("stat-summary"),
"{} file{} changed, {} insertion{}(+), {} deletion{}(-)",
total_files,
if total_files == 1 { "" } else { "s" },
total_added,
if total_added == 1 { "" } else { "s" },
total_removed,
if total_removed == 1 { "" } else { "s" },
)?;
Ok(())
}

Expand All @@ -1220,22 +1222,20 @@ pub fn show_types(
mut tree_diff: TreeDiffStream,
path_converter: &RepoPathUiConverter,
) -> io::Result<()> {
formatter.with_label("diff", |formatter| {
async {
while let Some((repo_path, diff)) = tree_diff.next().await {
let (before, after) = diff.unwrap();
writeln!(
formatter.labeled("modified"),
"{}{} {}",
diff_summary_char(&before),
diff_summary_char(&after),
path_converter.format_file_path(&repo_path)
)?;
}
Ok(())
async {
while let Some((repo_path, diff)) = tree_diff.next().await {
let (before, after) = diff.unwrap();
writeln!(
formatter.labeled("modified"),
"{}{} {}",
diff_summary_char(&before),
diff_summary_char(&after),
path_converter.format_file_path(&repo_path)
)?;
}
.block_on()
})
Ok(())
}
.block_on()
}

fn diff_summary_char(value: &MergedTreeValue) -> char {
Expand Down
56 changes: 28 additions & 28 deletions cli/tests/test_commit_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1015,37 +1015,37 @@ fn test_log_diff_predefined_formats() {
);
insta::assert_snapshot!(stdout, @r###"
<<log::=== color_words ===>>
[38;5;3m<<log diff color_words diff header::Modified regular file file1:>>[39m
[38;5;1m<<log diff color_words diff removed line_number:: 1>>[39m<<log diff color_words diff:: >>[38;5;2m<<log diff color_words diff added line_number:: 1>>[39m<<log diff color_words diff::: a>>
[38;5;1m<<log diff color_words diff removed line_number:: 2>>[39m<<log diff color_words diff:: >>[38;5;2m<<log diff color_words diff added line_number:: 2>>[39m<<log diff color_words diff::: b>>
<<log diff color_words diff:: >>[38;5;2m<<log diff color_words diff added line_number:: 3>>[39m<<log diff color_words diff::: >>[4m[38;5;2m<<log diff color_words diff added token::c>>[24m[39m
[38;5;3m<<log diff color_words diff header::Modified regular file file2:>>[39m
[38;5;1m<<log diff color_words diff removed line_number:: 1>>[39m<<log diff color_words diff:: >>[38;5;2m<<log diff color_words diff added line_number:: 1>>[39m<<log diff color_words diff::: >>[4m[38;5;1m<<log diff color_words diff removed token::a>>[38;5;2m<<log diff color_words diff added token::b>>[24m[39m<<log diff color_words diff::>>
<<log diff color_words diff:: >>[38;5;2m<<log diff color_words diff added line_number:: 2>>[39m<<log diff color_words diff::: >>[4m[38;5;2m<<log diff color_words diff added token::c>>[24m[39m
[38;5;3m<<log diff color_words header::Modified regular file file1:>>[39m
[38;5;1m<<log diff color_words removed line_number:: 1>>[39m<<log diff color_words:: >>[38;5;2m<<log diff color_words added line_number:: 1>>[39m<<log diff color_words::: a>>
[38;5;1m<<log diff color_words removed line_number:: 2>>[39m<<log diff color_words:: >>[38;5;2m<<log diff color_words added line_number:: 2>>[39m<<log diff color_words::: b>>
<<log diff color_words:: >>[38;5;2m<<log diff color_words added line_number:: 3>>[39m<<log diff color_words::: >>[4m[38;5;2m<<log diff color_words added token::c>>[24m[39m
[38;5;3m<<log diff color_words header::Modified regular file file2:>>[39m
[38;5;1m<<log diff color_words removed line_number:: 1>>[39m<<log diff color_words:: >>[38;5;2m<<log diff color_words added line_number:: 1>>[39m<<log diff color_words::: >>[4m[38;5;1m<<log diff color_words removed token::a>>[38;5;2m<<log diff color_words added token::b>>[24m[39m<<log diff color_words::>>
<<log diff color_words:: >>[38;5;2m<<log diff color_words added line_number:: 2>>[39m<<log diff color_words::: >>[4m[38;5;2m<<log diff color_words added token::c>>[24m[39m
<<log::=== git ===>>
[1m<<log diff git diff file_header::diff --git a/file1 b/file1>>[0m
[1m<<log diff git diff file_header::index 422c2b7ab3..de980441c3 100644>>[0m
[1m<<log diff git diff file_header::--- a/file1>>[0m
[1m<<log diff git diff file_header::+++ b/file1>>[0m
[38;5;6m<<log diff git diff hunk_header::@@ -1,2 +1,3 @@>>[39m
<<log diff git diff context:: a>>
<<log diff git diff context:: b>>
[38;5;2m<<log diff git diff added::+>>[4m<<log diff git diff added token::c>>[24m[39m
[1m<<log diff git diff file_header::diff --git a/file2 b/file2>>[0m
[1m<<log diff git diff file_header::index 7898192261..9ddeb5c484 100644>>[0m
[1m<<log diff git diff file_header::--- a/file2>>[0m
[1m<<log diff git diff file_header::+++ b/file2>>[0m
[38;5;6m<<log diff git diff hunk_header::@@ -1,1 +1,2 @@>>[39m
[38;5;1m<<log diff git diff removed::->>[4m<<log diff git diff removed token::a>>[24m<<log diff git diff removed::>>[39m
[38;5;2m<<log diff git diff added::+>>[4m<<log diff git diff added token::b>>[24m<<log diff git diff added::>>[39m
[38;5;2m<<log diff git diff added::+>>[4m<<log diff git diff added token::c>>[24m[39m
[1m<<log diff git file_header::diff --git a/file1 b/file1>>[0m
[1m<<log diff git file_header::index 422c2b7ab3..de980441c3 100644>>[0m
[1m<<log diff git file_header::--- a/file1>>[0m
[1m<<log diff git file_header::+++ b/file1>>[0m
[38;5;6m<<log diff git hunk_header::@@ -1,2 +1,3 @@>>[39m
<<log diff git context:: a>>
<<log diff git context:: b>>
[38;5;2m<<log diff git added::+>>[4m<<log diff git added token::c>>[24m[39m
[1m<<log diff git file_header::diff --git a/file2 b/file2>>[0m
[1m<<log diff git file_header::index 7898192261..9ddeb5c484 100644>>[0m
[1m<<log diff git file_header::--- a/file2>>[0m
[1m<<log diff git file_header::+++ b/file2>>[0m
[38;5;6m<<log diff git hunk_header::@@ -1,1 +1,2 @@>>[39m
[38;5;1m<<log diff git removed::->>[4m<<log diff git removed token::a>>[24m<<log diff git removed::>>[39m
[38;5;2m<<log diff git added::+>>[4m<<log diff git added token::b>>[24m<<log diff git added::>>[39m
[38;5;2m<<log diff git added::+>>[4m<<log diff git added token::c>>[24m[39m
<<log::=== stat ===>>
<<log diff stat diff::file1 | 1 >>[38;5;2m<<log diff stat diff added::+>>[38;5;1m<<log diff stat diff removed::>>[39m
<<log diff stat diff::file2 | 3 >>[38;5;2m<<log diff stat diff added::++>>[38;5;1m<<log diff stat diff removed::->>[39m
<<log diff stat diff stat-summary::2 files changed, 3 insertions(+), 1 deletion(-)>>
<<log diff stat::file1 | 1 >>[38;5;2m<<log diff stat added::+>>[38;5;1m<<log diff stat removed::>>[39m
<<log diff stat::file2 | 3 >>[38;5;2m<<log diff stat added::++>>[38;5;1m<<log diff stat removed::->>[39m
<<log diff stat stat-summary::2 files changed, 3 insertions(+), 1 deletion(-)>>
<<log::=== summary ===>>
[38;5;6m<<log diff summary diff modified::M file1>>[39m
[38;5;6m<<log diff summary diff modified::M file2>>[39m
[38;5;6m<<log diff summary modified::M file1>>[39m
[38;5;6m<<log diff summary modified::M file2>>[39m
"###);

// cwd != workspace root
Expand Down

0 comments on commit d0f6f42

Please sign in to comment.