rust-analyzer / expect-test

Minimalistic snapshot testing for Rust.
Apache License 2.0
248 stars 22 forks source link

Use foreground colors instead of background colors to highlight diffs #36

Closed knrafto closed 1 year ago

knrafto commented 1 year ago

I think foreground colors are more "standard", and the background colors make things unreadable in my color scheme.

Before:

Screenshot 2023-02-06 at 2 33 31 PM

After:

Screenshot 2023-02-06 at 2 38 40 PM
bjorn3 commented 1 year ago

Using foreground color would hide whitespace changes, which may be confusing in some cases. If that is a strong enough argument to keep changing the background color, I guess not.

lnicola commented 1 year ago

This bothered me a lot, and it looks much better now. But it might be possible to keep the background color on the whitespace characters, how does that sound?

knrafto commented 1 year ago

I'd rather keep this PR as a small tweak rather than making too many changes to the diffing algorithm (e.g. by detecting significant whitespace changes). How about showing the changes underlined like this?

Screenshot 2023-02-10 at 2 58 39 PM
knrafto commented 1 year ago

Hey, any thoughts on whether this PR should be merged?

knrafto commented 1 year ago

Thanks! 😊