fix: don't prepend the cwd to paths from git diff --no-index

[VXNCXNX]

Half of #1928: this fixes delta FILE1 FILE2, where delta runs git itself. The
piped form, git diff --no-index ... | delta, is still wrong for the reason in
"What I deliberately did not do" below, so the issue should stay open when this
merges.

The problem

Outside a repo, hyperlinks get the cwd prepended to a path that already carries it:

$ cd /tmp
$ delta /tmp/etcd/a.txt /tmp/etcd/b.txt --hyperlinks
file:///tmp/tmp/etcd/a.txt      <- doubled

The cause is on git's side, and it's easy to miss: given absolute path arguments, git diff --no-index strips the leading separator, so it emits tmp/etcd/a.txt. That looks exactly like a cwd-relative path, and absolute_path duly joins the cwd onto it.

Worth noting the issue reproduces with absolute arguments only. With relative ones (git diff --no-index a.txt b.txt) git emits a/a.txt, which really is cwd-relative, and delta was already correct.

The fix

When delta itself ran git diff --no-index — i.e. it was invoked as delta FILE1 FILE2 — it knows exactly which two files are being diffed, because they're already in config.minus_file / plus_file. If re-attaching the leading separator to an emitted path reproduces one of those two, that's the intended absolute path.

That's the whole rule. No filesystem probing, no guessing from the path shape.

What I deliberately did not do

My first version also tried to catch the case where the user ran git diff --no-index and piped into delta, by checking CallingProcess::GitDiff plus "the root-relative path exists and the cwd-relative one doesn't". A review pass caught that this regresses ordinary diffs, and I reproduced it:

# a repo containing a tracked file at etc/passwd, deleted in the worktree
$ git diff | delta --hyperlinks
before:  file:///tmp/delrepo3/etc/passwd     <- correct
with heuristic:  file:///etc/passwd          <- wrong file entirely

Any repo path that shadows a real root path (etc/…, usr/bin/…, bin/…, home/…) hits it, since CallingProcess::GitDiff matches every git diff, not just --no-index. It also added a stat() per hyperlinked line, which nothing else in path.rs does.

So the heuristic is gone. The consequence is that git diff --no-index ABS ABS | delta is still wrong — there's no signal available in that pipeline to distinguish it, since delta's sibling-process detection doesn't find git there. Fixing that means changing process detection, which is a different patch. This PR fixes the delta FILE1 FILE2 form.

Verification

delta /tmp/dtest2/a.txt /tmp/dtest2/b.txt   ->  file:///tmp/dtest2/a.txt     (was /tmp/tmp/...)
in-repo git diff, from repo root and a subdir  ->  byte-identical to baseline
the deleted-etc/passwd case above              ->  byte-identical to baseline

Two tests added, including one asserting that a path which isn't one of the diffed files still resolves against the cwd.

cargo test:   440 passed; 0 failed; 8 ignored   (438 before)
cargo fmt --check: clean
cargo clippy --all-targets: no warnings in the changed file

[xhon-pelushi]

Built this and exercised both invocation paths rather than just reading the diff. The fix works for the case it targets, and gating on minus_file/plus_file equality instead of guessing from the path shape is the right instinct — that is the part that makes it safe.

Confirmed working, delta running git diff --no-index itself:

$ delta --hyperlinks --hyperlinks-file-link-format 'LINK:{path}' .../dtest/a.txt .../dtest/b.txt
LINK:/home/xadmin/.../work/dtest/a.txt      # correct
LINK:/home/xadmin/.../work/dtest/b.txt

Both new tests pass, and the second one (test_absolute_path_of_repo_relative_path) is a good addition — it pins down that ordinary repo-relative paths are still joined to the cwd, which is the thing a change like this could plausibly break.

The piped form is still wrong

Worth being explicit that this closes half of #1928, because the other half looks identical to a user. Piping the same diff in produces a doubled path:

$ git diff --no-index .../dtest/a.txt .../dtest/b.txt | delta --hyperlinks --hyperlinks-file-link-format 'LINK:{path}'
LINK:/home/xadmin/.../work/dtest/home/xadmin/.../work/dtest/a.txt

which is the cwd prepended to the root-relative path, exactly the original bug. The input git produces is:

diff --git a/home/xadmin/.../work/dtest/a.txt b/home/xadmin/.../work/dtest/b.txt
--- a/home/xadmin/.../work/dtest/a.txt

minus_file/plus_file are unset in that path, so root_relative_path() returns None and it falls through to the cwd join.

I am not suggesting you widen the guard here — your comment already explains why guessing from the path alone is unsafe, and I agree that rewriting paths in ordinary diffs would be far worse than leaving this case broken. But since git diff --no-index ... | delta is a normal way to use delta, it is probably worth either saying so in the PR description or leaving #1928 open when this merges, so the remaining half does not get lost.

For what it is worth, the one signal available in the piped case that is not pure guesswork is existence: root-relative interpretation is only plausible when / + path exists and cwd + path does not. Still a heuristic, so entirely reasonable to decline.

Small nit: the guard is acquired before it can be used

let caller = calling_process();
if let Some(path) = root_relative_path(relative_path, config) {
    return Some(normalize_path(path));
}

In non-test builds calling_process() returns a MutexGuard<'static, CallingProcess> and wait_whiles on a condvar until caller determination finishes. Hoisting it above the early return means the root_relative_path fast path now takes that lock, and potentially blocks on it, for a value it never reads — and holds the guard across the root_relative_path call. Moving the binding below the early return keeps the previous behaviour:

if let Some(path) = root_relative_path(relative_path, config) {
    return Some(normalize_path(path));
}
let caller = calling_process();

Harmless as written, just unnecessary.

Tested on bef9b30, rustc 1.97.1, linux x86_64.

[VXNCXNX]

Thanks for building it and testing both paths, that is more than I had a right to expect.

Fixed the lock nit in d3b2a5c. You are right about why it was there: I hoisted the binding to get past the borrow checker and did not think about what calling_process() actually does in a non-test build. Moving it below the early return works fine and keeps the fast path off the mutex.

On the piped case, I reproduced it here and get the same doubled path you do. The two files really are unknown in that path: minus_file/plus_file are only set when delta ran git itself, so there is nothing left to key on except the path text.

I looked at the existence signal. It breaks when both interpretations exist, which is not exotic: from a directory that happens to contain a tmp/dtest/ subtree, both /tmp/dtest/a.txt and $cwd/tmp/dtest/a.txt resolve, and the heuristic silently picks one. Since the failure mode is a wrong link rather than a missing one, I would rather leave the piped case alone than guess.

So this is half of #1928 and the issue should stay open when it merges. I have updated the PR description to say that instead of "Fixes #1928".

[xhon-pelushi]

Re-checked d3b2a5c. cargo test utils::path:: -- --nocapture passes, and the updated code now returns from root_relative_path() before calling calling_process(), so the fast path no longer takes that mutex guard.

I also re-ran the real hyperlink cases. Direct invocation (delta /tmp/.../a.txt /tmp/.../b.txt) emits root-correct links for the absolute paths. The piped form still emits the cwd-doubled path, as expected from the missing minus_file/plus_file signal, and the PR description now leaves #1928 open for that half. Looks good for the scope this PR now claims.