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

🐛 Empty lines highlight plus-marker as whitespace-error #388

Closed
Nemo157 opened this issue Nov 13, 2020 · 8 comments · Fixed by #425
Closed

🐛 Empty lines highlight plus-marker as whitespace-error #388

Nemo157 opened this issue Nov 13, 2020 · 8 comments · Fixed by #425

Comments

@Nemo157
Copy link

Nemo157 commented Nov 13, 2020

With a test file:

No Spaces

Spaces
  

(Note that there are spaces on the last line)

Running

delta --no-gitconfig --keep-plus-minus-markers <(echo -n) test-file

highlights the plus-marker on the empty line as a whitespace error

temp

@Kr1ss-XD
Copy link
Contributor

Kr1ss-XD commented Nov 14, 2020

I guess it is a whitespace error diff, since newlines are whitespace, no ?

@Nemo157
Copy link
Author

Nemo157 commented Nov 14, 2020

It's very distracting having it highlighted in the middle of a block, and it's inconsistent with what git itself highlights as whitespace errors:

image

image

@Kr1ss-XD
Copy link
Contributor

Kr1ss-XD commented Nov 14, 2020

It's very distracting having it highlighted in the middle of a block

Agreed, though one could also argue that it might be advantageous to have whitespace diffs highlighted esp. when working with languages as python where they are crucial

it's inconsistent with what git itself highlights as whitespace errors

That depends on the configuration. Not sure though what the defaults are, to be honest, but IIRC git does highlight such a diff without any config.

EDIT/ This was after removing all git config files :

Anyways, I agree it would be quite handy to have it configurable in delta, too.

@dandavison
Copy link
Owner

Hi @Nemo157, thanks for this!

@Kr1ss-XD what @Nemo157 is pointing out is that completely normal, not-at-all-an-error blank lines are being styled with an error background color when using keep-plus-minus-markers. I would say this is definitely a bug!

image

@Kr1ss-XD
Copy link
Contributor

Oh, I see. I've missed the point then I guess.

@ericbn
Copy link

ericbn commented Nov 20, 2020

I think this is part of a bigger issue, as I reported in #345.

dandavison added a commit that referenced this issue Dec 5, 2020
* Add failing test of added empty line with keep-plus-minus-markers

* Fix whitespace error flagging with keep-plus-minus-markers

Fixes #388

* Possibly more efficient implementation

* Fix clippy

* Fix clippy II

* Increase tarpaulin timeout

* Increase tarpaulin timeout

* Add failing test of line with one non-whitespace character

* Fix test of line with one non-whitespace character

* Revert tarpaulin changes
@dandavison
Copy link
Owner

@Nemo157 thanks for this, it is fixed in master (unreleased).

@ericbn thanks for pointing out the link to #345. The fix that has been merged for this branch fixes a subset of the bugs you demonstrated in #345, but not all.

@dandavison
Copy link
Owner

This fix is released now (v0.4.5).

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.

4 participants