martinvonz / jj

A Git-compatible VCS that is both simple and powerful

https://martinvonz.github.io/jj/

Apache License 2.0

8.32k stars 284 forks source link

Change bg-color of hunks including both whitespace and text changes #3942

Open Damianrogers51 opened 3 months ago

Damianrogers51 commented 3 months ago

Add extra partioning for diff hunks, splitting up each slice into a section for leading whitespace, inner text, and trailing whitespace.

Previously, if a diff hunk contained both whitespace changes (leading/trailing) and text changes, the whitespace changes would not be visible within the terminal as these whitespace changes would be lumped together with text changes resulting in generic removed and added labels. But now with this extra partioning we are able to add labels to each individual section, meaning now we can style diff hunks with both text and whitespace changes.

Checklist

If applicable:

[ ] I have updated CHANGELOG.md
[ ] I have updated the documentation (README.md, docs/, demos/)
[ ] I have updated the config schema (cli/src/config-schema.json)
[x] I have added tests to cover my changes

google-cla[bot] commented 3 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

yuja commented 3 months ago

My feeling is that it's too verbose to highlight all leading/trailing whitespace hunks. I would rather want to highlight small hunks (whether or not they're whitespace-only) in a similar way to Mercurial's word-diff and Git's diff-highlight.

Something like:

if a hunk spans whole line, leave it
label other word-level-ish hunks as highlight
highlight them with underline or low-contrast bg color

I think this can also be applied to git diffs.

Thoughts?

ilyagr commented 3 months ago

@Damianrogers51, are you able to fix the CLA?

How is this related to #3914? Are these alternatives to each other? Looking at the test, they seem to have a very similar effect.

There is also a related discussion in https://github.com/martinvonz/jj/pull/3914#discussion_r1645418484.