diff: specify available terminal width by caller, subtract graph width

The width parameter is mandatory so it wouldn't fall back to ui.term_width() by
mistake. The API is getting messy and we might want to extract some parameters
to separate struct.

Fixes #4158

cli/src/commands/diff.rs

@@ -84,6 +84,7 @@ pub(crate) fn cmd_diff(
         &from_tree,
         &to_tree,
         matcher.as_ref(),
+        ui.term_width(),
     )?;
     print_unmatched_explicit_paths(
         ui,

cli/src/commands/interdiff.rs

@@ -66,6 +66,7 @@ pub(crate) fn cmd_interdiff(
         &from_tree,
         &to_tree,
         matcher.as_ref(),
+        ui.term_width(),
     )?;
     Ok(())
 }

cli/src/commands/log.rs

@@ -193,17 +193,25 @@ pub(crate) fn cmd_log(
                 let mut buffer = vec![];
                 let key = (commit_id, false);
                 let commit = store.get_commit(&key.0)?;
+                let graph_width = || graph.width(&key, &graphlog_edges);
                 with_content_format.write_graph_text(
                     ui.new_formatter(&mut buffer).as_mut(),
                     |formatter| template.format(&commit, formatter),
-                    || graph.width(&key, &graphlog_edges),
+                    graph_width,
                 )?;
                 if !buffer.ends_with(b"\n") {
                     buffer.push(b'\n');
                 }
                 if let Some(renderer) = &diff_renderer {
                     let mut formatter = ui.new_formatter(&mut buffer);
-                    renderer.show_patch(ui, formatter.as_mut(), &commit, matcher.as_ref())?;
+                    let width = usize::saturating_sub(ui.term_width(), graph_width());
+                    renderer.show_patch(
+                        ui,
+                        formatter.as_mut(),
+                        &commit,
+                        matcher.as_ref(),
+                        width,
+                    )?;
                 }
 
                 let node_symbol = format_template(ui, &Some(commit), &node_template);
@@ -243,7 +251,8 @@ pub(crate) fn cmd_log(
                 with_content_format
                     .write(formatter, |formatter| template.format(&commit, formatter))?;
                 if let Some(renderer) = &diff_renderer {
-                    renderer.show_patch(ui, formatter, &commit, matcher.as_ref())?;
+                    let width = ui.term_width();
+                    renderer.show_patch(ui, formatter, &commit, matcher.as_ref(), width)?;
                 }
             }
         }

cli/src/commands/obslog.rs

@@ -145,18 +145,20 @@ pub(crate) fn cmd_obslog(
             for predecessor in commit.predecessors() {
                 edges.push(Edge::Direct(predecessor?.id().clone()));
             }
+            let graph_width = || graph.width(commit.id(), &edges);
             let mut buffer = vec![];
             with_content_format.write_graph_text(
                 ui.new_formatter(&mut buffer).as_mut(),
                 |formatter| template.format(&commit, formatter),
-                || graph.width(commit.id(), &edges),
+                graph_width,
             )?;
             if !buffer.ends_with(b"\n") {
                 buffer.push(b'\n');
             }
             if let Some(renderer) = &diff_renderer {
                 let mut formatter = ui.new_formatter(&mut buffer);
-                show_predecessor_patch(ui, repo, renderer, formatter.as_mut(), &commit)?;
+                let width = usize::saturating_sub(ui.term_width(), graph_width());
+                show_predecessor_patch(ui, repo, renderer, formatter.as_mut(), &commit, width)?;
             }
             let node_symbol = format_template(ui, &Some(commit.clone()), &node_template);
             graph.add_node(
@@ -171,7 +173,8 @@ pub(crate) fn cmd_obslog(
             with_content_format
                 .write(formatter, |formatter| template.format(&commit, formatter))?;
             if let Some(renderer) = &diff_renderer {
-                show_predecessor_patch(ui, repo, renderer, formatter, &commit)?;
+                let width = ui.term_width();
+                show_predecessor_patch(ui, repo, renderer, formatter, &commit, width)?;
             }
         }
     }
@@ -185,6 +188,7 @@ fn show_predecessor_patch(
     renderer: &DiffRenderer,
     formatter: &mut dyn Formatter,
     commit: &Commit,
+    width: usize,
 ) -> Result<(), CommandError> {
     let mut predecessors = commit.predecessors();
     let predecessor = match predecessors.next() {
@@ -193,6 +197,13 @@ fn show_predecessor_patch(
     };
     let predecessor_tree = rebase_to_dest_parent(repo, &predecessor, commit)?;
     let tree = commit.tree()?;
-    renderer.show_diff(ui, formatter, &predecessor_tree, &tree, &EverythingMatcher)?;
+    renderer.show_diff(
+        ui,
+        formatter,
+        &predecessor_tree,
+        &tree,
+        &EverythingMatcher,
+        width,
+    )?;
     Ok(())
 }

cli/src/commands/operation/diff.rs

@@ -224,6 +224,7 @@ pub fn show_op_diff(
                     .iter()
                     .map(|edge| Edge::Direct(edge.target.clone()))
                     .collect_vec();
+                let graph_width = || graph.width(&change_id, &edges);
 
                 let mut buffer = vec![];
                 with_content_format.write_graph_text(
@@ -236,19 +237,21 @@ pub fn show_op_diff(
                             modified_change,
                         )
                     },
-                    || graph.width(&change_id, &edges),
+                    graph_width,
                 )?;
                 if !buffer.ends_with(b"\n") {
                     buffer.push(b'\n');
                 }
                 if let Some(diff_renderer) = &diff_renderer {
                     let mut formatter = ui.new_formatter(&mut buffer);
+                    let width = usize::saturating_sub(ui.term_width(), graph_width());
                     show_change_diff(
                         ui,
                         formatter.as_mut(),
                         current_repo,
                         diff_renderer,
                         modified_change,
+                        width,
                     )?;
                 }
 
@@ -271,7 +274,15 @@ pub fn show_op_diff(
                     modified_change,
                 )?;
                 if let Some(diff_renderer) = &diff_renderer {
-                    show_change_diff(ui, formatter, current_repo, diff_renderer, modified_change)?;
+                    let width = ui.term_width();
+                    show_change_diff(
+                        ui,
+                        formatter,
+                        current_repo,
+                        diff_renderer,
+                        modified_change,
+                        width,
+                    )?;
                 }
             }
         }
@@ -543,20 +554,28 @@ fn show_change_diff(
     repo: &dyn Repo,
     diff_renderer: &DiffRenderer,
     modified_change: &ModifiedChange,
+    width: usize,
 ) -> Result<(), CommandError> {
     if modified_change.added_commits.len() == 1 && modified_change.removed_commits.len() == 1 {
         let commit = &modified_change.added_commits[0];
         let predecessor = &modified_change.removed_commits[0];
         let predecessor_tree = rebase_to_dest_parent(repo, predecessor, commit)?;
         let tree = commit.tree()?;
-        diff_renderer.show_diff(ui, formatter, &predecessor_tree, &tree, &EverythingMatcher)?;
+        diff_renderer.show_diff(
+            ui,
+            formatter,
+            &predecessor_tree,
+            &tree,
+            &EverythingMatcher,
+            width,
+        )?;
     } else if modified_change.added_commits.len() == 1 {
         let commit = &modified_change.added_commits[0];
-        diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher)?;
+        diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher, width)?;
     } else if modified_change.removed_commits.len() == 1 {
         // TODO: Should we show a reverse diff?
         let commit = &modified_change.removed_commits[0];
-        diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher)?;
+        diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher, width)?;
     }
 
     Ok(())

cli/src/commands/show.rs

@@ -56,6 +56,6 @@ pub(crate) fn cmd_show(
     let mut formatter = ui.stdout_formatter();
     let formatter = formatter.as_mut();
     template.format(&commit, formatter)?;
-    diff_renderer.show_patch(ui, formatter, &commit, &EverythingMatcher)?;
+    diff_renderer.show_patch(ui, formatter, &commit, &EverythingMatcher, ui.term_width())?;
     Ok(())
 }

cli/src/commands/status.rs

@@ -66,7 +66,8 @@ pub(crate) fn cmd_status(
         } else {
             writeln!(formatter, "Working copy changes:")?;
             let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]);
-            diff_renderer.show_diff(ui, formatter, &parent_tree, &tree, &matcher)?;
+            let width = ui.term_width();
+            diff_renderer.show_diff(ui, formatter, &parent_tree, &tree, &matcher, width)?;
         }
 
         // TODO: Conflicts should also be filtered by the `matcher`. See the related

cli/src/diff_util.rs

@@ -245,6 +245,7 @@ impl<'a> DiffRenderer<'a> {
         from_tree: &MergedTree,
         to_tree: &MergedTree,
         matcher: &dyn Matcher,
+        width: usize,
     ) -> Result<(), DiffRenderError> {
         let store = self.repo.store();
         let path_converter = self.path_converter;
@@ -256,8 +257,6 @@ impl<'a> DiffRenderer<'a> {
                 }
                 DiffFormat::Stat => {
                     let tree_diff = from_tree.diff_stream(to_tree, matcher);
-                    // TODO: In graph log, graph width should be subtracted
-                    let width = ui.term_width();
                     show_diff_stat(formatter, store, tree_diff, path_converter, width)?;
                 }
                 DiffFormat::Types => {
@@ -307,10 +306,11 @@ impl<'a> DiffRenderer<'a> {
         formatter: &mut dyn Formatter,
         commit: &Commit,
         matcher: &dyn Matcher,
+        width: usize,
     ) -> Result<(), DiffRenderError> {
         let from_tree = commit.parent_tree(self.repo)?;
         let to_tree = commit.tree()?;
-        self.show_diff(ui, formatter, &from_tree, &to_tree, matcher)
+        self.show_diff(ui, formatter, &from_tree, &to_tree, matcher, width)
     }
 }
 

cli/tests/test_log_command.rs

@@ -1377,6 +1377,53 @@ fn test_log_word_wrap() {
     "###);
 }
 
+#[test]
+fn test_log_diff_stat_width() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+    let render = |args: &[&str], columns: u32| {
+        let assert = test_env
+            .jj_cmd(&repo_path, args)
+            .env("COLUMNS", columns.to_string())
+            .assert()
+            .success()
+            .stderr("");
+        get_stdout_string(&assert)
+    };
+
+    std::fs::write(repo_path.join("file1"), "foo\n".repeat(100)).unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["new", "root()"]);
+    std::fs::write(repo_path.join("file2"), "foo\n".repeat(100)).unwrap();
+
+    insta::assert_snapshot!(render(&["log", "--stat", "--no-graph"], 30), @r###"
+    rlvkpnrz [email protected] 2001-02-03 08:05:09 287520bf
+    (no description set)
+    file2 | 100 +++++++++++++++
+    1 file changed, 100 insertions(+), 0 deletions(-)
+    qpvuntsm [email protected] 2001-02-03 08:05:08 e292def1
+    (no description set)
+    file1 | 100 +++++++++++++++
+    1 file changed, 100 insertions(+), 0 deletions(-)
+    zzzzzzzz root() 00000000
+    0 files changed, 0 insertions(+), 0 deletions(-)
+    "###);
+
+    // Graph width should be subtracted
+    insta::assert_snapshot!(render(&["log", "--stat"], 30), @r###"
+    @  rlvkpnrz [email protected] 2001-02-03 08:05:09 287520bf
+    │  (no description set)
+    │  file2 | 100 ++++++++++++
+    │  1 file changed, 100 insertions(+), 0 deletions(-)
+    │ ○  qpvuntsm [email protected] 2001-02-03 08:05:08 e292def1
+    ├─╯  (no description set)
+    │    file1 | 100 ++++++++++
+    │    1 file changed, 100 insertions(+), 0 deletions(-)
+    ◆  zzzzzzzz root() 00000000
+       0 files changed, 0 insertions(+), 0 deletions(-)
+    "###);
+}
+
 #[test]
 fn test_elided() {
     // Test that elided commits are shown as synthetic nodes.