🚀 `ws-error-highlight`

[laniakea64]

So glad to have come across Delta. Thank you for making this helpful tool that improves accessibility of diffs!

Currently Delta only highlights whitespace errors in added/plus lines. For some diffs, fully understanding them involves knowing about whitespace errors in removed/minus lines as well, and on rare occasions it can also be useful in context lines. But as far as I can tell, Delta has no option to specifically highlight whitespace errors in any additional places other than added lines.

git diff has the option --ws-error-highlight for this. Tried passing git this option when piping to Delta, but this doesn't work, looks like Delta removes this highlighting along with the expected removal of other color codes.

Could Delta please have option to highlight whitespace errors in removed lines, and possibly also in context lines?

Thanks :slightly_smiling_face:

[phillipwood]

I agree it would be really nice for delta to support this. There is some support for trailing whitespace at the end of lines, but not for other whitespace attributes like blank-at-eof or space-before-tab. One way that would avoid having to re-implement the detection and config parsing in delta would be to parse the error highlights in the output from git, remembering their offset from the beginning of the hunk and using that information to render them when delta paints the processed lines. I'm not sure how easy that would be to implement though.

[dandavison]

This is just a thought, I sort of think it isn't really the right way forward:

The way that delta support git's "color-moved" feature is that, if we see that git has emitted a color other than the standard red and green (line_has_style_other_than), then we just emit the line raw. In other words, delta essentially gives up and supports color-moved in a rather hacky way.

So we could take the same approach here. In fact, one might think it would work already. The reason it doesn't is that line_has_style_other_than() is implemented in a rather odd way, partly out of concern for efficiency I think:

https://github.com/dandavison/delta/blob/4b80dfb04c4a6e2dc841408345a2f72d441cf161/src/style.rs#L369

Here's a proof-of-concept branch that reimplements line_has_style_other_than(): https://github.com/dandavison/delta/compare/1590-ws-error-highlight-workaround

with the result that git show --ws-error-highlight=all does work on that branch:

That said, ideally, delta would provide its own take on rendering both color-moved and ws-error-highlight, instead of just passing through git's raw output. On the plus side, delta does have pretty convenient (and efficient) primitives for parsing ANSI color escapes into style structs and working with those structs (e.g. see the diff in the branch linked above), so it might well be possible to do the more "ideal" version now.

I don't have an opinion on the right way forward yet, just throwing out an idea and pointing to some code; interested to know what others think.

As an aside GIT_DEFAULT_MINUS_STYLE and GIT_DEFAULT_PLUS_STYLE (i.e. the default "red" and "green" emitted by git) are currently hard-coded; they should be taken dynamically from user git config.

[laniakea64]

Thanks dandavison for looking into this :slightly_smiling_face:

Here's a proof-of-concept branch that reimplements line_has_style_other_than(): https://github.com/dandavison/delta/compare/1590-ws-error-highlight-workaround

with the result that git show --ws-error-highlight=all does work on that branch:

Just tried this branch, but am getting a very different result. Testing it with the last two commits on that branch, with passing --ws-error-highlight to git: lines with whitespace errors are highlighted red not just on the whitespace error, but all the way to the end of the line in the terminal? :confused:

In case it's related, I don't keep Delta configuration in gitconfig, instead I run Delta using a wrapper script that invokes git diff or git show with --color=always option (followed by whatever else is specified on the command line), piped to Delta invoked with these options:

  --keep-plus-minus-markers \
  --max-line-length 9999 \
  --syntax-theme "$syn" \
  --minus-style 'red auto' \
  --minus-emph-style '#ff8877 bold auto' \
  --plus-style 'green auto' \
  --plus-emph-style 'brightgreen bold auto' \

where $syn is none unless I explicitly specify otherwise.

That said, ideally, delta would provide its own take on rendering both color-moved and ws-error-highlight, instead of just passing through git's raw output.

+1. Mainly because this would avoid losing word diff highlighting, which is the reason I looked into Delta in the first place. Additionally, it would allow more control over how to style highlighted whitespace errors.