The original expand_node() body is migrated as follows:
- Identifier -> fold_identifier()
- FunctionCall -> fold_function_call()
expand_defn() now manages states stack by itself, which simplifies lifetime
parameters.
The templater implementation of FoldableExpression is a stripped-down version
of expand_node(). It's visitor-like because I'm going to write generic alias
substitution rules over abstract expression types (template, revset, fileset.)
Naming comes from rustc.
https://rust-unofficial.github.io/patterns/patterns/creational/fold.html
This is basically the same as the previous patch, but for error types. Some
of these functions could be encoded as "E: From<AliasExpandError<'i>>", but
alias substitution logic is recursive, so it would have to convert E back and
force.
I'm going to extract generic alias substitution functions, and these AST types
will be accessed there. Revset parsing will also be migrated to the generic
functions.
This will help extract common FunctionCallNode<'i, T> type. We don't need
freedom of arbitrary error type choices, but implementing From<_> is the
easiest option I can think of. Another option is to constrain error type by
the expression type T through "T::ParseError: ArgumentsParseError" or
something, but it seemed a bit weird that we have to use trait just for that.
I'll probably rewrite expand_aliases() in visitor-like interface, and tree
traversal logic will be implemented on ExpressionKind. That's why I made
expand_node() destructure ExpressionNode first.
This consolidates the type of substitution results. Before, symbol substitution
can return inner ExpressionKind internally, but function-parameter substitution
couldn't.
I'm trying to extract generic alias substitution functions, and some of them
will take ExpressionKind or Box<FunctionCallNode> by value, then return it or
substituted value of the same type. The cost of moving values wouldn't matter
in practice, but I think it's better to keep the value types small.
Now ExpressionKind is 4-word long.
Since fileset/revset/template expressions are specified as command-line
arguments, it's sometimes convenient to use single quotes instead of double
quotes. Various scripting languages parse single-quoted strings in various ways,
but I choose the TOML rule because it's simple and practically useful. TOML is
our config language, so copying the TOML syntax would be less surprising than
borrowing it from another language.
https://github.com/toml-lang/toml/issues/188
This patch adds "string_" prefix to the related rules to discriminate them from
integer_literal. I also renamed "raw_literal" because it sounds like a raw
string literal that preserves backslash characters.
After upgrading pest from 2.7.8 to 2.7.9, I noticed CLI tests got significantly
slow (something around 40sec -> 60sec on my laptop.) I suspect this would be
caused by detailed error state tracking introduced in 2.7.9, but it's also true
that our template grammar exercises such code path.
My understanding is that PEG is basically a top down parsing with unlimited
lookahead. Before this change, the default commit_summary template would be
parsed as follows:
1. parse the outermost separate(..) as "term"
2. "concat" rule can't continue, so
3. reparse the whole string as "expression"
Because this pattern is not uncommon, I think it's better to rewrite the
grammar to avoid large retry.
With this patch, our tests runs within ~50sec under debug build. It appears to
save a few milliseconds in release build, but my development environment isn't
quiet enough to say the difference is significant.
I'm going to add RevsetParseError constructor for InvalidFunctionArguments,
with/without a source error, and I don't want to duplicate code for all
combinations. The templater change is just for consistency.
I couldn't find a good naming convention for the builder-like API, so it's
called .with_source(mut self, _). Another option was .source_set(source).
Apparently, it's not uncommon to name consuming constructor as
with_<something>().
Because the CLI error handler now prints error sources in multi-line format,
it doesn't make much sense to render Revset/TemplateParseError differently.
This patch also fixes the source() of the SyntaxError kind. It should be
self.pest_error.source() (= None), not self.pest_error.
I'm going to make TemplateParseError hold RevsetParseError as Box<dyn _>, but
Box<dyn std::error::Error ..> doesn't implement Eq. I could remove Eq from
ErrorKind enums, but it's handly if these enums remain as value types.
This change will also simplify fmt::Display and error::Error impls.
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.
The translation from method error to keyword error can go wrong if the context
object had n-ary methods (n > 0), which isn't the case as of now. For
simplicity, arguments error is mapped to "self.<name>(..)" suggestion.
Local variables and "self" could be merged without using extra method, but
we'll need extend_*_candidates() to merge in symbol/function aliases anyway.
Follows up 9702a425e5 "Allow negative numbers in the template grammar."
Since we've added parsing rules for operator expressions, it makes sense to
parse unary '-' as operator.
These operator symbols are different from the ones in the revset language. I
have no idea if we need bitwise operators, but we'll probably add comparison
operators. It would look weird if 'x == y & z' were parsed as '(x == y) & z'.
These error types are special because the message is embedded in ASCII art. I
think it would be a source of bugs if some error types had ": {source}" but
others don't. So I'm going to remove all ": {source}"s, and let the callers
concatenate them when needed.
Summary: In preparation for unifying all workspace dependencies across all
crates, let's go ahead and move the jj-cli crate into its own new directory.
This will also be a nicer and more uniform layout as we add new `jj-*` crates.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Icf94e7ae5f290dc8e181215727b38ada
