Merging is currently done with line-level granularity, so it makes
sense to have newlines after the markers. That makes them easier to
edit out when resolving conflicts.
This lets you use the same operator as we currently have for ancestors
and descendants (`,,`) to also specify a DAG range. That's what
Mercurial uses the `::` operator for and what Git has `git log
--ancestry-path` for.
It seems clearer to let the parsed `RevsetExpression`s have only root
and head expression instead of adding the ancestors when building the
expression tree.
I really liked the idea of having the operators for parents and
ancestors (etc.) look similar, but that turned out to be problematic
when we want to add an infix operator for a DAG range (hg's `::`
revset operator and git's `--ancestry-path` flag). Let's say we chose
`:*:` as the operator. Part of the problem is how to parse `foo:*:bar`
without eagerly parsing the `foo:`. It would also be nicer to use
exactly the same operator as prefix, postfix, and infix. Since the
"parents" operator can be repeated, we can't have it be just `:` and
the "ancestors" operator be `::`. We could make the "ancestors"
operator be something like `*:*` (or anything symmetric with the `:`
symbol on the inside). However, at that point, the operator is getting
ugly and hard to type. Another option would be to use `:` for
ancestors and `::` for parents, but that is counterintuitive and get
annoying if you want to repeat it. So it seems that the best option is
to simply pick different symbols for parents/children and
ancestors/descendants/range.
This patch changes the ancestors/descendants operators to both be
`,,`. I'm not at all attached to that particular symbol. I suspect
we'll change it later.
Now that expressions may contain literal strings, we can simply have
functions accept only expressions arguments. That simplifies both the
grammar and the code.
A small drawback is that `description((foo), bar)` is now allowed and
does a search for the string "foo" (not "(foo)"). That seems
unlikely to trip up users.
Git refs with names containing e.g "-" are currently not accepted
symbol names, and I don't plan to change the grammar to accept
them. Instead, let's have the user quote symbol names containing
unusual characters. That way we can keep these symbols reserved for
revset operators.
With this patch the user can do e.g. `jj diff -r '"v2.9.0-rc2"'`.
This adds `children(<set>)` and `<set>:` for the children of the given
set, and `descendants(<set>)` and `<set>:*` for the descendants of the
given set. The children and descendants are filtered to be among
ancestors of non-obsolete commits. I haven't added a way of overriding
that yet.
This is especially important now that we leak the rule names into the
`SyntaxError` message. For example, the error message when doing `jj
diff -r :` will now mention "expected parents_op, ancestors_op, or
primary". It seems much clearer with the "_op" suffixes there. Longer
term, we should think more about how we can best surface syntax errors
from the library crate.
The tests don't need any complex set up (no repo necessary), so they
can be in the `revset` module itself. I'm sure we'll need to split up
that module later (at least separate out the parsing), but that's a
separate problem.
I don't know why I made it walk by generation number to start
with. Walking by position is better in at least two ways: 1) revsets
now depend on the walks to be by descending index position (though
they could equally well depend on the walks to be by generation number
-- it just needs to be consistent), and 2) the log output gets less
interleaved.
This commit makes the number of bytes in the graphlog output in the
git.git repo drop by ~40% due to the reduced amount of
interleaving. Also, it reduces the time of `jj bench walkrevs v1.0.0
v2.0.0` in the git.git repo by 32% (9.4ms -> 6.4ms) and `jj bench
walkrevs v2.0.0 v1.0.0` by 33% (7.7ms -> 5.1ms).
This change adds a `non_obsolete_heads(<set>)` revset, which walks up
ancestors of the input set until it gets to a non-obsolete and
non-pruned commit. That's what we do by default in `jj log`
(i.e. without `--all`). Now we can make `jj log` use revsets and teach
it a `-r` option!
This adds `parents(foo)` and `ancestors(foo)` as alternative ways of
writing `:foo` and `*:foo`.
I haven't added support for for whitespace yet; the parsing is very
strict. The error messages will also need to be improved later.
This patch adds initial support for a DSL for specifying revisions
inspired by Mercurial's "revset" language. The initial support
includes prefix operators ":" (parents) and "*:" (ancestors) with
naive parsing of the revsets. Mercurial uses postfix operator "^" for
parent 1 just like Git does. It uses prefix operator "::" for
ancestors and the same operator as postfix operator for descendants. I
did it differently because I like the idea of using the same operator
as prefix/postfix depending on desired direction, so I wanted to apply
that to parents/children as well (and for
predecessors/successors). The "*" in the "*:" operator is copied from
regular expression syntax. Let's see how it works out. This is an
experimental VCS, after all.
I've updated the CLI to use the new revset support.
The implementation feels a little messy, but you have to start
somewhere...
This actually seems to make it slightly slower, but it fixes an
important bug (we used to evolve only one topological branch per `jj
evolve` call). The slowdown seemed to be on the order of 5% when
evolving 100 commits on git.git's "what's cooking" branch.
I suspect that at least one reason that I didn't make
`MutableRepo::base_repo` by an `Arc<ReadonlyRepo>` before was that I
thought that that would mean that `start_transaction()` would need be
moved off of `ReadonlyRepo` so it can be given an
`&Arc<ReadonlyRepo>`, which would make it much less convenient to
use. It turns out that a `self` argument can actually be of type
`&Arc<ReadonlyRepo>`.
See test case for details.
Before:
test bench_diff_10k_lines_reversed ... bench: 36,249,659 ns/iter (+/- 174,455)
test bench_diff_10k_modified_lines ... bench: 37,258,890 ns/iter (+/- 803,963)
test bench_diff_10k_unchanged_lines ... bench: 4,252 ns/iter (+/- 69)
test bench_diff_1k_lines_reversed ... bench: 982,834 ns/iter (+/- 6,467)
test bench_diff_1k_modified_lines ... bench: 3,343,469 ns/iter (+/- 23,243)
test bench_diff_1k_unchanged_lines ... bench: 231 ns/iter (+/- 2)
test bench_diff_git_git_read_tree_c ... bench: 95,559 ns/iter (+/- 816)
After:
test bench_diff_10k_lines_reversed ... bench: 36,186,715 ns/iter (+/- 196,903)
test bench_diff_10k_modified_lines ... bench: 37,511,000 ns/iter (+/- 1,370,476)
test bench_diff_10k_unchanged_lines ... bench: 3,099 ns/iter (+/- 8)
test bench_diff_1k_lines_reversed ... bench: 986,010 ns/iter (+/- 11,565)
test bench_diff_1k_modified_lines ... bench: 3,370,938 ns/iter (+/- 17,041)
test bench_diff_1k_unchanged_lines ... bench: 230 ns/iter (+/- 2)
test bench_diff_git_git_read_tree_c ... bench: 102,189 ns/iter (+/- 1,052)
So this patch makes diffing even slower (but still easily fast enough
for all cases I've run into in real life). There's probably a lot that
can be done to make things faster, but the first priority is that the
diffs are correct and easy to read.
This is yet another step towards making it easy to propagate
`BrokenPipe` errors. The `jj diff` code (naturally) diffs two trees
and prints the diffs. If the printing fails, we shouldn't just crash
like we do today.
The new code is probably slower since it does more copying (the
callback got references to the `FileRepoPath` and `TreeValue`). I hope
that won't make a noticeable difference. At least `jj diff -r
334afbc76fbd --summary` didn't seem to get measurably slower.
The iterator version is easier to use and we get rid of the ugly type
parameter for the error type. I also simplified the code by using
`Peekable` iterators.
The new diff algorithm produces pretty bad diffs in some cases, such
as cc4b1e9230 in this repo (the parent of this commit). I think the
problem there is that many words are repeated over and over. Diffing
first at the line level and then refining the diff of the changed
ranges at the word level gives much better results. That's what this
patch does. After this patch, `jj diff -r cc4b1e923091` looks pretty
similar to the diff in GitHub's UI.
I hope to get around to doing the same for the merge code soon.
Impact on benchmarks:
Before:
test bench_diff_10k_lines_reversed ... bench: 42,647,532 ns/iter (+/- 765,347)
test bench_diff_10k_modified_lines ... bench: 21,407,980 ns/iter (+/- 126,366)
test bench_diff_10k_unchanged_lines ... bench: 4,235 ns/iter (+/- 16)
test bench_diff_1k_lines_reversed ... bench: 1,190,483 ns/iter (+/- 7,192)
test bench_diff_1k_modified_lines ... bench: 1,919,766 ns/iter (+/- 9,665)
test bench_diff_1k_unchanged_lines ... bench: 231 ns/iter (+/- 1)
test bench_diff_git_git_read_tree_c ... bench: 174,702 ns/iter (+/- 1,199)
After:
test bench_diff_10k_lines_reversed ... bench: 38,289,509 ns/iter (+/- 129,004)
test bench_diff_10k_modified_lines ... bench: 33,140,659 ns/iter (+/- 3,989,339)
test bench_diff_10k_unchanged_lines ... bench: 3,099 ns/iter (+/- 14)
test bench_diff_1k_lines_reversed ... bench: 973,551 ns/iter (+/- 94,895)
test bench_diff_1k_modified_lines ... bench: 3,033,818 ns/iter (+/- 29,513)
test bench_diff_1k_unchanged_lines ... bench: 230 ns/iter (+/- 1)
test bench_diff_git_git_read_tree_c ... bench: 79,100 ns/iter (+/- 963)
So most of them get slower, as expected. The last one, taken from a
real diff in the git.git repo, get faster, however (which is also what
I would have expected).
I made a quite late change in a recent patch to make the merge code to
merge based on lines instead of words. I forgot to update the tests
(and to even run them). Sorry :(
The previous patch switched over the content-merge code to use the new
histogram diff code. This patch switches over the content-diff code to
use the histogram diff code. As before, the immediate goal is to speed
it up. `jj diff -r c28ded83fc` in the git.git repo is a good example
of a diff that's extremely slow to calculate with our current
LCS-based diff. With this patch, that drops from 35 s to 0.12 s.
The diff was slightly better before. I think that's mostly because of
our different definition of a "word" in the data. We can improve that
later. The speedup we get now is easily worth the slightly worse diff.
With the histogram diff code from the previous patch, we can now start
using that for finding the "sync regions" in 3-way merge. That helps a
lot with the slow merging we had before this patch. `jj diff -r
9d540e9726` in the git.git repo drops from 22 s to 0.15 s with this
patch. (That commit is a rather arbitrary merge commit from aroun 5
years ago.)
With the new diff algorithm, the output of `jj diff -r 9d540e9726` in
git.git looks better if we find unchanged sync regions based on lines
than on words, so that's what I'm using in this patch. That's a change
compared the the LCS-based diff we used before this patch. I suspect
the reason that finding sync regions based on words works worse now is
not because of the change from LCS to histogram but because of the
change in how we define a word. My goal right now is mostly to make it
faster; I'll get back to refining the diff result later.
The current diff algorithm does a full LCS on the words of the texts,
which is really slow. Diffing the working copy when e.g.
`src/commands.py` has changes far apart takes seconds. This patch adds
an implementation inspired by JGit's Histogram diff. I say "inspired"
because I just didn't quite understand it :P In particular, I didn't
understand what it does when it finds non-unique elements. I decided
to line up the leading common elements on both sides of the merge. I
don't know if that usually gives good enough results in practice.
I'm sure this can still be optimized a lot, but this seems good enough
as a start. There is also many things to improve about the quality of
the diffs.
