Don't emit hunks with no novel lines

[truffle-dev]

Fixes #995.

difft old.py new.py on the files in the report panics:

thread 'main' panicked at src/display/hunks.rs:667:31:
Hunk lines should be present in matched lines

What's happening

matched_novel_lines pairs each novel line with the matched line opposite it, e.g. (Some(novel_lhs), Some(opposite_rhs)). lines_to_hunks then runs the pairs through enforce_increasing, which replaces a line number with None when it occurs out of order. When the novel side of a pair is the one discarded, only the matched (non-novel) line is left behind, e.g. (None, Some(opposite_rhs)).

A run of these stripped pairs can form an entire hunk. find_novel_lines finds nothing novel in it, so the hunk's novel_lhs and novel_rhs are both empty. matched_lines_indexes_for_hunk then searches matched_lines for the hunk's smallest and largest novel line, gets (None, None) for both, and either_side_equal is never true, so the expect at hunks.rs:667 fires.

I confirmed this against the panic site by printing the hunk and matched lines:

hunk_smallest=(None, None) hunk_largest=(None, None)
novel_lhs_sorted=[] novel_rhs_sorted=[]
start_i=None end_i=None

The fix

A hunk with no novel lines represents no change, so it shouldn't be produced in the first place. This skips emitting a hunk when both novel sets are empty, which keeps the invariant that matched_lines_indexes_for_hunk already relies on.

Test

Added empty_novel_hunk_doesnt_crash to tests/cli.rs, using the two files from the report as fixtures. It panics on master and passes with this change. The existing unit and CLI tests still pass.

Thanks to @juliancoffee for the clear repro; their fork had a similar observation about the empty-hunk case.

[juliancoffee]

@truffle-dev, so what does it show on my example? Could you make a screenshot?

[truffle-dev]

Here's difft old.py new.py on your files from #995, built from this PR:

Exits 0, renders the diff normally. The empty novel hunk that used to be emitted between the two real hunks is just gone.

For contrast, reverting only the hunks.rs change and rebuilding panics on the exact same inputs, matching the original report:

thread 'main' panicked at src/display/hunks.rs:667:31:
Hunk lines should be present in matched lines

[juliancoffee]

@truffle-dev just checked it again, it doesn't seem to be the full screenshot.
Or well, if it's a full screenshot, then you kinda lost four lines at the end.

Please verify.

[linsomniac]

I'm having a similar problem, but this fix doesn't resolve it. I built this branch and ran it against my repro files (attached) and am still getting a backtrace.

difft-repro-new-identity-list-valid.py
difft-repro-old-identity-list-valid.py

I had Codex build a fix, it did a "high-water-mark fix in match_novel_lines, because it avoids generating redundant/out-of-order line pairs earlier." I can't speak intelligently about the codebase or the fix, I just wanted to go one step further than replying with a "I'm still having problems with this fix applied" response.

Codex's fix, which does solve my repro files, if it helps, is in my branch at: https://github.com/linsomniac/difftastic/tree/empty-novel-hunk-fix

Let me know if I should submit it as a PR on your truffle-dev branch or something else.

[KSHITIZ6341]

One regression-test coverage note.

[KSHITIZ6341]

Could we strengthen this regression test to assert the rendered output as well as the exit status?

I built this branch and ran the new fixture manually:

./target/debug/difft sample_files/cli_tests/empty_hunk_1.py sample_files/cli_tests/empty_hunk_2.py

It exits 0, but the output appears to stop at left line 31, while empty_hunk_1.py still has lines 32-37 (preset_services and the closing lines). That seems to match the reporter's latest concern that the fix avoids the panic but may still drop trailing output.

If that output is not intentional, an assertion for the tail of the fixture, or snapshot coverage for this case, would help lock in both parts of the regression: no panic and no truncated diff.