139 Commits

Author SHA1 Message Date
Dave Townsend
891fa88be0 diff: Allow setting the default level of context in config. 2024-10-25 17:56:56 +01:00
Yuya Nishihara
153873a093 diff: remove duplicated source message from AccessDenied error 2024-10-23 23:51:21 +09:00
Yuya Nishihara
ad4b940daa object_id: implement Display on ObjectId types
It's convenient if id can be inlined in error messages.
2024-10-16 09:12:16 +09:00
Yuya Nishihara
8bb806e7b6 formatter: flush color escape sequence when raw() stream is requested
This ensures that the data printed through the raw stream is colorized if the
formatter already had color labels, and if the raw data doesn't reset the
surrounding color. This would only matter in templates containing
label(.., raw_escape_sequence() ..) expression.

Fixes #4631
2024-10-14 11:44:24 +09:00
Yuya Nishihara
e5e85e781f diff: inline contents and ranges vecs up to two sides
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
```
2024-10-13 21:28:39 +09:00
Vamsi Avula
92103f4db2 formatters: implement raw() for FormatRecorder
This adds `raw_escape_sequence(...)` support for things that use
FormatRecorder like wrapped text / `fill(...)` / `indent(...)`.

Change-Id: Id00000004248b10feb2acd54d90115b783fac0ff
2024-10-13 12:02:40 +05:30
Yuya Nishihara
68f48605e9 cli: in color-words diffs, highlight word-level hunks in context lines
Since context lines are styled in the same way as diff lines, it makes sense
to highlight words within context lines.
2024-10-06 09:45:27 +09:00
Yuya Nishihara
1eeeb1462a cli: extract helpers from show_color_words_context_lines()
This should make the next patch a bit easier to read.
2024-10-06 09:45:27 +09:00
Yuya Nishihara
6877ec4316 cli: add diff --ignore-all-space/--ignore-space-change flags
These flags only apply to line-based diffs. This is easy, and seems still useful
to highlight whitespace changes (that could be ignored by line diffing.)

I've added short options only to "diff"-like commands. It seemed unclear if
they were added to deeply-nested commands such as "op log".

Closes #3781
2024-10-06 09:45:27 +09:00
Yuya Nishihara
dc954517b9 cli: extract UnifiedDiff/DiffStatOptions structs
UnifiedDiffOptions could be wrapped by GitDiffOptions, but there are no Git-diff
specific options right now.
2024-10-06 09:45:27 +09:00
Yuya Nishihara
df40a09a5d cli: rename ColorWordsOptions to ColorWordsDiffOptions
I'm going to add an options struct for git diff, but GitOptions, UnifiedOptions,
StatOptions, etc. sound odd.
2024-10-06 09:45:27 +09:00
Yuya Nishihara
383cca4c4d diff: return matching hunk contents from all inputs
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.
2024-10-06 09:45:27 +09:00
Samuel Tardieu
3f2ef2ee04 style: add semicolon at the end of expressions used as statements 2024-10-04 22:29:13 +02:00
Yuya Nishihara
0c9bc37f10 diff: buffer color-words context lines
I'm testing simple conflicts diffs locally, and we'll probably need to handle
consecutive context hunks when we add some form of unmaterialized conflicts
diffs. Let's buffer context hunks (up to 1 right now.) The new code looks
simpler.
2024-09-19 20:01:51 +09:00
Yuya Nishihara
9f5ce934d2 diff: extract function that prints color-words diff lines
This block is relatively big, and will become even bigger if we add support
for unmaterialized conflicts diffs.
2024-09-19 20:01:51 +09:00
Yuya Nishihara
79e76c5dad diff: extract line number handling from show_color_words_diff_line()
This seems more consistent with !can_inline case, and we can remove extra
vec![] allocation from context line processing.
2024-09-19 20:01:51 +09:00
Yuya Nishihara
03487af192 diff: extract inter-diff helper
This isn't as primitive as show_diff() or show_patch(), but there are three
callers. Let's add a helper method to remove repo argument.
2024-09-07 09:09:27 +09:00
Yuya Nishihara
9c9e564dc4 conflicts: pass around conflict contents without materialization
We haven't decided how conflict diffs should be rendered, but whatever style
we'll choose, we'll need raw unmaterialized conflict contents.
2024-08-28 10:23:57 +09:00
Yuya Nishihara
73a8b13e4c conflicts: split materialized value to FileConflict and OtherConflict
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.
2024-08-28 10:23:57 +09:00
Yuya Nishihara
87fb169266 cleanup: remove redundant ::{self} from use declarations 2024-08-23 13:05:27 +09:00
Yuya Nishihara
4ffd3a3055 diff: pass tree diff stream in to inner show_*() functions as before 2024-08-23 10:29:12 +09:00
Yuya Nishihara
f5187fa063 copies: determine copy/rename operation by CopiesTreeDiffStream
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.
2024-08-23 10:29:12 +09:00
Yuya Nishihara
08262eb152 copies: extract (source, target) path pair to separate type
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.
2024-08-23 10:29:12 +09:00
Yuya Nishihara
43bf195314 merged_tree: rename diff entry field from "value" to "values"
It seems a slightly better, and aligns with the local variable name in
materialized_diff_stream().
2024-08-23 10:29:12 +09:00
Matt Kulukundis
8ead72e99f formatting only: switch to Item level import ganularity 2024-08-22 14:52:54 -04:00
Yuya Nishihara
352a4a0eea copies: filter rename source entries by CopiesTreeDiffStream 2024-08-22 20:17:19 +09:00
Yuya Nishihara
7684ab5994 diff: omit rename source entries from color-words diffs
Rename source entries will be handled by CopiesTreeDiffStream.
2024-08-22 20:17:19 +09:00
Yuya Nishihara
2cffcc9323 copies: provide source path mapping by CopyRecords
All for/has_source/target() combinations are added for API consistency.
2024-08-22 20:17:19 +09:00
Yuya Nishihara
5e356ffd24 diff: filter out uninteresting copy records by matcher
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.
2024-08-22 20:17:19 +09:00
Yuya Nishihara
d85e66bbb4 copies: turn add_records() into non-stream API, block_on_stream() by caller
This is simpler, and I think it's generally better to not spawn executor in
library code.
2024-08-22 20:17:19 +09:00
Yuya Nishihara
a83dadd5a9 diff: add option to display complex color-words diffs without inlining
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)
}
```
2024-08-21 17:48:52 +09:00
Yuya Nishihara
be9b7ed88f diff: pad last newline by show_color_words_diff_line()
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.
2024-08-21 17:48:52 +09:00
Yuya Nishihara
bb686c1459 diff: extract helper that prints color-words line numbers 2024-08-21 17:48:52 +09:00
Yuya Nishihara
6b2e191b2b diff: extract helpers that process word-level diffs without interleaving
These functions will be reused by non-inline version of color-words diffs.
2024-08-21 17:48:52 +09:00
Yuya Nishihara
decd913cea cli: introduce options struct specific to color-words diffs
I'll add a threshold whether to inline word changes.
2024-08-21 17:48:52 +09:00
Yuya Nishihara
5ff4e2d0a2 cli: pass &DiffFormatArgs in to default_diff_format()
Suppose we add -w/--ignore-all-space flag for example, it's probably better to
pass optional parameters by struct, not by separate arguments.
2024-08-21 17:48:52 +09:00
Yuya Nishihara
f76db7fa4b cli: flatten diff-args-to-formats translation
I'll add more parameters to color-words diffs, and the format variant will no
longer be trivially constructible.
2024-08-21 17:48:52 +09:00
Martin von Zweigbergk
721aa1238c copies: add a separate diff stream item type with copy info
The goal is to have the new item type know if it represent a copy, a
rename, a deleted rename source, or a regular copy-unrelated item.
2024-08-18 22:16:41 -07:00
Martin von Zweigbergk
70598498b0 merged_tree: provide separate version of diff_stream() with copy info
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.
2024-08-18 22:16:41 -07:00
Martin von Zweigbergk
fd9a236be5 copies: move CopyRecords to new copies module
Copy/rename handling is complicated. It seems worth having a module
for it. I'm going to add more content to it next.
2024-08-18 22:16:41 -07:00
Yuya Nishihara
1be955ea4e diff: simplify conditions whether to emit color-words context lines
This appears to fix redundant "    ..." line for empty diffs.
2024-08-18 12:40:07 +09:00
Yuya Nishihara
9beb57018a diff: split color-words diffing to line-based and refinement stages
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.
2024-08-18 12:40:07 +09:00
Yuya Nishihara
59745fb67f files: allow DiffLineIterator users to specify and retrieve line numbers
The added functions will be used in order to iterate middle hunks which don't
start from line_number = 1.
2024-08-18 12:40:07 +09:00
Yuya Nishihara
2be8e596e2 diff: extract Diff::by_word() function
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.
2024-08-18 12:40:07 +09:00
Matt Kulukundis
2f2e5fb72a copy-tracking: implement copy tracking for external tools 2024-08-16 07:48:43 -04:00
Yuya Nishihara
a973c7b0ea files: replace precomputed has_left/right_content flags with functions
I don't think the iteration cost would matter here, and it doesn't make sense
that has_left/right_content are cached whereas is_unmodified() isn't.
2024-08-16 09:30:30 +09:00
Yuya Nishihara
cca5277184 diff: clarify that DiffLine hunk doesn't have [left, right] diff pair
This will simplify users of line.hunks[] which I'm going to add.
2024-08-16 09:30:30 +09:00
Matt Kulukundis
95e8dd51eb copy-tracking: add support for diff --git 2024-08-15 11:03:39 -04:00
Yuya Nishihara
78c0128ec3 files: make DiffLineIterator accept generic DiffLine iterator
I'm thinking of adding some heuristics to render hunks containing lots of
small word changes differently, in a similar manner to the unified diffs. This
patch might help add some pre/post-processing at consumer.

files::diff() is inlined to caller to get around 'self borrowing.
2024-08-15 20:06:12 +09:00
Yuya Nishihara
8b222e4038 cli: propagate BackendError from tree diffs 2024-08-15 20:02:56 +09:00