I just choose "clru" because it already exists in our dependency tree thorough
gix. I don't think LRU is the best cache eviction policy for our use case (a
simpler FIFO-based one might be good enough?), but it wouldn't matter for CLI
or GUI use case. I don't see significant performance degradation with "jj log
--stat -n1000".
RwLock is replaced with Mutex since get() is inherently a mutable operation.
If merge-heavy history was abandoned, intermediate parent chains can have tons
of duplicates, and the process explodes soon. Instead, we can skip any parent
ids that have been remapped.
We can no longer detect cycles reliably, but I think that's okay so long as
the function terminates.
Fixes#4352
I played with max-inline-alternation = 3 for a couple of weeks, and it's pretty
good. I think somewhere between 2 and 4 is good default because one or two
remove + add sequences are easy to parse.
FileConflict will be changed to not materialize Merge<BString>. I also updated
the revset engine to ignore non-file conflict. It doesn't make sense to grep
conflict description.
If movement commands don't find a target commit, they fail. However,
it's usually not intuitive why they fail because in non-edit mode the
start commit is the parent of the working commit.
Adding the start commit change hash to the error message makes it easier
for the user to figure out what is going on.
Also, specifying 'No **other** descendant...' helps make it clear what
`jj` is really looking for.
Part of #3947
The native backend currently errors out if you ask it about copies. So
does the test backend. I think it's better to return an empty stream
of copies so it doesn't prevent other functionality.
Not all callers need this information, but I assumed it's relatively cheap to
look up the source path in the target tree compared to diffing.
This could be represented as Regular(_)|Copied(_, _)|Renamed(_, _), but it's
a bit weird if Copied and Renamed were separate variants. Instead, I decided
to wrap copy metadata in Option.
This patch adds accessor methods as I'm going to change the underlying data
types. Since entry values are consumed separately, these methods are implemented
on CopiesTreeDiffEntryPath, not on *TreeDiffEntry.
Git reports a rename source as deleted if the rename target is excluded. I
think that's because Git restricts the search space to the specified paths. For
example, Git doesn't also recognize a rename if the source path is excluded
whereas jj does.
I don't think we need to copy the exact behavior of Git, so this patch just
moves matcher application to earlier stage. This change will help remove
collect_copied_sources().
The added get_copy_records() helper could be moved to jj_lib, but we'll probably
want a stream version of this function in library, and writing a stream adapter
isn't as simple as iterator.
In this patch, I use the number of adds<->removes alternation as a threshold,
which approximates the visual complexity of diff hunks. I don't think user can
choose the threshold intuitively, but we need a config knob to try out some.
I set `max-inline-alternation = 3` locally. 0 and 1 mean "disable inlining"
and "inline adds-only/removes-only lines" respectively.
I've added "diff.<format>" config namespace assuming "ui.diff" will be
reorganized as "ui.diff-formatter" or something. #3327
Some other metrics I've tried:
```
// Per-line alternation. This also works well, but can't measure complexity of
// changes across lines.
fn count_max_diff_alternation_per_line(diff_lines: &[DiffLine]) -> usize {
diff_lines
.iter()
.map(|line| {
let sides = line.hunks.iter().map(|&(side, _)| side);
sides
.filter(|&side| side != DiffLineHunkSide::Both)
.dedup() // omit e.g. left->both->left
.count()
})
.max()
.unwrap_or(0)
}
// Per-line occupancy of changes. Large diffs don't always look complex.
fn max_diff_token_ratio_per_line(diff_lines: &[DiffLine]) -> f32 {
diff_lines
.iter()
.filter_map(|line| {
let [both_len, left_len, right_len] =
line.hunks.iter().fold([0, 0, 0], |mut acc, (side, data)| {
let index = match side {
DiffLineHunkSide::Both => 0,
DiffLineHunkSide::Left => 1,
DiffLineHunkSide::Right => 2,
};
acc[index] += data.len();
acc
});
// left/right-only change is readable
(left_len != 0 && right_len != 0).then(|| {
let diff_len = left_len + right_len;
let total_len = both_len + left_len + right_len;
(diff_len as f32) / (total_len as f32)
})
})
.reduce(f32::max)
.unwrap_or(0.0)
}
// Total occupancy of changes. Large diffs don't always look complex.
fn total_change_ratio(diff_lines: &[DiffLine]) -> f32 {
let (diff_len, total_len) = diff_lines
.iter()
.flat_map(|line| &line.hunks)
.fold((0, 0), |(diff_len, total_len), (side, data)| {
let l = data.len();
match side {
DiffLineHunkSide::Both => (diff_len, total_len + l),
DiffLineHunkSide::Left => (diff_len + l, total_len + l),
DiffLineHunkSide::Right => (diff_len + l, total_len + l),
}
});
(diff_len as f32) / (total_len as f32)
}
```
Though this is needed only for the last line, checking it for each line is
cheap. As I'm going to add another rendering style, the condition to pad "\n"
would become more complicated.
