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.
* We started with a tristate flag where:
- Auto - Maintain current behaviour. This edits if
the wc parent is not a head commit. Else, it will
create a new commit on the parent of the wc in
the direction of movement.
- Always - Always edit
- Never - Never edit, prefer the new+squash workflow.
However, consensus the review thread is that `auto` mode where we try to infer when to
switch to `edit mode`, should be removed. So `ui.movement.edit` is a boolean flag now.
- true: edit mode
- false: new+squash mode
* Also add a `--no-edit` flag as the explicit inverse of `--edit` and
ensure both flags take precedence over the config.
* Update tests that assumed edit mode inference, to specify `--edit` explicitly.
NOTE: #4302 was squashed into this commit, so see that closed PR for review history.
Part of #3947
[VSCodium](https://vscodium.com/) is a free/libre distribution of
Microsoft's Visual Studio Code editor, it's functionally more or less
the same, but distributed under a FOSS license, unlike VS Code.
This adds VSCodium as a merge tool.
I plan to provide a richer version of `TreeDiffEntry` with copy info
(and to make `TreeDiffEntry` itself "poorer"). Most callers want to
know about copies/renames, but at least working copy implementations
probably don't. This patch adds separate `diff_stream()` and
`diff_stream_with_copies()` so we can provide the simpler interface
for callers that don't need copy info.
The support for copy tracing is already simply added to the stream
just before yielding the item, so we can easily implement it as a
stream adapter. That ensures that we use the same logic for the
iterator- and stream-based versions. More importantly, it enables
further cleanups and a simpler interface.
So that more tests can leverage diff::diff() helper.
I also removed the fast path for identical inputs. This function is only used by
tests and benches, and production code usually compares content hashes first.
The tree-level conflicts have worked well in practice and we don't
want to allow users to use legacy trees for new commits. We don't
really support legacy trees very well since 0590f8bece anyway.
* Derive a bunch of standard and useful traits for `movement_util::Direction`
as it is a simple type. Importantly `Copy`.
* Return `&'static str` from Direction.cmd()
* Refactor out `MovementArgs` to reduce the number of arguments
to `movement_util::move_to_commit`.
* Implement `From<&NextArgs/&PrevArgs>` for MovementArgs
Part of #3947
This allows us to select rendering function hunk by hunk. For example, a hunk
with lots of small changes could be rendered without interleaving left/right
words. Another good thing is that context line handling can be simplified as
the whole context hunk is available.
I'm going to split color-words diffs to by_line() and by_word() stages.
Perhaps, Diff::default_refinement() can be removed once all non-test callers
are migrated.
The code in both cli/src/commands/{next,prev}.rs is identical except
for the direction of movement. This commit pull the parts that make
sense out into cli/src/movement_util.rs so it's easier to see the
differences.
Part of #3947
Add gratuitous `jj log` output to more points in the tests.
This makes it easier to understand the intended changes
by literally visualizing the commit tree we are after each movement.
This is at least useful for me since I find the new+squash workflow
confusing.
Test behaviour is not changed.
Part of #3947
We all noticed that x86 macOS binaries are no longer being provided on release,
due to `macos-11` runners going the way of the Dodo a while back. Nobody
alterted us to this, funny enough.
After some quick discussion, we concluded some things:
- x86 macOS runners are likely oversubscribed, and hurt CI latency badly
- `macos-12` is also deprecated; `macos-13` is the best x86 runner available
- GitHub probably isn't going to expand macOS runner capacity; `macos-13` will
one day go away
- Some people are still using `jj` on Intel Macs. We didn't get alerted because
they do their own builds for now, but may not always do that.
- We can just try to build on `macos-13` and make it optional for merges.
So that's what this does. It might be mergeable outright, but we can also use it
to measure build latency impacts.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
