From d04ff1213f6594bc04c225432234f6f4fd56088d Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 7 Sep 2024 17:37:35 +0900 Subject: [PATCH] op diff: set up diff/template context without using WorkspaceCommandHelper I'll use a similar setup in "op log", but for each log entry. We might want to extract some parts to helper function, but I don't have a good idea right now. CommandHelper::operation_template_extensions() is removed because it's unlikely to parse operation template without loading a workspace. --- cli/src/cli_util.rs | 22 ++-------------------- cli/src/commands/operation/diff.rs | 25 +++++++++++++++++-------- cli/src/commands/operation/show.rs | 24 +++++++++++++++++++----- 3 files changed, 38 insertions(+), 33 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index cf8d1f4750..eabb519154 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -346,10 +346,6 @@ impl CommandHelper { )?) } - pub fn operation_template_extensions(&self) -> &[Arc] { - &self.data.operation_template_extensions - } - pub fn workspace_loader(&self) -> Result<&dyn WorkspaceLoader, CommandError> { self.data .maybe_workspace_loader @@ -494,20 +490,6 @@ impl CommandHelper { let loaded_at_head = true; WorkspaceCommandHelper::new(workspace, repo, env, loaded_at_head) } - - /// Creates helper for the repo whose view might be out of sync with - /// the working copy. Therefore, the working copy should not be updated. - #[instrument(skip_all)] - pub fn for_temporary_repo( - &self, - ui: &Ui, - workspace: Workspace, - repo: Arc, - ) -> Result { - let env = self.workspace_environment(ui, &workspace)?; - let loaded_at_head = false; - WorkspaceCommandHelper::new(workspace, repo, env, loaded_at_head) - } } /// A ReadonlyRepo along with user-config-dependent derived data. The derived @@ -668,7 +650,7 @@ impl WorkspaceCommandEnvironment { /// Creates fresh new context which manages cache of short commit/change ID /// prefixes. New context should be created per repo view (or operation.) - fn new_id_prefix_context(&self) -> IdPrefixContext { + pub fn new_id_prefix_context(&self) -> IdPrefixContext { let context = IdPrefixContext::new(self.command.revset_extensions().clone()); match &self.short_prefixes_expression { None => context, @@ -726,7 +708,7 @@ impl WorkspaceCommandEnvironment { } pub fn operation_template_extensions(&self) -> &[Arc] { - self.command.operation_template_extensions() + &self.command.data.operation_template_extensions } } diff --git a/cli/src/commands/operation/diff.rs b/cli/src/commands/operation/diff.rs index 44c9eb91b0..1c309e7f66 100644 --- a/cli/src/commands/operation/diff.rs +++ b/cli/src/commands/operation/diff.rs @@ -40,6 +40,7 @@ use crate::cli_util::short_operation_hash; use crate::cli_util::CommandHelper; use crate::cli_util::LogContentFormat; use crate::command_error::CommandError; +use crate::commit_templater::CommitTemplateLanguage; use crate::diff_util::diff_formats_for_log; use crate::diff_util::DiffFormatArgs; use crate::diff_util::DiffRenderer; @@ -82,6 +83,7 @@ pub fn cmd_op_diff( args: &OperationDiffArgs, ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; + let workspace_env = workspace_command.env(); let repo = workspace_command.repo(); let repo_loader = &repo.loader(); let from_op; @@ -101,19 +103,26 @@ pub fn cmd_op_diff( let to_repo = repo_loader.load_at(&to_op)?; // Create a new transaction starting from `to_repo`. - let mut workspace_command = - command.for_temporary_repo(ui, command.load_workspace()?, to_repo.clone())?; - let mut tx = workspace_command.start_transaction(); + let mut tx = to_repo.start_transaction(command.settings()); // Merge index from `from_repo` to `to_repo`, so commits in `from_repo` are // accessible. tx.repo_mut().merge_index(&from_repo); + let merged_repo = tx.repo(); + let diff_renderer = { - // diff_renderer_for_log(), but captures the merged MutableRepo. let formats = diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?; - let path_converter = tx.base_workspace_helper().path_converter(); - (!formats.is_empty()).then(|| DiffRenderer::new(tx.repo(), path_converter, formats)) + let path_converter = workspace_env.path_converter(); + (!formats.is_empty()).then(|| DiffRenderer::new(merged_repo, path_converter, formats)) + }; + let id_prefix_context = workspace_env.new_id_prefix_context(); + let commit_summary_template = { + let language = workspace_env.commit_template_language(merged_repo, &id_prefix_context); + let text = command + .settings() + .config() + .get_string("templates.commit_summary")?; + workspace_env.parse_template(&language, &text, CommitTemplateLanguage::wrap_commit)? }; - let commit_summary_template = tx.commit_summary_template(); ui.request_pager(); let mut formatter = ui.stdout_formatter(); @@ -157,7 +166,7 @@ pub fn cmd_op_diff( show_op_diff( ui, formatter.as_mut(), - tx.repo(), + merged_repo, &from_repo, &to_repo, &commit_summary_template, diff --git a/cli/src/commands/operation/show.rs b/cli/src/commands/operation/show.rs index 016ef00f27..e35eb3bac2 100644 --- a/cli/src/commands/operation/show.rs +++ b/cli/src/commands/operation/show.rs @@ -18,7 +18,10 @@ use super::diff::show_op_diff; use crate::cli_util::CommandHelper; use crate::cli_util::LogContentFormat; use crate::command_error::CommandError; +use crate::commit_templater::CommitTemplateLanguage; +use crate::diff_util::diff_formats_for_log; use crate::diff_util::DiffFormatArgs; +use crate::diff_util::DiffRenderer; use crate::graphlog::GraphStyle; use crate::operation_templater::OperationTemplateLanguage; use crate::ui::Ui; @@ -49,6 +52,7 @@ pub fn cmd_op_show( args: &OperationShowArgs, ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; + let workspace_env = workspace_command.env(); let repo = workspace_command.repo(); let current_op_id = repo.operation().id(); let repo_loader = &repo.loader(); @@ -58,13 +62,23 @@ pub fn cmd_op_show( let parent_repo = repo_loader.load_at(&parent_op)?; let repo = repo_loader.load_at(&op)?; - let workspace_command = - command.for_temporary_repo(ui, command.load_workspace()?, repo.clone())?; - let commit_summary_template = workspace_command.commit_summary_template(); + let id_prefix_context = workspace_env.new_id_prefix_context(); + let commit_summary_template = { + let language = workspace_env.commit_template_language(repo.as_ref(), &id_prefix_context); + let text = command + .settings() + .config() + .get_string("templates.commit_summary")?; + workspace_env.parse_template(&language, &text, CommitTemplateLanguage::wrap_commit)? + }; let graph_style = GraphStyle::from_settings(command.settings())?; let with_content_format = LogContentFormat::new(ui, command.settings())?; - let diff_renderer = workspace_command.diff_renderer_for_log(&args.diff_format, args.patch)?; + let diff_renderer = { + let formats = diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?; + let path_converter = workspace_env.path_converter(); + (!formats.is_empty()).then(|| DiffRenderer::new(repo.as_ref(), path_converter, formats)) + }; // TODO: Should we make this customizable via clap arg? let template; @@ -72,7 +86,7 @@ pub fn cmd_op_show( let language = OperationTemplateLanguage::new( repo_loader.op_store().root_operation_id(), Some(current_op_id), - command.operation_template_extensions(), + workspace_env.operation_template_extensions(), ); let text = command.settings().config().get_string("templates.op_log")?; template = workspace_command