From 939528122df051b77bd2b0fa93018c982446e9f1 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Wed, 30 Aug 2023 15:48:41 -0700
Subject: [PATCH] cli: fix crash on diff stat with long path name

We would run into a panic due to "attempt to subtract with overflow"
if the path was long. This patch fixes that and adds tests showing the
current behavior when there are long paths and/or large diffs.
---
 cli/src/diff_util.rs           |  4 +--
 cli/tests/test_diff_command.rs | 54 ++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs
index 686a9bc62..a7ee35f3e 100644
--- a/cli/src/diff_util.rs
+++ b/cli/src/diff_util.rs
@@ -792,8 +792,8 @@ pub fn show_diff_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 max_bar_length = display_width
+        .saturating_sub(max_path_length + " | ".len() + max_diffs.to_string().len() + 1);
     let factor = if max_diffs < max_bar_length {
         1.0
     } else {
diff --git a/cli/tests/test_diff_command.rs b/cli/tests/test_diff_command.rs
index fadef3e28..e3b6feadb 100644
--- a/cli/tests/test_diff_command.rs
+++ b/cli/tests/test_diff_command.rs
@@ -783,3 +783,57 @@ fn test_diff_stat() {
     1 file changed, 0 insertions(+), 1 deletion(-)
     "###);
 }
+
+#[test]
+fn test_diff_stat_long_name_or_stat() {
+    let mut test_env = TestEnvironment::default();
+    test_env.add_env_var("COLUMNS", "50");
+    test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    let get_stat = |path_length: usize, stat_size: usize| {
+        test_env.jj_cmd_success(&repo_path, &["new", "root"]);
+        let mut name = "abcdefghij".repeat(path_length / 10 + 1);
+        name.truncate(path_length);
+        let content = "content line\n".repeat(stat_size);
+        std::fs::write(repo_path.join(name), content).unwrap();
+        test_env.jj_cmd_success(&repo_path, &["diff", "--stat"])
+    };
+
+    insta::assert_snapshot!(get_stat(1, 1), @r###"
+    a | 1 +
+    1 file changed, 1 insertion(+), 0 deletions(-)
+    "###);
+    insta::assert_snapshot!(get_stat(1, 10), @r###"
+    a | 10 ++++++++++
+    1 file changed, 10 insertions(+), 0 deletions(-)
+    "###);
+    insta::assert_snapshot!(get_stat(1, 100), @r###"
+    a | 100 ++++++++++++++++++++++++++++++++++++++
+    1 file changed, 100 insertions(+), 0 deletions(-)
+    "###);
+    insta::assert_snapshot!(get_stat(10, 1), @r###"
+    abcdefghij | 1 +
+    1 file changed, 1 insertion(+), 0 deletions(-)
+    "###);
+    insta::assert_snapshot!(get_stat(10, 10), @r###"
+    abcdefghij | 10 ++++++++++
+    1 file changed, 10 insertions(+), 0 deletions(-)
+    "###);
+    insta::assert_snapshot!(get_stat(10, 100), @r###"
+    abcdefghij | 100 +++++++++++++++++++++++++++++
+    1 file changed, 100 insertions(+), 0 deletions(-)
+    "###);
+    insta::assert_snapshot!(get_stat(100, 1), @r###"
+    abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij | 1
+    1 file changed, 1 insertion(+), 0 deletions(-)
+    "###);
+    insta::assert_snapshot!(get_stat(100, 10), @r###"
+    abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij | 10
+    1 file changed, 10 insertions(+), 0 deletions(-)
+    "###);
+    insta::assert_snapshot!(get_stat(100, 100), @r###"
+    abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij | 100
+    1 file changed, 100 insertions(+), 0 deletions(-)
+    "###);
+}