Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix --word-diff / --color-words handling #833

Merged
merged 7 commits into from
Dec 12, 2021
Merged

Conversation

dandavison
Copy link
Owner

Fixes #829 WIP

#807 was a pretty bad attempt at handling this correctly: while the colored content from git was emitted raw, in so doing they were not recognized as hunk lines and we lost line numbers and hunk headers.

@dandavison dandavison mentioned this pull request Dec 7, 2021
@dandavison dandavison force-pushed the 829-color-words-2 branch 2 times, most recently from 97f9da8 to f522c3d Compare December 8, 2021 15:06
Base automatically changed from 829-color-words-refactor to master December 10, 2021 01:46
@dandavison
Copy link
Owner Author

@mrjoel this branch fixes --word-diff / --color-words support, but, there's no support for line-numbers (or side-by-side) -- both will be silently ignored if --word-diff / --color-words input is detected.

For line-numbers the reason is that it would be a bit more work to implement: to calculate line numbers in the "before" and "after" state, delta normally relies on the - and + prefixes to indicate removed and added lines, but with --word-diff we don't have those and we'd have to detect wholly added/removed lines from the ANSI color sequences alone. In contrast, the implementation of --word-diff handling in this branch is based on processing all lines in the same way that unchanged (context) lines are usually processed.

For side-by-side the reason is that (a) one usually uses this with line numbers and (b) I don't think it makes much sense with --word-diff output since lines with both additions and removals will be the same in the left and right panels.

The previous implementation (5895cfa) was incorrect.

Fixes #829
We currently have no way of computing correct line numbers: it seems that we would need to identify
wholly added and removed lines from the ANSI color sequences. side-by-side is (I imagine) almost
always used with line numbers, and it doesn't make much sense for word-diff anyway.
@dandavison
Copy link
Owner Author

dandavison commented Dec 12, 2021

@th1000s de37c99 adds a builder method DeltaTest::with_calling_process(command: &str) allowing your FakeParentArgs functionality to be accessed easily in DeltaTest. Hopefully you agree with the spirit of the commit if not the exact implementation :)

@dandavison dandavison merged commit f88f0a4 into master Dec 12, 2021
@dandavison dandavison deleted the 829-color-words-2 branch December 12, 2021 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 --color-words output doesn't show hunk headers or line numbers
1 participant