95 Commits

Author SHA1 Message Date
Yuya Nishihara
79fb219159 files: make DiffLineIterator omit blank right line following matching+left
I was wondering whether this is presentation issue or not, and I think it is a
matter of DiffLineIterator. For matching hunks, DiffLineIterator flushes the
current_line buffer and bumps the line numbers for the next line. This should
guarantee that there are no blank DiffLine to be queued. However, for different
hunks, only the left line number can be bumped in the first loop, so there may
be an empty-looking hunk having the same right line number.

Closes #6471
2025-05-09 00:49:10 +00:00
Yuya Nishihara
39295238ac tests: demonstrate redundant right blank lines in color-words diffs 2025-05-09 00:49:10 +00:00
Martin von Zweigbergk
acd824269f git: write change-id header by default
We haven't had any reports of problems from people who opted in. Since
it's early in the release cycle now, let's now test it on everyone who
builds from head, so we get almost a month of testing from those
people before it's enabled by default in a released version.

This impacts lots of test cases because the change-id header is added
to the Git commit. Most are uninteresting. `test_git_fetch` now sees
some divergent changes where it used to see only divergent bookmarks,
which makes sense.
2025-05-08 23:05:38 +00:00
Yuya Nishihara
0e17c0e17d copies: ignore existing tree when testing copy/rename operation type
In this context, a tree value should be considered absent.
2025-04-22 00:44:13 +00:00
Yuya Nishihara
b6a861b529 git_backend: do not include renamed trees in copy records
We could instead make CopiesTreeDiffStream reject bad copy records, but it's
pretty much wrong to construct a FileId for tree.

Fixes #6390
2025-04-22 00:44:13 +00:00
Yuya Nishihara
b63ab33b10 diff: add option to render color-words diffs without materializing conflicts
The original idea was to flatten left/right conflict trees and pair up adjacent
negative/positive terms. For example, diff(A, B-C+D) could be rendered as
diff(A, B) and diff(C, D). The problem of this formalization is that one of the
diff pairs is often empty (because e.g. A=B), so the context is fully omitted.
The resulting diff(C, D) doesn't provide any notion why the hunk is conflicted,
and how it is different from A.

Instead, this patch implements diffs in which each left/right pair is compared.
In the example above, the left terms are padded, and the diffs are rendered as
diff(A, B), diff(-A, -C), diff(A, D). This appears to be working reasonably well
so long as either side is resolved or both sides have the same numbers of terms.

Closes #4062
2025-04-08 09:12:39 +00:00
Yuya Nishihara
8af238f68a diff: test identical contents early when rendering color-words diffs
This helps extract hunk rendering function for non-materialized color-words
diffs. In conflict hunk, an identical diff pair will be omitted with "..."
marker.

This patch introduces a subtle behavior change when "ignore whitespace" options
are used. Before, "..."  wouldn't be printed if contents differ only in
whitespace. I don't think the new behavior is bad because the file header says
"Modified regular file" in that case.
2025-04-08 09:12:39 +00:00
Yuya Nishihara
4ae00a6cb4 tests: add samples of conflict diff rendering 2025-04-08 09:12:39 +00:00
Yuya Nishihara
b000c94a7a tests: add work_dir.dir(path), make create_dir*() return helper for sub dir
I found this pattern is common.
2025-03-18 01:25:08 +00:00
Ilya Grigoriev
acaedc3382 cleanup: enable unused_trait_names clippy lint and run clippy --fix 2025-03-16 00:35:56 +00:00
Yuya Nishihara
31620a11ae tests: port test_diff_command.rs to TestWorkDir API 2025-03-15 02:41:22 +00:00
Yuya Nishihara
fc84c639f7 tests: configure fake editor early in test_diff*_command.rs
This works around borrowing issue of test_env.
2025-03-15 02:41:22 +00:00
Martin von Zweigbergk
dc7216d73a cli: diff: support multiple revisions to -r 2025-03-11 06:27:51 +00:00
Ilya Grigoriev
5eae2d92a0 tests: run insta --force-update-snapshots
This is a replacement for #5558.

Thanks to @yuja 's https://github.com/mitsuhiko/insta/pull/722, this is
now easy to generate.
2025-03-06 21:35:08 +00:00
Yuya Nishihara
7fa8420908 tests: resolve directory path to run_jj_in() relative to env_root
Suppose we'll add test_env.init_workspace(path) or something, this will probably
make sense.
2025-02-25 15:40:14 +00:00
Yuya Nishihara
ad6985fd9c tests: migrate unusual patterns of jj_cmd_ok() to run_jj_in()
These callers are manually ported to run_jj_in().
2025-02-24 15:39:11 +00:00
Yuya Nishihara
804d175fd9 tests: migrate non-snapshot users of jj_cmd_ok() to run_jj_in().success()
These callers were mostly substituted mechanically.
2025-02-24 15:39:11 +00:00
Yuya Nishihara
3193513049 tests: migrate snapshot users of jj_cmd_ok() to run_jj_in()
These callers were mostly substituted mechanically, then fixed up formatting.
2025-02-24 15:39:11 +00:00
Yuya Nishihara
cdcc777730 tests: migrate snapshot users of jj_cmd_success() to run_jj_in()
These callers were mostly substituted mechanically, then fixed up minor
formatting and name errors.
2025-02-23 13:34:17 +00:00
Yuya Nishihara
317993894b tests: migrate unusual users of jj_cmd_success() to run_jj_in/with()
These callers are manually ported to run_jj_in(). assert_eq!() is changed to
insta::assert_snapshot!() where possible.
2025-02-23 13:34:17 +00:00
Yuya Nishihara
a326b6e792 tests: migrate jj_cmd_success() output wrappers to run_jj_in() 2025-02-23 13:34:17 +00:00
Yuya Nishihara
b79b29c288 tests: migrate callers of jj_cmd_cli_error() to run_jj_in() 2025-02-23 03:06:59 +00:00
Yuya Nishihara
f1cd3a4e53 tests: migrate some jj_cmd_success() output wrappers to run_jj_in() 2025-02-22 01:23:04 +00:00
Yuya Nishihara
e23290d95d tests: migrate users of strip_last_line() to run_jj_in() function 2025-02-21 01:49:41 +00:00
Yuya Nishihara
cf7f1dfc88 tests: migrate users of normalize_exit_status() to run_jj_in() function 2025-02-21 01:49:41 +00:00
Yuya Nishihara
630036eeb0 tests: add [EOF] marker to command output when displaying
It's important to test that command output is (usually) terminated with newline,
but insta::assert_snapshot!() is lax about that.
2025-02-19 02:31:59 +00:00
Yuya Nishihara
a54165230a tests: add CommandOutputString wrapper
I'm going to add "[EOF]" marker to test that command output is terminated by
newline char. This patch ensures that callers who expect a raw output string
would never be affected by any normalization passes.

Some common normalization functions are extracted as CommandOutputString
methods.
2025-02-19 02:31:59 +00:00
Bryce Berger
b1b2c62c3e diff: add merge-tools.*.diff-expected-exit-codes
Certain tools (`diff`, `delta`) exit with code 1 to indicate there was
a difference. This allows selectively suppressing the "Tool exited with
... status" warning from jj when generating a diff.

example:
```toml
[merge.tools.delta]
diff-expected-exit-codes = [0, 1]
```
2025-01-08 08:03:59 +00:00
Yuya Nishihara
d7e0ab6119 tests: use toml_edit to escape editor path, set ui.editor instead of $EDITOR
Most callers don't need the $EDITOR variable.
2024-12-20 19:28:01 +09:00
Martin von Zweigbergk
a414441dd6 cli: refer to fileset argument using FILESETS in synopsis
This should help clarify that the arguments are not just simple paths
(assuming `ui.allow-filesets` is not disabled).
2024-12-18 10:13:44 -08:00
Yuya Nishihara
abf48576ea cli: replace --config-toml=TOML with --config=NAME=VALUE 2024-12-17 20:12:12 +09:00
Yuya Nishihara
8c4ba4a144 diff: fix unified diff to emit one-lower start line number for empty hunks
Both Mercurial and Git (xdiff) have a special case for empty hunks.

https://repo.mercurial-scm.org/hg/rev/2b1ec74c961f

I also changed the internal line numbers to start from 0 so we wouldn't have
to think about whether "N - 1" would underflow.

Fixes #5049
2024-12-09 09:52:07 +09:00
Vamsi Avula
0794f87324 templates: align attributes in builtin_log_detailed
Change-Id: Id00000003feb0508bf7f08e0a0275ba7f7695b70
2024-12-08 15:17:43 +05:30
Scott Taylor
7f57866332 merge_tools: allow setting conflict marker style per-tool
I left the "merge-tool-edits-conflict-markers" option unchanged,
since removing it would likely break some existing configurations. It
also seems like it could be useful to have a merge tool use the default
conflict markers instead of requiring the conflict marker style to
always be set for the merge tool (e.g. if a merge tool allows the user
to manually edit the conflicts).
2024-11-26 09:05:17 -06:00
Dave Townsend
891fa88be0 diff: Allow setting the default level of context in config. 2024-10-25 17:56:56 +01:00
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
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
Samuel Tardieu
46e2723464 style: inline variables into format strings 2024-10-04 22:29:13 +02:00
Yuya Nishihara
5af906d924 cli: change default inline threshold of color-words diffs
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.
2024-08-28 10:33:33 +09:00
Matt Kulukundis
8ead72e99f formatting only: switch to Item level import ganularity 2024-08-22 14:52:54 -04: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
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
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
be35ab164c tests: restore snapshots of line/word-oriented diff hunks
test_diff_basic() is now testing file-level changes such as renames.
2024-08-16 22:16:36 +09:00
Matt Kulukundis
2f2e5fb72a copy-tracking: implement copy tracking for external tools 2024-08-16 07:48:43 -04:00
Matt Kulukundis
95e8dd51eb copy-tracking: add support for diff --git 2024-08-15 11:03:39 -04:00
Matt Kulukundis
0b179dcbde copy-tracking: implement copy-tracking for --types 2024-08-14 20:48:43 -04:00
Matt Kulukundis
ec99a17ae8 copy-tracking: improve --summary and add --stat
- add support for copy tracking to `diff --stat`
- switch `--summary` to match git's output more closely
- rework `show_diff_summary` signature to be more consistent
2024-08-13 21:37:45 -04:00
Matt Kulukundis
5911e5c9b2 copy-tracking: Add copy tracking as a post iteration step
- force each diff command to explicitly enable copy tracking
- enable copy tracking in diff_summary
- post-process for diff iterator
- post-process for diff stream
- update changelog
2024-08-11 17:01:45 -04:00
Yuya Nishihara
8fec7500c3 cli: enable fileset by default
I've tested it for months and found no problems.
2024-07-24 10:49:46 +09:00