This appears to be a bit faster if there are tons of unchanged ranges.
```
group new old
----- --- ---
bench_diff_git_git_read_tree_c 1.00 58.5±0.12µs 1.07 62.7±0.60µs
bench_diff_lines/modified/10k 1.00 34.2±0.72ms 1.08 37.0±1.09ms
bench_diff_lines/modified/1k 1.00 3.1±0.08ms 1.12 3.5±0.01ms
bench_diff_lines/reversed/10k 1.00 28.0±0.15ms 1.01 28.4±0.51ms
bench_diff_lines/reversed/1k 1.00 616.0±16.20µs 1.00 617.0±9.29µs
bench_diff_lines/unchanged/10k 1.00 3.5±0.04ms 1.10 3.9±0.06ms
bench_diff_lines/unchanged/1k 1.00 328.4±4.44µs 1.07 352.0±1.41µs
```
One particular use case for these is escape sequences -- and to that
end, I'm also adding `\e` as a shorthand for `\x1b`.
Change-Id: Id000000040ea6fd8e2d720219931485960c570dd
The Ord implementation didn't conform to Eq (which compares "self.others"
literally), and we don't need Ord to insert non-overlapping ranges before the
current range.
This isn't fancy, but I couldn't find a better abstraction. Note that
MutableRepo::base_repo() isn't removed as it has to return &Arc<_>, whereas
<ReadonlyRepo as Repo>::base_repo() can't return &Arc<_>.
The id.shortest() template prints a warning and falls back to repo-global
resolution. This seems better than erroring out. There are a few edge cases
in which the short-prefixes resolution can fail unexpectedly. For example, the
trunk() revision might not exist in operations before "jj git clone".
The idea is that the disambiguation index can be loaded from a repo which is
different from the symbol resolution context.
Suppose we add at_operation(op, expr) revset, a symbol inside at_operation()
expression will have to be resolved within that operation, whereas the
disambiguation index is cached globally by WorkspaceCommandHelper. We could
build temporary disambiguation index for each at-op repo, but that would be
complicated implementation-wise, and wouldn't be useful. For example, a query
"x | at_operation(@-, x)" might be resolved to "xy | at_operation(@-, xz)"
if disambiguation index were reloaded for the @- operation. Instead, the
short change ID "x" can be disambiguated to "xy", then resolved to the
corresponding commit IDs at each operation.
This unblocks reuse of a symbol resolver instance for a different repo view
specified by at_operation() revset. See later commits for details. It's also
easier to handle error if there is a single function that can fail.
This removes an invalid View state from the root operation.
Note that the root index will have to be reindexed in order to resolve "root()"
in the root operation. I don't think this would practically matter, so this
patch doesn't bump the index version to invalidate the existing indexes.
See also 48a9f9ef56 "repo: use Transaction for creating repo-init operation."
See the next patch for why. It might look odd that OpStore depends on the root
CommitId, but that seems okay because OpStore manages Views, and a View is
basically a set of CommitIds.
For #3673, we will have aliases such as:
```toml
'upload(revision)' = [
["fix", "-r", "$revision"],
["lint", "-r", "$revision"],
["git", "push", "-r", "$revision"],
]
```
Template aliases:
1) Start as Config::Value
2) Are converted to String
3) Are placed in the alias map
4) Expand to a TemplateExpression type via expand_defn.
However, command aliases:
1) Start as Config::Value
2) Are converted to Vec<Vec<String>>
3) Are placed in an alias map
4) Do not expand
Thus, AliasesMap will need to support non-string values.
We're likely to use the right (or new) context lines in rendered diffs, but
it's odd that the hunks iterator choose which context hunk to return. We'll
also need both contents to calculate left/right line numbers.
Since the hunk content types are the same, I also split enum DiffHunk into
{ kind, contents } pair.
This change made some diff benches slow, maybe because the generated code
becomes slightly worse due to the added abstraction? I'll revisit the
performance problem later. There are a couple of ways to mitigate it.
```
group new old
----- --- ---
bench_diff_git_git_read_tree_c 1.02 61.0±0.23µs 1.00 59.7±0.38µs
bench_diff_lines/modified/10k 1.00 41.6±0.24ms 1.02 42.3±0.22ms
bench_diff_lines/modified/1k 1.00 3.8±0.07ms 1.00 3.8±0.03ms
bench_diff_lines/reversed/10k 1.29 23.4±0.20ms 1.00 18.2±0.26ms
bench_diff_lines/reversed/1k 1.05 517.2±5.55µs 1.00 493.7±59.72µs
bench_diff_lines/unchanged/10k 1.00 3.9±0.10ms 1.08 4.2±0.10ms
bench_diff_lines/unchanged/1k 1.01 356.8±2.33µs 1.00 353.7±1.99µs
```
(I don't get stable results on my noisy machine, so the results would vary.)
The added comparison functions correspond to --ignore-all-space and
--ignore-space-change. --ignore-space-at-eol can be combined with the other
flags, so it will have to be implemented as a preprocessing function.
--ignore-blank-lines will also require some change in the tokenizer function.
This could be implemented as a newtype `Wrapper<'a>(&'a [u8])`, but a lifetime
of the wrap function couldn't be specified correctly:
fn diff(left: &[u8], right: &[u8], wrap_fn: F, ..)
where
F: for<'a> Fn(&'a [u8]) -> W<'a>, // F::Output<'a> can't be specified
W: Copy + Eq + Hash
If the wrapper were of `&Wrapper([u8])` type, `Fn(&[u8]) -> &W` works. However,
it means we can no longer set comparison parameter (such as Regex) dynamically.
Another idea is to add some filter function of `Fn(&[u8]) -> Cow<'_, [u8]>`
type, but I don't think we would want to pay the allocation cost in
hashing/comparison code. `Fn(&[u8]) -> impl Iterator<Item = &[u8]>` might work,
but it would be equally complex.
We'll use low-level HashTable to customize Eq/Hash without implementing newtype
wrappers.
Unneeded default features are disabled for now. Note that the new default
hasher, foldhash, is released under the Zlib license, which isn't currently
included in the allow list.
Most collection references implement `.into_iter()` or its mutable version,
so it is possible to iterate over the elements without using an explicit
method to do so.
We have two options to achieve "diff --ignore-*-space":
a. preprocess contents to be diffed, then translate hunk ranges back
b. add hooks to customize eq and hash functions
I originally thought (a) would be easier, but actually, there aren't many
changes needed to implement (b). And (b) should have a fewer logic errors.
This patch removes assumption that each unchanged region has the same content
length. It won't be true if whitespace characters are ignored.
Intersection of unchanged ranges becomes a simple merge-join loop, so I've
removed the existing tests. I also added a fast path for the common 2-way
diffs in which we don't have to build vec![(pos, vec![pos])].
One source of confusion introduced by this change is that WordPosition means
both global and local indices. This is covered by the added tests, but I might
add separate local/global types later.
It's silly that we build new Vec for each recursion stack and merge elements
back. I don't see a measurable performance difference in the diff bench, but
this change will help simplify the next patch. If a result vec were created for
each unchanged_ranges() invocation, it would probably make more sense to return
a list of "local" word positions. Then, callers would have to translate the
returned positions to the caller's local positions.
We can assign a unique integer to each (word, occurrence) pair instead. As a
bonus, HashMap can be replaced with Vec.
```
group new old
----- --- ---
bench_diff_git_git_read_tree_c 1.00 72.5±3.25µs 1.08 78.5±0.48µs
bench_diff_lines/modified/10k 1.00 45.1±1.18ms 1.10 49.8±1.85ms
bench_diff_lines/modified/1k 1.00 4.1±0.07ms 1.11 4.5±0.34ms
bench_diff_lines/reversed/10k 1.00 19.0±0.12ms 1.12 21.2±1.26ms
bench_diff_lines/reversed/1k 1.00 558.5±37.42µs 1.17 655.6±16.27µs
bench_diff_lines/unchanged/10k 1.00 5.3±0.78ms 1.33 7.0±0.89ms
bench_diff_lines/unchanged/1k 1.00 422.0±16.68µs 1.28 540.7±13.96µs
```