From bbf88643cccacebbc099a786929bcdd0999fea17 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 23 Feb 2024 12:39:14 +0900 Subject: [PATCH] templater: inline wrap_fn() helpers If I remember correctly, wrap_fn() was added to help type inference. It no longer makes sense because the type is coerced by TemplateFunction::new() and language.wrap_*(). --- cli/src/commit_templater.rs | 64 ++++++++++++++++------------------ cli/src/operation_templater.rs | 41 ++++++++-------------- 2 files changed, 46 insertions(+), 59 deletions(-) diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index b7e8e6af2..6c7b430c0 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -243,68 +243,62 @@ fn build_commit_method<'repo>( self_property: impl TemplateProperty + 'repo, function: &FunctionCallNode, ) -> TemplateParseResult> { - fn wrap_fn<'repo, O>( - property: impl TemplateProperty + 'repo, - f: impl Fn(&Commit) -> O + 'repo, - ) -> impl TemplateProperty + 'repo { - TemplateFunction::new(property, move |commit| f(&commit)) - } - fn wrap_repo_fn<'repo, O>( - repo: &'repo dyn Repo, - property: impl TemplateProperty + 'repo, - f: impl Fn(&dyn Repo, &Commit) -> O + 'repo, - ) -> impl TemplateProperty + 'repo { - TemplateFunction::new(property, move |commit| f(repo, &commit)) - } - let repo = language.repo; let cache = &language.keyword_cache; let property = match function.name { "description" => { template_parser::expect_no_arguments(function)?; - language.wrap_string(wrap_fn(self_property, |commit| { + language.wrap_string(TemplateFunction::new(self_property, |commit| { text_util::complete_newline(commit.description()) })) } "change_id" => { template_parser::expect_no_arguments(function)?; - language.wrap_commit_or_change_id(wrap_fn(self_property, |commit| { + language.wrap_commit_or_change_id(TemplateFunction::new(self_property, |commit| { CommitOrChangeId::Change(commit.change_id().to_owned()) })) } "commit_id" => { template_parser::expect_no_arguments(function)?; - language.wrap_commit_or_change_id(wrap_fn(self_property, |commit| { + language.wrap_commit_or_change_id(TemplateFunction::new(self_property, |commit| { CommitOrChangeId::Commit(commit.id().to_owned()) })) } "parents" => { template_parser::expect_no_arguments(function)?; - language.wrap_commit_list(wrap_fn(self_property, |commit| commit.parents())) + language.wrap_commit_list(TemplateFunction::new(self_property, |commit| { + commit.parents() + })) } "author" => { template_parser::expect_no_arguments(function)?; - language.wrap_signature(wrap_fn(self_property, |commit| commit.author().clone())) + language.wrap_signature(TemplateFunction::new(self_property, |commit| { + commit.author().clone() + })) } "committer" => { template_parser::expect_no_arguments(function)?; - language.wrap_signature(wrap_fn(self_property, |commit| commit.committer().clone())) + language.wrap_signature(TemplateFunction::new(self_property, |commit| { + commit.committer().clone() + })) } "working_copies" => { template_parser::expect_no_arguments(function)?; - language.wrap_string(wrap_repo_fn(repo, self_property, extract_working_copies)) + language.wrap_string(TemplateFunction::new(self_property, |commit| { + extract_working_copies(repo, &commit) + })) } "current_working_copy" => { template_parser::expect_no_arguments(function)?; let workspace_id = language.workspace_id.clone(); - language.wrap_boolean(wrap_fn(self_property, move |commit| { + language.wrap_boolean(TemplateFunction::new(self_property, move |commit| { Some(commit.id()) == repo.view().get_wc_commit_id(&workspace_id) })) } "branches" => { template_parser::expect_no_arguments(function)?; let index = cache.branches_index(repo).clone(); - language.wrap_ref_name_list(wrap_fn(self_property, move |commit| { + language.wrap_ref_name_list(TemplateFunction::new(self_property, move |commit| { index .get(commit.id()) .iter() @@ -316,7 +310,7 @@ fn build_commit_method<'repo>( "local_branches" => { template_parser::expect_no_arguments(function)?; let index = cache.branches_index(repo).clone(); - language.wrap_ref_name_list(wrap_fn(self_property, move |commit| { + language.wrap_ref_name_list(TemplateFunction::new(self_property, move |commit| { index .get(commit.id()) .iter() @@ -328,7 +322,7 @@ fn build_commit_method<'repo>( "remote_branches" => { template_parser::expect_no_arguments(function)?; let index = cache.branches_index(repo).clone(); - language.wrap_ref_name_list(wrap_fn(self_property, move |commit| { + language.wrap_ref_name_list(TemplateFunction::new(self_property, move |commit| { index .get(commit.id()) .iter() @@ -340,24 +334,26 @@ fn build_commit_method<'repo>( "tags" => { template_parser::expect_no_arguments(function)?; let index = cache.tags_index(repo).clone(); - language.wrap_ref_name_list(wrap_fn(self_property, move |commit| { + language.wrap_ref_name_list(TemplateFunction::new(self_property, move |commit| { index.get(commit.id()).to_vec() })) } "git_refs" => { template_parser::expect_no_arguments(function)?; let index = cache.git_refs_index(repo).clone(); - language.wrap_ref_name_list(wrap_fn(self_property, move |commit| { + language.wrap_ref_name_list(TemplateFunction::new(self_property, move |commit| { index.get(commit.id()).to_vec() })) } "git_head" => { template_parser::expect_no_arguments(function)?; - language.wrap_ref_name_list(wrap_repo_fn(repo, self_property, extract_git_head)) + language.wrap_ref_name_list(TemplateFunction::new(self_property, |commit| { + extract_git_head(repo, &commit) + })) } "divergent" => { template_parser::expect_no_arguments(function)?; - language.wrap_boolean(wrap_fn(self_property, |commit| { + language.wrap_boolean(TemplateFunction::new(self_property, |commit| { // The given commit could be hidden in e.g. obslog. let maybe_entries = repo.resolve_change_id(commit.change_id()); maybe_entries.map_or(0, |entries| entries.len()) > 1 @@ -365,18 +361,20 @@ fn build_commit_method<'repo>( } "hidden" => { template_parser::expect_no_arguments(function)?; - language.wrap_boolean(wrap_fn(self_property, |commit| { + language.wrap_boolean(TemplateFunction::new(self_property, |commit| { let maybe_entries = repo.resolve_change_id(commit.change_id()); maybe_entries.map_or(true, |entries| !entries.contains(commit.id())) })) } "conflict" => { template_parser::expect_no_arguments(function)?; - language.wrap_boolean(wrap_fn(self_property, |commit| commit.has_conflict().unwrap())) + language.wrap_boolean(TemplateFunction::new(self_property, |commit| { + commit.has_conflict().unwrap() + })) } "empty" => { template_parser::expect_no_arguments(function)?; - language.wrap_boolean(wrap_fn(self_property, |commit| { + language.wrap_boolean(TemplateFunction::new(self_property, |commit| { if let [parent] = &commit.parents()[..] { return parent.tree_id() == commit.tree_id(); } @@ -386,7 +384,7 @@ fn build_commit_method<'repo>( } "root" => { template_parser::expect_no_arguments(function)?; - language.wrap_boolean(wrap_fn(self_property, move |commit| { + language.wrap_boolean(TemplateFunction::new(self_property, |commit| { commit.id() == repo.store().root_commit_id() })) } diff --git a/cli/src/operation_templater.rs b/cli/src/operation_templater.rs index 622e533cc..82c1b10d3 100644 --- a/cli/src/operation_templater.rs +++ b/cli/src/operation_templater.rs @@ -16,7 +16,7 @@ use std::io; use itertools::Itertools as _; use jj_lib::object_id::ObjectId; -use jj_lib::op_store::{OperationId, OperationMetadata}; +use jj_lib::op_store::OperationId; use jj_lib::operation::Operation; use crate::formatter::Formatter; @@ -130,42 +130,29 @@ fn build_operation_method( self_property: impl TemplateProperty + 'static, function: &FunctionCallNode, ) -> TemplateParseResult { - fn wrap_fn( - property: impl TemplateProperty, - f: impl Fn(&Operation) -> O, - ) -> impl TemplateProperty { - TemplateFunction::new(property, move |op| f(&op)) - } - fn wrap_metadata_fn( - property: impl TemplateProperty, - f: impl Fn(&OperationMetadata) -> O, - ) -> impl TemplateProperty { - TemplateFunction::new(property, move |op| f(op.metadata())) - } - let property = match function.name { "current_operation" => { template_parser::expect_no_arguments(function)?; let current_op_id = language.current_op_id.cloned(); - language.wrap_boolean(wrap_fn(self_property, move |op| { + language.wrap_boolean(TemplateFunction::new(self_property, move |op| { Some(op.id()) == current_op_id.as_ref() })) } "description" => { template_parser::expect_no_arguments(function)?; - language.wrap_string(wrap_metadata_fn(self_property, |metadata| { - metadata.description.clone() + language.wrap_string(TemplateFunction::new(self_property, |op| { + op.metadata().description.clone() })) } "id" => { template_parser::expect_no_arguments(function)?; - language.wrap_operation_id(wrap_fn(self_property, |op| op.id().clone())) + language.wrap_operation_id(TemplateFunction::new(self_property, |op| op.id().clone())) } "tags" => { template_parser::expect_no_arguments(function)?; - language.wrap_string(wrap_metadata_fn(self_property, |metadata| { + language.wrap_string(TemplateFunction::new(self_property, |op| { // TODO: introduce map type - metadata + op.metadata() .tags .iter() .map(|(key, value)| format!("{key}: {value}")) @@ -174,24 +161,26 @@ fn build_operation_method( } "time" => { template_parser::expect_no_arguments(function)?; - language.wrap_timestamp_range(wrap_metadata_fn(self_property, |metadata| { + language.wrap_timestamp_range(TemplateFunction::new(self_property, |op| { TimestampRange { - start: metadata.start_time.clone(), - end: metadata.end_time.clone(), + start: op.metadata().start_time.clone(), + end: op.metadata().end_time.clone(), } })) } "user" => { template_parser::expect_no_arguments(function)?; - language.wrap_string(wrap_metadata_fn(self_property, |metadata| { + language.wrap_string(TemplateFunction::new(self_property, |op| { // TODO: introduce dedicated type and provide accessors? - format!("{}@{}", metadata.username, metadata.hostname) + format!("{}@{}", op.metadata().username, op.metadata().hostname) })) } "root" => { template_parser::expect_no_arguments(function)?; let root_op_id = language.root_op_id.clone(); - language.wrap_boolean(wrap_fn(self_property, move |op| op.id() == &root_op_id)) + language.wrap_boolean(TemplateFunction::new(self_property, move |op| { + op.id() == &root_op_id + })) } _ => return Err(TemplateParseError::no_such_method("Operation", function)), };