ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

diff_util: remove WorkspaceCommandHelper dependency from show functions

Browse source 
I've added a wrapper struct in order to get around too many arguments warning.
It captures &dyn Repo as CommitTemplateLanguage would do. OTOH, &Ui is passed
by argument because the caller might need &mut Ui after the renderer object was
configured.
This commit is contained in:
Yuya Nishihara 2024-05-12 10:31:35 +09:00
parent 7341bff2f4
commit 4d9c84a17e
9 changed files with 122 additions and 144 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
cli/src/cli_util.rs
View file

@ -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<DiffFormat>) -> 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.

7
cli/src/commands/diff.rs
View file

@ -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,

5
cli/src/commands/interdiff.rs
View file

@ -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(())
}

24
cli/src/commands/log.rs
View file

@ -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())?;
}
}
}

41
cli/src/commands/obslog.rs
View file

@ -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(())
}

10
cli/src/commands/show.rs
View file

@ -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(())
}

13
cli/src/commands/status.rs
View file

@ -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

12
cli/src/description_util.rs
View file

@ -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<String, CommandError> {
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<String, CommandError> {
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() {

143
cli/src/diff_util.rs
View file

@ -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<DiffFormat>,
}
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<DiffFormat>,
) -> 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(