changedetection.io icon indicating copy to clipboard operation
changedetection.io copied to clipboard

[feature] Diff is highlighting whitespace changes even if "Ignore whitespace" option is ticked

Open jbd7 opened this issue 2 years ago • 5 comments

Setup: v0.40.0.4 self hosted with Docker on Ubuntu 22. In "Global filters" of the settings, I have ticked " Ignore whitespace Ignore whitespace, tabs and new-lines/line-feeds when considering if a change was detected."

Use case: I monitor webpages and get alerted. When I receive an alert, I click on the Diff link and quickly scan the Diff page to see what has changed.

Unexpected behavior: Since I have the "Ignore whitespace" option ticked, I do not expect to see them colored on the Diff page. However, they are. See screenshot.

Why it is bothering: It's not critical. It makes the Diff feature unusable for some pages, because I would scroll the page until I see color. But since many "false positive" changes are colored red and green, I am not able to quickly notice the real changes.

image

jbd7 avatar Feb 01 '23 16:02 jbd7

Hi, I filed this one in November 2022 but I do not know how to fix it https://github.com/kpdecker/jsdiff/issues/389

I'm open to a PR if you want to submit something, otherwise, probably this doesnt get fixed soon

If anyone can help with that library, I would be very grateful (and i'de probably find a way to pay you for your help too)

dgtlmoon avatar Feb 01 '23 16:02 dgtlmoon

I'm open to a PR if you want to submit something, otherwise, probably this doesnt get fixed soon

Sorry, I'll probably break more things than anything else :)

This issue and https://github.com/kpdecker/jsdiff/issues/389 are different though. If I understand correctly, you want jsdiff to ignore whitespace in chosing when to highlight the lines, but still output whitespace. That'd make the diff more readable indeed! I think the main purpose of a Diff is to highlight relevant changes, before maintaining the layout, so I can live with it.

For this issue, which focuses on the highlighting, maybe I found a workaround:

Currently: even if Ignore whitespace is selected in Global Filters, opening a https://cd.io/diff/abcdef-123456/ link opens it without the "Ignore Whitespace" tickbox ticked (see screenshot). Proposed: Pages https://cd.io/diff/abcdef-123456/ load by default with the "Ignore Whitespace" tickbox set to the status of "Ignore whitespace" in Global Filters at /settings#filters

What do you think?

image

jbd7 avatar Feb 01 '23 17:02 jbd7

if you select Words it's a lot easier to see, does that help?

dgtlmoon avatar Feb 11 '23 11:02 dgtlmoon

if you select Words it's a lot easier to see, does that help?

Agreed, showing "Words" instead of "Lines" is an improvement compared to the current default. I'd still think Lines+IgnoreWhitespaceTicked is better, since:

  • Words+IgnoreWhitespaceUnticked can make it easy to skip through small changes (one letter or one word in a long line), while it's hard to scroll past a whole line highlighted.
  • Words instead of Lines, without IgnoreWhitespace ticked, still highlights single spaces.

jbd7 avatar Feb 11 '23 18:02 jbd7

+1 for jbd7's proposal would also like to have Ignore Whitespace pre-checked on http://127.0.0.1:5000/diff/... pages NOTE: tried to mimic such feature by /^\s+/m (and without m) in page (local) Ignore text and /\S+/m (and without m) in Extract text (just a test) but it did not work (any idea why?)

jathri avatar May 09 '23 10:05 jathri