We want to move UI-independent logic that's currently in `jj-cli` into
`jj-lib`. `WorkspaceCommandHelper` is perhaps the most important part
to start moving. As I was looking into what to move from
`WorkspaceCommandHelper`, the first thing I saw there was the
`cwd`. It might seem like a good candidate to start moving. However,
when running a server, you might be running operations on repos stored
in database, so `cwd` and the workspace root don't make sense then
(because the repo is not stored at a particular path).
So, instead, this patch starts abstracting out our uses of those two
paths by adding an enum for converting between `RepoPath` and paths as
they are presented in the UI. I added a variant for repos stored in a
file system, and made `WorkspaceCommandHelper` use that to show that
it works. We'll probably add a server variant later.
I put the new type in `repo_path.rs` because at least the
file-system-based implementation is closely related to
`RepoPath::parse_fs_path()`.
Before this patch, `MergedTreeBuilder::write_tree()` would create a
new legacy tree if the base tree was a legacy tree. This patch makes
it always write multiple root trees instead (if there are conflicts).
We still support reading legacy conflicts if the
`format.tree-level-conflicts` config is set.
It's been about six months since we started using tree-level conflicts
by default. I can't imagine we would switch back. So let's continue
the migration by always using tree-level conflicts when merging trees,
even if all inputs were legacy trees.
In chromium/src.git, this gives an approximate ~0.83s speedup for
commands if the Git index is empty, and a ~0.14s slowdown if the Git
index is non-empty.
As most users using jj will likely be using jj to write to a colocated
repo - and therefore avoid modifying the Git index - this should be a
general speedup for most colocated checkouts.
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.
This revset correctly implements "reachability" from a set of source commits following both parent and child edges as far as they can go within a domain set. This type of 'bfs' query is currently impossible to express with existing revset functions.
This follows up on https://github.com/martinvonz/jj/pull/3459 and adds a
label to the closing delimeter of each conflict, e.g. "Conflict 1 of 3
ends".
I didn't initially put any label at the ending delimeter since the
starting delimeter is already marked with "Conflict 1 of 3". However,
I'm now realizing that when I resolve conflicts, I usually go from top
to bottom. The first thing I do is delete the starting conflict
delimeter. It is when I get to the *end* of the conflict that I wonder
whether there are any more conflicts left in the file.
When I addded the workaround in 256988de65, I missed the comment
just below explaining that heads removed by the other side were
already handled. Since that's not handled when using non-default
indexes now, we need to handle it in an `else` block.
The function now returns an iterator over `Result`s, matching
`Operation::parents()`.
I updated callers to also propagate the error where it was trivial.
We do a 3-way merge of repo views in a few different
scenarios. Perhaps the most obvious one is when merging concurrent
operations. Another case is when undoing an operation.
One step of the 3-way repo view merge is to find which added commits
are rewrites of which removed commits (by comparing change IDs). With
the high commit rate in the Google repo (combined with storing the
commits on a server), this step can get extremely slow.
This patch adds a hack to disable the slow step when using a
non-standard `Index` implementation. The Google index is the only
non-standard implementation I'm aware of, so I don't think this will
affect anyone else. The patch is also small enough that I don't think
it will cause much maintenance overhead for the project.
parse_function_argument_to_file/string_pattern() functions aren't moved.
They are more like functions that transform parsed tree to intermediate
representation.
I'm planning to rewrite the parser in a similar way to fileset/template parsing,
and the revset_parser module will host functions and types for the first stage.
I haven't started the rewrite, but it seems good to split the revset module
even if we reject the idea.
This is more consistent with the other method and it makes some extension operations easier, by giving access to the OpStore and other relevant context for custom index extensions.
