Skip to content

Commit

Permalink
op diff: set up diff/template context without using WorkspaceCommandH…
Browse files Browse the repository at this point in the history
…elper

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.
  • Loading branch information
yuja committed Sep 9, 2024
1 parent 1e17078 commit d04ff12
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 33 deletions.
22 changes: 2 additions & 20 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,6 @@ impl CommandHelper {
)?)
}

pub fn operation_template_extensions(&self) -> &[Arc<dyn OperationTemplateLanguageExtension>] {
&self.data.operation_template_extensions
}

pub fn workspace_loader(&self) -> Result<&dyn WorkspaceLoader, CommandError> {
self.data
.maybe_workspace_loader
Expand Down Expand Up @@ -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<ReadonlyRepo>,
) -> Result<WorkspaceCommandHelper, CommandError> {
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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -726,7 +708,7 @@ impl WorkspaceCommandEnvironment {
}

pub fn operation_template_extensions(&self) -> &[Arc<dyn OperationTemplateLanguageExtension>] {
self.command.operation_template_extensions()
&self.command.data.operation_template_extensions
}
}

Expand Down
25 changes: 17 additions & 8 deletions cli/src/commands/operation/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
24 changes: 19 additions & 5 deletions cli/src/commands/operation/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -58,21 +62,31 @@ 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;
{
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
Expand Down

0 comments on commit d04ff12

Please sign in to comment.