search

mookid / diffr

Yet another diff highlighting tool
MIT License
572 stars 22 forks source link

Ignore word-level diff on new or deleted lines #78

Open bgshacklett opened 3 years ago

bgshacklett commented 3 years ago

Can word-level diff be ignored for lines which are completely new or completely removed from a given diff? With the default configuration, added/removed lines are highlighted in there entirety.

mookid commented 2 years ago

sure! I think that it would be an easy enough change to make. Maybe defining a new style for added/removed entire lines is what you want.

I don't have time to work on it myself, but I would be glad to review and merge the change it it were written :)

Hi-Angel commented 1 year ago

FTR: were it added, please make it either optional or at least configurable. I think the current behavior of highlighting is better because it is unambiguous.

The goal of inspecting a diff is to figure out what changes were done. As diff format may only show that in terms of complete lines added/removed even even for a one-character change, we try to improve on that by using diffr to highlight the actual new changes. IOW diffr allows us to see what text was actually added/removed, by highlighting it.

Now, suppose a diff has a new line added, and (per this report) we didn't highlight it. That will make it look same way as lines that were always there. You'll easily distinguish the cases if a diff has no lines deleted. However, for more involved diff, like with multiple deletions/additions, such highlight style may confuse you, because to see whether it's a completely new line or one with a whitespace changed (arguably, whitespace changes should be highlighted though) you have to start studying the diff on per-line basis, which breaks the entire point of diffr automating such work.

I'm not against such feature, but it looks more like a fancy, because its ambiguity makes it impractical. Fancies don't have to be practical, they're rather made for visual pleasure, but before making one a default behavior practicality should be considered.

mookid commented 1 year ago

I agree that if added, it should be behind a feature flag to opt-in for that behaviour.