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

Warning highlight for trailing whitespace #137

Closed
herbygillot opened this issue Apr 28, 2020 · 10 comments · Fixed by #1037
Closed

Warning highlight for trailing whitespace #137

herbygillot opened this issue Apr 28, 2020 · 10 comments · Fixed by #1037

Comments

@herbygillot
Copy link

herbygillot commented Apr 28, 2020

The standard pager for git will highlight additions of lines whose only content are spaces or tabs in red. The same for any trailing whitespace. Is it possible to have similar behavior in delta?

Thanks in advance.

@dandavison
Copy link
Owner

Hi @herbygillot, this is a good call, thanks!

Notes to help implementing a fix / unit tests for this:

Here's an example

diff --git a/z.js b/z.js
index cf70fdb..c528683 100644
--- a/z.js
+++ b/z.js
@@ -3,5 +3,6 @@ function test() {
 }
 
 
+   
 test();
 console.log("this isn't in the hunk!")

which git --no-pager diff displays as

image

but with delta this is

image

@dandavison dandavison changed the title Highlight empty lines? Warning highlight for trailing whitespace Apr 28, 2020
@aureliojargas
Copy link

By reading the documentation, I was expecting that forcing something like --whitespace-error-style 'red reverse' would give me that behaviour.

--whitespace-error-style
Style for whitespace errors. Defaults to color.diff.whitespace if that is set in git config, or else
'magenta reverse' [default: auto auto]

Unfortunately, it also does not work.

@dandavison
Copy link
Owner

Hi @aureliojargas, let me give a quick response where I may be missing something:

I actually thought that this was fixed and that I'd forgotten to close the issue. Which delta version are you using? With 0.3.0 this is what I get using the diff pasted above. Is this what you want the output to be?

image

@aureliojargas
Copy link

Hi @dandavison, thanks for the super quick answer!

From what I can tell, it seems it is working correctly for lines containing only whitespace (as in your test), but not for lines with content and trailing whitespace. I'm using delta 0.3.0 in macOS 10.13.6.

Considering a text file foo that has a single x line, I've added two spaces after the x and a new line with another two spaces.

echo x > foo
git add foo
echo -e "x  \n  "  > foo
git diff foo > foo.diff

Note that the plain git diff marks both cases as red, while delta does it only in the empty line:

image

Please tell me if you need additional tests or information from my side, I'm glad to help.

@dandavison
Copy link
Owner

Thanks @aureliojargas that’s very helpful. I should be able to get a fix in shortly.

@ryuheechul
Copy link

It's been a week that I've been enjoying delta and I just arrived to this issue too 😅 hence curious about updates on the issue as well.

wescande added a commit to wescande/delta that referenced this issue Apr 6, 2022
wescande added a commit to wescande/delta that referenced this issue Apr 6, 2022
wescande added a commit to wescande/delta that referenced this issue Apr 6, 2022
wescande added a commit to wescande/delta that referenced this issue May 23, 2022
@dvoytik
Copy link

dvoytik commented Mar 7, 2023

Yeah, this is a really missing feature from standard git diff output to highlight trailing white-spaces.
Please let me know if you need help and would accept a patch fixing this.

@dandavison
Copy link
Owner

@dvoytik sorry I missed your offer of help here. There's discussion in #1037 and @amtoine has offered to work on this. What I'm hoping to do is first get some tests that demonstrate exactly what we're trying to fix. Once we have those tests let's check that we're all in agreement about what the behavior should be.

wescande added a commit to wescande/delta that referenced this issue May 6, 2023
wescande added a commit to wescande/delta that referenced this issue May 6, 2023
wescande added a commit to wescande/delta that referenced this issue May 7, 2023
wescande added a commit to wescande/delta that referenced this issue May 7, 2023
wescande added a commit to wescande/delta that referenced this issue May 7, 2023
@dandavison
Copy link
Owner

@dvoytik @aureliojargas @herbygillot if you are able to test the branch #1037 to see whether it behaves as you would want w.r.t. whitespace changes that would be fantastic. The work in that branch has been done by @wescande.

wescande added a commit to wescande/delta that referenced this issue May 8, 2023
wescande added a commit to wescande/delta that referenced this issue May 17, 2023
@aureliojargas
Copy link

@dandavison I'm sorry for the late response, but I just installed delta 0.16.5 and I confirm this is now working. Thank you!

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 a pull request may close this issue.

5 participants