diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index e14a0fead..38d12a755 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -86,7 +86,7 @@ use crate::commit_templater::{CommitTemplateLanguage, CommitTemplateLanguageExte use crate::config::{ new_config_path, AnnotatedValue, CommandNameAndArgs, ConfigSource, LayeredConfigs, }; -use crate::diff_util::DiffWorkspaceContext; +use crate::diff_util::{DiffFormat, DiffRenderer, DiffWorkspaceContext}; use crate::formatter::{FormatRecorder, Formatter, PlainTextFormatter}; use crate::git_util::{ is_colocated_git_workspace, print_failed_git_export, print_git_import_stats, @@ -808,12 +808,13 @@ impl WorkspaceCommandHelper { Ok(git_ignores) } - // TODO: make it private - pub(crate) fn diff_context(&self) -> DiffWorkspaceContext<'_> { - DiffWorkspaceContext { + /// Creates textual diff renderer of the specified `formats`. + pub fn diff_renderer(&self, formats: Vec) -> DiffRenderer<'_> { + let workspace_ctx = DiffWorkspaceContext { cwd: &self.cwd, workspace_root: self.workspace.workspace_root(), - } + }; + DiffRenderer::new(self.repo().as_ref(), workspace_ctx, formats) } /// Loads diff editor from the settings. diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index e17434473..cd2b1c55b 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -16,7 +16,7 @@ use tracing::instrument; use crate::cli_util::{print_unmatched_explicit_paths, CommandHelper, RevisionArg}; use crate::command_error::CommandError; -use crate::diff_util::{diff_formats_for, show_diff, DiffFormatArgs}; +use crate::diff_util::{diff_formats_for, DiffFormatArgs}; use crate::ui::Ui; /// Compare file contents between two revisions @@ -77,15 +77,14 @@ pub(crate) fn cmd_diff( let fileset_expression = workspace_command.parse_file_patterns(&args.paths)?; let matcher = fileset_expression.to_matcher(); let diff_formats = diff_formats_for(command.settings(), &args.format)?; + let diff_renderer = workspace_command.diff_renderer(diff_formats); ui.request_pager(); - show_diff( + diff_renderer.show_diff( ui, ui.stdout_formatter().as_mut(), - &workspace_command, &from_tree, &to_tree, matcher.as_ref(), - &diff_formats, )?; print_unmatched_explicit_paths( ui, diff --git a/cli/src/commands/interdiff.rs b/cli/src/commands/interdiff.rs index b08b9af99..97451eb0d 100644 --- a/cli/src/commands/interdiff.rs +++ b/cli/src/commands/interdiff.rs @@ -59,15 +59,14 @@ pub(crate) fn cmd_interdiff( .parse_file_patterns(&args.paths)? .to_matcher(); let diff_formats = diff_util::diff_formats_for(command.settings(), &args.format)?; + let diff_renderer = workspace_command.diff_renderer(diff_formats); ui.request_pager(); - diff_util::show_diff( + diff_renderer.show_diff( ui, ui.stdout_formatter().as_mut(), - &workspace_command, &from_tree, &to_tree, matcher.as_ref(), - &diff_formats, )?; Ok(()) } diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index 1b981fb89..29ea8a477 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -104,6 +104,8 @@ pub(crate) fn cmd_log( let store = repo.store(); let diff_formats = diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?; + let diff_renderer = + (!diff_formats.is_empty()).then(|| workspace_command.diff_renderer(diff_formats)); let use_elided_nodes = command .settings() @@ -188,16 +190,9 @@ pub(crate) fn cmd_log( if !buffer.ends_with(b"\n") { buffer.push(b'\n'); } - if !diff_formats.is_empty() { + if let Some(renderer) = &diff_renderer { let mut formatter = ui.new_formatter(&mut buffer); - diff_util::show_patch( - ui, - formatter.as_mut(), - &workspace_command, - &commit, - matcher.as_ref(), - &diff_formats, - )?; + renderer.show_patch(ui, formatter.as_mut(), &commit, matcher.as_ref())?; } let node_symbol = format_template(ui, &Some(commit), &node_template); @@ -236,15 +231,8 @@ pub(crate) fn cmd_log( let commit = commit_or_error?; with_content_format .write(formatter, |formatter| template.format(&commit, formatter))?; - if !diff_formats.is_empty() { - diff_util::show_patch( - ui, - formatter, - &workspace_command, - &commit, - matcher.as_ref(), - &diff_formats, - )?; + if let Some(renderer) = &diff_renderer { + renderer.show_patch(ui, formatter, &commit, matcher.as_ref())?; } } } diff --git a/cli/src/commands/obslog.rs b/cli/src/commands/obslog.rs index d677395b0..0334cac42 100644 --- a/cli/src/commands/obslog.rs +++ b/cli/src/commands/obslog.rs @@ -16,15 +16,14 @@ use itertools::Itertools; use jj_lib::commit::Commit; use jj_lib::dag_walk::topo_order_reverse_ok; use jj_lib::matchers::EverythingMatcher; +use jj_lib::repo::Repo; use jj_lib::rewrite::rebase_to_dest_parent; use tracing::instrument; -use crate::cli_util::{ - format_template, CommandHelper, LogContentFormat, RevisionArg, WorkspaceCommandHelper, -}; +use crate::cli_util::{format_template, CommandHelper, LogContentFormat, RevisionArg}; use crate::command_error::CommandError; use crate::commit_templater::CommitTemplateLanguage; -use crate::diff_util::{self, DiffFormat, DiffFormatArgs}; +use crate::diff_util::{self, DiffFormatArgs, DiffRenderer}; use crate::formatter::Formatter; use crate::graphlog::{get_graphlog, Edge}; use crate::ui::Ui; @@ -68,11 +67,14 @@ pub(crate) fn cmd_obslog( args: &ObslogArgs, ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; + let repo = workspace_command.repo().as_ref(); let start_commit = workspace_command.resolve_single_rev(&args.revision)?; let diff_formats = diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?; + let diff_renderer = + (!diff_formats.is_empty()).then(|| workspace_command.diff_renderer(diff_formats)); let with_content_format = LogContentFormat::new(ui, command.settings())?; let template; @@ -127,15 +129,9 @@ pub(crate) fn cmd_obslog( if !buffer.ends_with(b"\n") { buffer.push(b'\n'); } - if !diff_formats.is_empty() { + if let Some(renderer) = &diff_renderer { let mut formatter = ui.new_formatter(&mut buffer); - show_predecessor_patch( - ui, - formatter.as_mut(), - &workspace_command, - &commit, - &diff_formats, - )?; + show_predecessor_patch(ui, repo, renderer, formatter.as_mut(), &commit)?; } let node_symbol = format_template(ui, &Some(commit.clone()), &node_template); graph.add_node( @@ -149,8 +145,8 @@ pub(crate) fn cmd_obslog( for commit in commits { with_content_format .write(formatter, |formatter| template.format(&commit, formatter))?; - if !diff_formats.is_empty() { - show_predecessor_patch(ui, formatter, &workspace_command, &commit, &diff_formats)?; + if let Some(renderer) = &diff_renderer { + show_predecessor_patch(ui, repo, renderer, formatter, &commit)?; } } } @@ -160,27 +156,18 @@ pub(crate) fn cmd_obslog( fn show_predecessor_patch( ui: &Ui, + repo: &dyn Repo, + renderer: &DiffRenderer, formatter: &mut dyn Formatter, - workspace_command: &WorkspaceCommandHelper, commit: &Commit, - diff_formats: &[DiffFormat], ) -> Result<(), CommandError> { let mut predecessors = commit.predecessors(); let predecessor = match predecessors.next() { Some(predecessor) => predecessor?, None => return Ok(()), }; - let predecessor_tree = - rebase_to_dest_parent(workspace_command.repo().as_ref(), &predecessor, commit)?; + let predecessor_tree = rebase_to_dest_parent(repo, &predecessor, commit)?; let tree = commit.tree()?; - diff_util::show_diff( - ui, - formatter, - workspace_command, - &predecessor_tree, - &tree, - &EverythingMatcher, - diff_formats, - )?; + renderer.show_diff(ui, formatter, &predecessor_tree, &tree, &EverythingMatcher)?; Ok(()) } diff --git a/cli/src/commands/show.rs b/cli/src/commands/show.rs index 19e276a14..a3c81b271 100644 --- a/cli/src/commands/show.rs +++ b/cli/src/commands/show.rs @@ -52,17 +52,11 @@ pub(crate) fn cmd_show( }; let template = workspace_command.parse_commit_template(&template_string)?; let diff_formats = diff_util::diff_formats_for(command.settings(), &args.format)?; + let diff_renderer = workspace_command.diff_renderer(diff_formats); ui.request_pager(); let mut formatter = ui.stdout_formatter(); let formatter = formatter.as_mut(); template.format(&commit, formatter)?; - diff_util::show_patch( - ui, - formatter, - &workspace_command, - &commit, - &EverythingMatcher, - &diff_formats, - )?; + diff_renderer.show_patch(ui, formatter, &commit, &EverythingMatcher)?; Ok(()) } diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs index cf2dabdf9..788cd5b64 100644 --- a/cli/src/commands/status.rs +++ b/cli/src/commands/status.rs @@ -20,8 +20,8 @@ use tracing::instrument; use crate::cli_util::{print_conflicted_paths, CommandHelper}; use crate::command_error::CommandError; use crate::diff_util::DiffFormat; +use crate::revset_util; use crate::ui::Ui; -use crate::{diff_util, revset_util}; /// Show high-level repo status /// @@ -65,15 +65,8 @@ pub(crate) fn cmd_status( writeln!(formatter, "The working copy is clean")?; } else { writeln!(formatter, "Working copy changes:")?; - diff_util::show_diff( - ui, - formatter, - &workspace_command, - &parent_tree, - &tree, - &matcher, - &[DiffFormat::Summary], - )?; + let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]); + diff_renderer.show_diff(ui, formatter, &parent_tree, &tree, &matcher)?; } // TODO: Conflicts should also be filtered by the `matcher`. See the related diff --git a/cli/src/description_util.rs b/cli/src/description_util.rs index f3ce542db..e55f6785b 100644 --- a/cli/src/description_util.rs +++ b/cli/src/description_util.rs @@ -7,7 +7,7 @@ use jj_lib::settings::UserSettings; use crate::cli_util::{edit_temp_file, WorkspaceCommandHelper}; use crate::command_error::CommandError; -use crate::diff_util::{self, DiffFormat}; +use crate::diff_util::DiffFormat; use crate::formatter::PlainTextFormatter; use crate::text_util; use crate::ui::Ui; @@ -97,13 +97,12 @@ pub fn description_template_for_describe( commit: &Commit, ) -> Result { let mut diff_summary_bytes = Vec::new(); - diff_util::show_patch( + let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]); + diff_renderer.show_patch( ui, &mut PlainTextFormatter::new(&mut diff_summary_bytes), - workspace_command, commit, &EverythingMatcher, - &[DiffFormat::Summary], )?; let description = if commit.description().is_empty() { settings.default_description() @@ -127,14 +126,13 @@ pub fn description_template_for_commit( to_tree: &MergedTree, ) -> Result { let mut diff_summary_bytes = Vec::new(); - diff_util::show_diff( + let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]); + diff_renderer.show_diff( ui, &mut PlainTextFormatter::new(&mut diff_summary_bytes), - workspace_command, from_tree, to_tree, &EverythingMatcher, - &[DiffFormat::Summary], )?; let mut template_chunks = Vec::new(); if !intro.is_empty() { diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 2f2cbe256..6b88f6b6e 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -39,7 +39,6 @@ use thiserror::Error; use tracing::instrument; use unicode_width::UnicodeWidthStr as _; -use crate::cli_util::WorkspaceCommandHelper; use crate::config::CommandNameAndArgs; use crate::formatter::Formatter; use crate::merge_tools::{self, DiffGenerateError, ExternalMergeTool}; @@ -217,69 +216,89 @@ impl DiffWorkspaceContext<'_> { } } -pub fn show_diff( - ui: &Ui, - formatter: &mut dyn Formatter, - workspace_command: &WorkspaceCommandHelper, - from_tree: &MergedTree, - to_tree: &MergedTree, - matcher: &dyn Matcher, - formats: &[DiffFormat], -) -> Result<(), DiffRenderError> { - let repo = workspace_command.repo().as_ref(); - let workspace_ctx = workspace_command.diff_context(); - for format in formats { - match format { - DiffFormat::Summary => { - let tree_diff = from_tree.diff_stream(to_tree, matcher); - show_diff_summary(formatter, tree_diff, &workspace_ctx)?; - } - DiffFormat::Stat => { - let tree_diff = from_tree.diff_stream(to_tree, matcher); - // TODO: In graph log, graph width should be subtracted - let width = usize::from(ui.term_width().unwrap_or(80)); - show_diff_stat(repo, formatter, tree_diff, &workspace_ctx, width)?; - } - DiffFormat::Types => { - let tree_diff = from_tree.diff_stream(to_tree, matcher); - show_types(formatter, tree_diff, &workspace_ctx)?; - } - DiffFormat::Git { context } => { - let tree_diff = from_tree.diff_stream(to_tree, matcher); - show_git_diff(repo, formatter, *context, tree_diff)?; - } - DiffFormat::ColorWords { context } => { - let tree_diff = from_tree.diff_stream(to_tree, matcher); - show_color_words_diff(repo, formatter, *context, tree_diff, &workspace_ctx)?; - } - DiffFormat::Tool(tool) => { - merge_tools::generate_diff(ui, formatter.raw(), from_tree, to_tree, matcher, tool) - .map_err(DiffRenderError::DiffGenerate)?; - } - } - } - Ok(()) +/// Configuration and environment to render textual diff. +pub struct DiffRenderer<'a> { + repo: &'a dyn Repo, + workspace_ctx: DiffWorkspaceContext<'a>, + formats: Vec, } -pub fn show_patch( - ui: &Ui, - formatter: &mut dyn Formatter, - workspace_command: &WorkspaceCommandHelper, - commit: &Commit, - matcher: &dyn Matcher, - formats: &[DiffFormat], -) -> Result<(), DiffRenderError> { - let from_tree = commit.parent_tree(workspace_command.repo().as_ref())?; - let to_tree = commit.tree()?; - show_diff( - ui, - formatter, - workspace_command, - &from_tree, - &to_tree, - matcher, - formats, - ) +impl<'a> DiffRenderer<'a> { + pub fn new( + repo: &'a dyn Repo, + workspace_ctx: DiffWorkspaceContext<'a>, + formats: Vec, + ) -> Self { + DiffRenderer { + repo, + formats, + workspace_ctx, + } + } + + /// Generates diff between `from_tree` and `to_tree`. + pub fn show_diff( + &self, + ui: &Ui, // TODO: remove Ui dependency if possible + formatter: &mut dyn Formatter, + from_tree: &MergedTree, + to_tree: &MergedTree, + matcher: &dyn Matcher, + ) -> Result<(), DiffRenderError> { + let repo = self.repo; + let workspace_ctx = &self.workspace_ctx; + for format in &self.formats { + match format { + DiffFormat::Summary => { + let tree_diff = from_tree.diff_stream(to_tree, matcher); + show_diff_summary(formatter, tree_diff, workspace_ctx)?; + } + DiffFormat::Stat => { + let tree_diff = from_tree.diff_stream(to_tree, matcher); + // TODO: In graph log, graph width should be subtracted + let width = usize::from(ui.term_width().unwrap_or(80)); + show_diff_stat(repo, formatter, tree_diff, workspace_ctx, width)?; + } + DiffFormat::Types => { + let tree_diff = from_tree.diff_stream(to_tree, matcher); + show_types(formatter, tree_diff, workspace_ctx)?; + } + DiffFormat::Git { context } => { + let tree_diff = from_tree.diff_stream(to_tree, matcher); + show_git_diff(repo, formatter, *context, tree_diff)?; + } + DiffFormat::ColorWords { context } => { + let tree_diff = from_tree.diff_stream(to_tree, matcher); + show_color_words_diff(repo, formatter, *context, tree_diff, workspace_ctx)?; + } + DiffFormat::Tool(tool) => { + merge_tools::generate_diff( + ui, + formatter.raw(), + from_tree, + to_tree, + matcher, + tool, + ) + .map_err(DiffRenderError::DiffGenerate)?; + } + } + } + Ok(()) + } + + /// Generates diff of the given `commit` compared to its parents. + pub fn show_patch( + &self, + ui: &Ui, + formatter: &mut dyn Formatter, + commit: &Commit, + matcher: &dyn Matcher, + ) -> 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) + } } fn show_color_words_diff_hunks(