dgtlmoon / changedetection.io

The best and simplest free open source web page change detection, website watcher, restock monitor and notification service. Restock Monitor, change detection. Designed for simplicity - Simply monitor which websites had a text change for free. Free Open source web page change detection, Website defacement monitoring, Price change notification
https://changedetection.io
Apache License 2.0
16.94k stars 947 forks source link

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

Open jbd7 opened 1 year ago

jbd7 commented 1 year ago

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

dgtlmoon commented 1 year ago

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)

jbd7 commented 1 year ago

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

dgtlmoon commented 1 year ago

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

jbd7 commented 1 year ago

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:

jathri commented 1 year ago

+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?)