mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-01-14 16:37:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

templater: migrate global functions to table-based lookup

Browse source 
The original plan was to extend the globals table to implement "revset(expr)".
I'm not sure if that's more discoverable than "self.contained_in(revset_expr)"
method, but we can decide that later. Anyways, this patch adds typo suggestion
for global functions.
This commit is contained in:
Yuya Nishihara 2024-03-14 19:42:08 +09:00
parent 85f2f3a439
commit 82b6d073f1
6 changed files with 147 additions and 110 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
cli/src/commit_templater.rs
View file

@ -112,7 +112,8 @@ impl<'repo> TemplateLanguage<'repo> for CommitTemplateLanguage<'repo> {
build_ctx: &BuildContext<Self::Property>, build_ctx: &BuildContext<Self::Property>,
function: &FunctionCallNode, function: &FunctionCallNode,
) -> TemplateParseResult<Self::Property> { ) -> TemplateParseResult<Self::Property> {
template_builder::build_global_function(self, build_ctx, function) let table = &self.build_fn_table.core;
table.build_function(self, build_ctx, function)
} }
fn build_method( fn build_method(

3
cli/src/generic_templater.rs
View file

@ -88,7 +88,8 @@ impl<'a, C: 'a> TemplateLanguage<'a> for GenericTemplateLanguage<'a, C> {
build_ctx: &BuildContext<Self::Property>, build_ctx: &BuildContext<Self::Property>,
function: &FunctionCallNode, function: &FunctionCallNode,
) -> TemplateParseResult<Self::Property> { ) -> TemplateParseResult<Self::Property> {
template_builder::build_global_function(self, build_ctx, function) let table = &self.build_fn_table.core;
table.build_function(self, build_ctx, function)
} }
fn build_method( fn build_method(

3
cli/src/operation_templater.rs
View file

@ -64,7 +64,8 @@ impl TemplateLanguage<'static> for OperationTemplateLanguage {
build_ctx: &BuildContext<Self::Property>, build_ctx: &BuildContext<Self::Property>,
function: &FunctionCallNode, function: &FunctionCallNode,
) -> TemplateParseResult<Self::Property> { ) -> TemplateParseResult<Self::Property> {
template_builder::build_global_function(self, build_ctx, function) let table = &self.build_fn_table.core;
table.build_function(self, build_ctx, function)
} }
fn build_method( fn build_method(

206
cli/src/template_builder.rs
View file

@ -74,6 +74,9 @@ pub trait TemplateLanguage<'a> {
fn build_self(&self) -> Self::Property; fn build_self(&self) -> Self::Property;
/// Translates the given global `function` call to a property. /// Translates the given global `function` call to a property.
///
/// This should be delegated to
/// `CoreTemplateBuildFnTable::build_function()`.
fn build_function( fn build_function(
&self, &self,
build_ctx: &BuildContext<Self::Property>, build_ctx: &BuildContext<Self::Property>,
@ -233,12 +236,20 @@ impl<'a, I: 'a> IntoTemplateProperty<'a, I> for CoreTemplatePropertyKind<'a, I>
} }
} }
/// Function that translates method call node of self type `T`. /// Function that translates global function call node.
// The lifetime parameter 'a could be replaced with for<'a> to keep the method // The lifetime parameter 'a could be replaced with for<'a> to keep the method
// table away from a certain lifetime. That's technically more correct, but I // table away from a certain lifetime. That's technically more correct, but I
// couldn't find an easy way to expand that to the core template methods, which // couldn't find an easy way to expand that to the core template methods, which
// are defined for L: TemplateLanguage<'a>. That's why the build fn table is // are defined for L: TemplateLanguage<'a>. That's why the build fn table is
// bound to a named lifetime, and therefore can't be cached statically. // bound to a named lifetime, and therefore can't be cached statically.
pub type TemplateBuildFunctionFn<'a, L> =
fn(
&L,
&BuildContext<<L as TemplateLanguage<'a>>::Property>,
&FunctionCallNode,
) -> TemplateParseResult<<L as TemplateLanguage<'a>>::Property>;
/// Function that translates method call node of self type `T`.
pub type TemplateBuildMethodFn<'a, L, T> = pub type TemplateBuildMethodFn<'a, L, T> =
fn( fn(
&L, &L,
@ -247,19 +258,22 @@ pub type TemplateBuildMethodFn<'a, L, T> =
&FunctionCallNode, &FunctionCallNode,
) -> TemplateParseResult<<L as TemplateLanguage<'a>>::Property>; ) -> TemplateParseResult<<L as TemplateLanguage<'a>>::Property>;
/// Table of functions that translate global function call node.
pub type TemplateBuildFunctionFnMap<'a, L> = HashMap<&'static str, TemplateBuildFunctionFn<'a, L>>;
/// Table of functions that translate method call node of self type `T`. /// Table of functions that translate method call node of self type `T`.
pub type TemplateBuildMethodFnMap<'a, L, T> = pub type TemplateBuildMethodFnMap<'a, L, T> =
HashMap<&'static str, TemplateBuildMethodFn<'a, L, T>>; HashMap<&'static str, TemplateBuildMethodFn<'a, L, T>>;
/// Symbol table of methods available in the core template. /// Symbol table of functions and methods available in the core template.
pub struct CoreTemplateBuildFnTable<'a, L: TemplateLanguage<'a> + ?Sized> { pub struct CoreTemplateBuildFnTable<'a, L: TemplateLanguage<'a> + ?Sized> {
pub functions: TemplateBuildFunctionFnMap<'a, L>,
pub string_methods: TemplateBuildMethodFnMap<'a, L, String>, pub string_methods: TemplateBuildMethodFnMap<'a, L, String>,
pub boolean_methods: TemplateBuildMethodFnMap<'a, L, bool>, pub boolean_methods: TemplateBuildMethodFnMap<'a, L, bool>,
pub integer_methods: TemplateBuildMethodFnMap<'a, L, i64>, pub integer_methods: TemplateBuildMethodFnMap<'a, L, i64>,
pub signature_methods: TemplateBuildMethodFnMap<'a, L, Signature>, pub signature_methods: TemplateBuildMethodFnMap<'a, L, Signature>,
pub timestamp_methods: TemplateBuildMethodFnMap<'a, L, Timestamp>, pub timestamp_methods: TemplateBuildMethodFnMap<'a, L, Timestamp>,
pub timestamp_range_methods: TemplateBuildMethodFnMap<'a, L, TimestampRange>, pub timestamp_range_methods: TemplateBuildMethodFnMap<'a, L, TimestampRange>,
// TODO: add global functions table?
} }
pub fn merge_fn_map<'s, F>(base: &mut HashMap<&'s str, F>, extension: HashMap<&'s str, F>) { pub fn merge_fn_map<'s, F>(base: &mut HashMap<&'s str, F>, extension: HashMap<&'s str, F>) {
@ -271,9 +285,10 @@ pub fn merge_fn_map<'s, F>(base: &mut HashMap<&'s str, F>, extension: HashMap<&'
} }
impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> { impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> {
/// Creates new symbol table containing the builtin methods. /// Creates new symbol table containing the builtin functions and methods.
pub fn builtin() -> Self { pub fn builtin() -> Self {
CoreTemplateBuildFnTable { CoreTemplateBuildFnTable {
functions: builtin_functions(),
string_methods: builtin_string_methods(), string_methods: builtin_string_methods(),
boolean_methods: HashMap::new(), boolean_methods: HashMap::new(),
integer_methods: HashMap::new(), integer_methods: HashMap::new(),
@ -285,6 +300,7 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> {
pub fn empty() -> Self { pub fn empty() -> Self {
CoreTemplateBuildFnTable { CoreTemplateBuildFnTable {
functions: HashMap::new(),
string_methods: HashMap::new(), string_methods: HashMap::new(),
boolean_methods: HashMap::new(), boolean_methods: HashMap::new(),
integer_methods: HashMap::new(), integer_methods: HashMap::new(),
@ -296,6 +312,7 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> {
pub fn merge(&mut self, extension: CoreTemplateBuildFnTable<'a, L>) { pub fn merge(&mut self, extension: CoreTemplateBuildFnTable<'a, L>) {
let CoreTemplateBuildFnTable { let CoreTemplateBuildFnTable {
functions,
string_methods, string_methods,
boolean_methods, boolean_methods,
integer_methods, integer_methods,
@ -304,6 +321,7 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> {
timestamp_range_methods, timestamp_range_methods,
} = extension; } = extension;
merge_fn_map(&mut self.functions, functions);
merge_fn_map(&mut self.string_methods, string_methods); merge_fn_map(&mut self.string_methods, string_methods);
merge_fn_map(&mut self.boolean_methods, boolean_methods); merge_fn_map(&mut self.boolean_methods, boolean_methods);
merge_fn_map(&mut self.integer_methods, integer_methods); merge_fn_map(&mut self.integer_methods, integer_methods);
@ -312,6 +330,18 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> {
merge_fn_map(&mut self.timestamp_range_methods, timestamp_range_methods); merge_fn_map(&mut self.timestamp_range_methods, timestamp_range_methods);
} }
/// Translates the function call node `function` by using this symbol table.
pub fn build_function(
&self,
language: &L,
build_ctx: &BuildContext<L::Property>,
function: &FunctionCallNode,
) -> TemplateParseResult<L::Property> {
let table = &self.functions;
let build = template_parser::lookup_function(table, function)?;
build(language, build_ctx, function)
}
/// Applies the method call node `function` to the given `property` by using /// Applies the method call node `function` to the given `property` by using
/// this symbol table. /// this symbol table.
pub fn build_method( pub fn build_method(
@ -895,92 +925,88 @@ where
Ok(language.wrap_list_template(Box::new(list_template))) Ok(language.wrap_list_template(Box::new(list_template)))
} }
pub fn build_global_function<'a, L: TemplateLanguage<'a> + ?Sized>( fn builtin_functions<'a, L: TemplateLanguage<'a> + ?Sized>() -> TemplateBuildFunctionFnMap<'a, L> {
language: &L, // Not using maplit::hashmap!{} or custom declarative macro here because
build_ctx: &BuildContext<L::Property>, // code completion inside macro is quite restricted.
function: &FunctionCallNode, let mut map = TemplateBuildFunctionFnMap::<L>::new();
) -> TemplateParseResult<L::Property> { map.insert("fill", |language, build_ctx, function| {
match function.name { let [width_node, content_node] = template_parser::expect_exact_arguments(function)?;
"fill" => { let width = expect_usize_expression(language, build_ctx, width_node)?;
let [width_node, content_node] = template_parser::expect_exact_arguments(function)?; let content = expect_template_expression(language, build_ctx, content_node)?;
let width = expect_usize_expression(language, build_ctx, width_node)?; let template = ReformatTemplate::new(content, move |context, formatter, recorded| {
let content = expect_template_expression(language, build_ctx, content_node)?; match width.extract(context) {
let template = ReformatTemplate::new(content, move |context, formatter, recorded| { Ok(width) => text_util::write_wrapped(formatter, recorded, width),
match width.extract(context) { Err(err) => err.format(&(), formatter),
Ok(width) => text_util::write_wrapped(formatter, recorded, width), }
Err(err) => err.format(&(), formatter), });
} Ok(language.wrap_template(Box::new(template)))
}); });
Ok(language.wrap_template(Box::new(template))) map.insert("indent", |language, build_ctx, function| {
} let [prefix_node, content_node] = template_parser::expect_exact_arguments(function)?;
"indent" => { let prefix = expect_template_expression(language, build_ctx, prefix_node)?;
let [prefix_node, content_node] = template_parser::expect_exact_arguments(function)?; let content = expect_template_expression(language, build_ctx, content_node)?;
let prefix = expect_template_expression(language, build_ctx, prefix_node)?; let template = ReformatTemplate::new(content, move |context, formatter, recorded| {
let content = expect_template_expression(language, build_ctx, content_node)?; text_util::write_indented(formatter, recorded, |formatter| {
let template = ReformatTemplate::new(content, move |context, formatter, recorded| { prefix.format(context, formatter)
text_util::write_indented(formatter, recorded, |formatter| { })
prefix.format(context, formatter) });
}) Ok(language.wrap_template(Box::new(template)))
}); });
Ok(language.wrap_template(Box::new(template))) map.insert("label", |language, build_ctx, function| {
} let [label_node, content_node] = template_parser::expect_exact_arguments(function)?;
"label" => { let label_property = expect_plain_text_expression(language, build_ctx, label_node)?;
let [label_node, content_node] = template_parser::expect_exact_arguments(function)?; let content = expect_template_expression(language, build_ctx, content_node)?;
let label_property = expect_plain_text_expression(language, build_ctx, label_node)?; let labels = TemplateFunction::new(label_property, |s| {
let content = expect_template_expression(language, build_ctx, content_node)?; Ok(s.split_whitespace().map(ToString::to_string).collect())
let labels = TemplateFunction::new(label_property, |s| { });
Ok(s.split_whitespace().map(ToString::to_string).collect()) Ok(language.wrap_template(Box::new(LabelTemplate::new(content, labels))))
}); });
Ok(language.wrap_template(Box::new(LabelTemplate::new(content, labels)))) map.insert("if", |language, build_ctx, function| {
} let ([condition_node, true_node], [false_node]) =
"if" => { template_parser::expect_arguments(function)?;
let ([condition_node, true_node], [false_node]) = let condition = expect_boolean_expression(language, build_ctx, condition_node)?;
template_parser::expect_arguments(function)?; let true_template = expect_template_expression(language, build_ctx, true_node)?;
let condition = expect_boolean_expression(language, build_ctx, condition_node)?; let false_template = false_node
let true_template = expect_template_expression(language, build_ctx, true_node)?; .map(|node| expect_template_expression(language, build_ctx, node))
let false_template = false_node .transpose()?;
.map(|node| expect_template_expression(language, build_ctx, node)) let template = ConditionalTemplate::new(condition, true_template, false_template);
.transpose()?; Ok(language.wrap_template(Box::new(template)))
let template = ConditionalTemplate::new(condition, true_template, false_template); });
Ok(language.wrap_template(Box::new(template))) map.insert("concat", |language, build_ctx, function| {
} let contents = function
"concat" => { .args
let contents = function .iter()
.args .map(|node| expect_template_expression(language, build_ctx, node))
.iter() .try_collect()?;
.map(|node| expect_template_expression(language, build_ctx, node)) Ok(language.wrap_template(Box::new(ConcatTemplate(contents))))
.try_collect()?; });
Ok(language.wrap_template(Box::new(ConcatTemplate(contents)))) map.insert("separate", |language, build_ctx, function| {
} let ([separator_node], content_nodes) = template_parser::expect_some_arguments(function)?;
"separate" => { let separator = expect_template_expression(language, build_ctx, separator_node)?;
let ([separator_node], content_nodes) = let contents = content_nodes
template_parser::expect_some_arguments(function)?; .iter()
let separator = expect_template_expression(language, build_ctx, separator_node)?; .map(|node| expect_template_expression(language, build_ctx, node))
let contents = content_nodes .try_collect()?;
.iter() Ok(language.wrap_template(Box::new(SeparateTemplate::new(separator, contents))))
.map(|node| expect_template_expression(language, build_ctx, node)) });
.try_collect()?; map.insert("surround", |language, build_ctx, function| {
Ok(language.wrap_template(Box::new(SeparateTemplate::new(separator, contents)))) let [prefix_node, suffix_node, content_node] =
} template_parser::expect_exact_arguments(function)?;
"surround" => { let prefix = expect_template_expression(language, build_ctx, prefix_node)?;
let [prefix_node, suffix_node, content_node] = let suffix = expect_template_expression(language, build_ctx, suffix_node)?;
template_parser::expect_exact_arguments(function)?; let content = expect_template_expression(language, build_ctx, content_node)?;
let prefix = expect_template_expression(language, build_ctx, prefix_node)?; let template = ReformatTemplate::new(content, move |context, formatter, recorded| {
let suffix = expect_template_expression(language, build_ctx, suffix_node)?; if recorded.data().is_empty() {
let content = expect_template_expression(language, build_ctx, content_node)?; return Ok(());
let template = ReformatTemplate::new(content, move |context, formatter, recorded| { }
if recorded.data().is_empty() { prefix.format(context, formatter)?;
return Ok(()); recorded.replay(formatter)?;
} suffix.format(context, formatter)?;
prefix.format(context, formatter)?; Ok(())
recorded.replay(formatter)?; });
suffix.format(context, formatter)?; Ok(language.wrap_template(Box::new(template)))
Ok(()) });
}); map
Ok(language.wrap_template(Box::new(template)))
}
_ => Err(TemplateParseError::no_such_function(function)),
}
} }
/// Builds intermediate expression tree from AST nodes. /// Builds intermediate expression tree from AST nodes.

30
cli/src/template_parser.rs
View file

@ -136,17 +136,6 @@ impl TemplateParseError {
} }
} }
// TODO: migrate callers to something like lookup_method()
pub(crate) fn no_such_function(function: &FunctionCallNode) -> Self {
TemplateParseError::with_span(
TemplateParseErrorKind::NoSuchFunction {
name: function.name.to_owned(),
candidates: vec![],
},
function.name_span,
)
}
// TODO: migrate all callers to table-based lookup_method() // TODO: migrate all callers to table-based lookup_method()
pub(crate) fn no_such_method( pub(crate) fn no_such_method(
type_name: impl Into<String>, type_name: impl Into<String>,
@ -916,6 +905,25 @@ pub fn expect_lambda_with<'a, 'i, T>(
} }
} }
/// Looks up `table` by the given function name.
pub fn lookup_function<'a, V>(
table: &'a HashMap<&str, V>,
function: &FunctionCallNode,
) -> TemplateParseResult<&'a V> {
if let Some(value) = table.get(function.name) {
Ok(value)
} else {
let candidates = collect_similar(function.name, table.keys());
Err(TemplateParseError::with_span(
TemplateParseErrorKind::NoSuchFunction {
name: function.name.to_owned(),
candidates,
},
function.name_span,
))
}
}
/// Looks up `table` by the given method name. /// Looks up `table` by the given method name.
pub fn lookup_method<'a, V>( pub fn lookup_method<'a, V>(
type_name: impl Into<String>, type_name: impl Into<String>,

12
cli/tests/test_templater.rs
View file

@ -38,7 +38,7 @@ fn test_templater_parse_error() {
[template-aliases] [template-aliases]
'conflicting' = '' 'conflicting' = ''
'shorted()' = '' 'shorted()' = ''
'cap(x)' = 'x' 'socat(x)' = 'x'
'format_id(id)' = 'id.sort()' 'format_id(id)' = 'id.sort()'
"###, "###,
); );
@ -60,14 +60,14 @@ fn test_templater_parse_error() {
= Method "shorter" doesn't exist for type "CommitOrChangeId" = Method "shorter" doesn't exist for type "CommitOrChangeId"
Hint: Did you mean "short", "shortest"? Hint: Did you mean "short", "shortest"?
"###); "###);
insta::assert_snapshot!(render_err(r#"cat()"#), @r###" insta::assert_snapshot!(render_err(r#"oncat()"#), @r###"
Error: Failed to parse template: --> 1:1 Error: Failed to parse template: --> 1:1
| |
1 | cat() 1 | oncat()
| ^-^ | ^---^
| |
= Function "cat" doesn't exist = Function "oncat" doesn't exist
Hint: Did you mean "cap"? Hint: Did you mean "concat", "socat"?
"###); "###);
insta::assert_snapshot!(render_err(r#""".lines().map(|s| se)"#), @r###" insta::assert_snapshot!(render_err(r#""".lines().map(|s| se)"#), @r###"
Error: Failed to parse template: --> 1:20 Error: Failed to parse template: --> 1:20