From 1c7d2b9a965b925ce20136da970f154ed17413ec Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 1 Sep 2023 15:28:37 +0900 Subject: [PATCH] cli: make diff stat calculate path length based on unicode width --- cli/src/diff_util.rs | 33 +++++++++------------------ cli/tests/test_diff_command.rs | 41 +++++++++++++++++++--------------- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 3ca32d593..4dacd5344 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -31,10 +31,12 @@ use jj_lib::repo_path::RepoPath; use jj_lib::settings::{ConfigResultExt as _, UserSettings}; use jj_lib::{conflicts, diff, files, rewrite}; use tracing::instrument; +use unicode_width::UnicodeWidthStr as _; use crate::cli_util::{CommandError, WorkspaceCommandHelper}; use crate::formatter::Formatter; use crate::merge_tools::{self, ExternalMergeTool}; +use crate::text_util; use crate::ui::Ui; #[derive(clap::Args, Clone, Debug)] @@ -779,13 +781,13 @@ pub fn show_diff_stat( tree_diff: TreeDiffIterator, ) -> Result<(), CommandError> { let mut stats: Vec = vec![]; - let mut max_path_length = 0; + let mut max_path_width = 0; let mut max_diffs = 0; for (repo_path, left, right) in tree_diff { let path = workspace_command.format_file_path(&repo_path); let left_content = diff_content(workspace_command.repo(), &repo_path, &left)?; let right_content = diff_content(workspace_command.repo(), &repo_path, &right)?; - max_path_length = max(max_path_length, path.chars().count()); + max_path_width = max(max_path_width, path.width()); let stat = get_diff_stat(path, &left_content, &right_content); max_diffs = max(max_diffs, stat.added + stat.removed); stats.push(stat); @@ -797,8 +799,8 @@ pub fn show_diff_stat( usize::from(ui.term_width().unwrap_or(80)).saturating_sub(4 + " | ".len() + number_padding); // Always give at least a tiny bit of room let available_width = max(available_width, 5); - let max_path_length = max_path_length.clamp(3, (0.7 * available_width as f64) as usize); - let max_bar_length = available_width.saturating_sub(max_path_length); + let max_path_width = max_path_width.clamp(3, (0.7 * available_width as f64) as usize); + let max_bar_length = available_width.saturating_sub(max_path_width); let factor = if max_diffs < max_bar_length { 1.0 } else { @@ -809,31 +811,18 @@ pub fn show_diff_stat( let mut total_added = 0; let mut total_removed = 0; let total_files = stats.len(); - for mut stat in stats { + 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 - if stat.path.chars().count() > max_path_length { - stat.path = if max_path_length <= 3 { - "...".to_string() - } else { - let start_index = stat - .path - .char_indices() - .rev() - .nth(max_path_length - 4) - .unwrap() - .0; - format!("...{}", &stat.path[start_index..]) - } - } - // pad to max_path_length + let (path, path_width) = text_util::elide_start(&stat.path, "...", max_path_width); + let path_pad_width = max_path_width - path_width; write!( formatter, - "{:number_padding$}{}", - stat.path, + "{path}{:path_pad_width$} | {:>number_padding$}{}", + "", // pad to max_path_width stat.added + stat.removed, if bar_added + bar_removed > 0 { " " } else { "" }, )?; diff --git a/cli/tests/test_diff_command.rs b/cli/tests/test_diff_command.rs index 86069c856..3284a9184 100644 --- a/cli/tests/test_diff_command.rs +++ b/cli/tests/test_diff_command.rs @@ -808,69 +808,69 @@ fn test_diff_stat_long_name_or_stat() { insta::assert_snapshot!(get_stat(&test_env, 1, 1), @r###" 1 | 1 + - 一 | 1 + + 一 | 1 + 2 files changed, 2 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 1, 10), @r###" 1 | 10 ++++++++++ - 一 | 10 ++++++++++ + 一 | 10 ++++++++++ 2 files changed, 20 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 1, 100), @r###" 1 | 100 +++++++++++++++++ - 一 | 100 +++++++++++++++++ + 一 | 100 +++++++++++++++++ 2 files changed, 200 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 10, 1), @r###" - 1234567890 | 1 + - 一二三四五六七八九十 | 1 + + 1234567890 | 1 + + ...五六七八九十 | 1 + 2 files changed, 2 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 10, 10), @r###" - 1234567890 | 10 ++++++++++ - 一二三四五六七八九十 | 10 ++++++++++ + 1234567890 | 10 +++++++ + ...六七八九十 | 10 +++++++ 2 files changed, 20 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 10, 100), @r###" - 1234567890 | 100 ++++++++++ - 一二三四五六七八九十 | 100 ++++++++++ + 1234567890 | 100 ++++++ + ...六七八九十 | 100 ++++++ 2 files changed, 200 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 50, 1), @r###" ...901234567890 | 1 + - ...九十一二三四五六七八九十 | 1 + + ...五六七八九十 | 1 + 2 files changed, 2 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 50, 10), @r###" ...01234567890 | 10 +++++++ - ...十一二三四五六七八九十 | 10 +++++++ + ...六七八九十 | 10 +++++++ 2 files changed, 20 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 50, 100), @r###" ...01234567890 | 100 ++++++ - ...十一二三四五六七八九十 | 100 ++++++ + ...六七八九十 | 100 ++++++ 2 files changed, 200 insertions(+), 0 deletions(-) "###); // Lengths around where we introduce the ellipsis insta::assert_snapshot!(get_stat(&test_env, 13, 100), @r###" - 1234567890123 | 100 ++++++++ - 一二三四五六七八九十一二三 | 100 ++++++++ + 1234567890123 | 100 ++++++ + ...九十一二三 | 100 ++++++ 2 files changed, 200 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 14, 100), @r###" 12345678901234 | 100 ++++++ - 一二三四五六七八九十一二三四 | 100 ++++++ + ...十一二三四 | 100 ++++++ 2 files changed, 200 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 15, 100), @r###" ...56789012345 | 100 ++++++ - ...五六七八九十一二三四五 | 100 ++++++ + ...一二三四五 | 100 ++++++ 2 files changed, 200 insertions(+), 0 deletions(-) "###); insta::assert_snapshot!(get_stat(&test_env, 16, 100), @r###" ...67890123456 | 100 ++++++ - ...六七八九十一二三四五六 | 100 ++++++ + ...二三四五六 | 100 ++++++ 2 files changed, 200 insertions(+), 0 deletions(-) "###); @@ -889,7 +889,12 @@ fn test_diff_stat_long_name_or_stat() { "###); insta::assert_snapshot!(get_stat(&test_env, 3, 10), @r###" 123 | 10 ++ - 一二三 | 10 ++ + ... | 10 ++ + 2 files changed, 20 insertions(+), 0 deletions(-) + "###); + insta::assert_snapshot!(get_stat(&test_env, 1, 10), @r###" + 1 | 10 ++ + 一 | 10 ++ 2 files changed, 20 insertions(+), 0 deletions(-) "###); }