Fossil Forum

New Fossil Diff Logic
Login

New Fossil Diff Logic

New Fossil Diff Logic

(1.1) By Richard Hipp (drh) on 2021-09-03 23:25:18 edited from 1.0 [link] [source]

I've been working to improve and clean up the "diff" logic in Fossil in the diff-color-enhancements branch. (The branch name is something of a misnomer as there are more than just enhancements to the colors.) I'm about ready to merge these changes to trunk and continue the development there. This is your opportunity to object if you feel this would would be disruptive.

The new Fossil is currently running the experimental server: https://fossil-scm.org/fossil-ex.

Significant changes:

  • Partial-line diff highlighting in unified diffs

  • Improved partial-line diff highlighting in side-by-side diffs

  • Simplify and rationalize the DOM and class names for web-based diff displays, so that they are easier to understand and easier to customize. This change might break custom skins!

  • Style tweaks in the default skin.

  • Significant internal reorganization of how diffs are generated.

The work on diff logic is not complete. Further enhancements are coming. But I think it has reached a point where it needs to be on trunk. In particular, I already miss the new changes when I work with older versions of Fossil.

If you have any objections to these changes, speak up soon, as otherwise I will merge to trunk shortly.

Comparisons:

Update (2021-09-03 23:24:46 UTC):

I have not yet merged the changes. However, I have running the branch version of Fossil as the primary Fossil engine on the https://fossil-scm.org/ website (and also on https://sqlite.org/). So visiting the primary website will now show you the new diffs.

(2) By Larry Brasfield (larrybr) on 2021-09-03 13:29:58 in reply to 1.0 [link] [source]

(Not an objection, so perhaps off-topic:)

I just about weighed in on the need for the blue highlit, "changed" line diff display format, citing similar effective use in a couple of 3rd party diff-merge tools that I have found very useful sometimes. I have continued to use one of them even after becoming a semi-dedicated Fossil user for difference comprehension.

With the changes evident in the old vs new displays, I see the end of my using those dedicated tools as diff-viewers for almost anything in a Fossil repo. Hooray!

Merge tools are another story. They put up a 3rd window for an "edited" merge output. A great feature in some of these tools is the "take this" or "take that" button, operating on individual changes or highlighted portions thereof.

I do not suggest by this that Fossil should evolve a 3rd-window, merge feature.

The demonstrated changes completely address my main objection to some of the intermediate, blue-less diff display proposals. I prefer not to have to sift through mostly identical lines to perceive their small differences. I consider it a waste of a scarce resource: brain cycles while considering a potentially complex situation.

(3.1) By Marcelo Huerta (richieadler) on 2021-09-03 16:36:30 edited from 3.0 in reply to 1.0 [link] [source]

With the three latest checkins in the diff-color-enhancements branch, I get this:

unmatched open quote in list
    while executing
"lindex $line 0"
    (procedure "readDiffs" line 16)
    invoked from within
"readDiffs $fossilcmd"
    ("eval" body line 364)
    invoked from within
"eval $prog"
    (file "file-61720b2f05d6fc34" line 504)

when I try to run

fossil diff -tk

version:

This is fossil version 2.17 [0755a81bb5] 2021-09-03 14:33:58 UTC
Compiled on Sep  3 2021 12:01:09 using msc-19.23 (32-bit)
Schema version 2015-01-24
Detected memory page size is 4096 bytes
zlib 1.2.11, loaded 1.2.11
hardened-SHA1 by Marc Stevens and Dan Shumow
SSL (OpenSSL 1.1.1k  25 Mar 2021)
FOSSIL_ENABLE_LEGACY_MV_RM
JSON (API 20120713)
MARKDOWN
UNICODE_COMMAND_LINE
FOSSIL_STATIC_BUILD

(4) By Larry Brasfield (larrybr) on 2021-09-03 15:26:35 in reply to 3.0 [link] [source]

To save some time, I ask:
What inputs did you provide to induce the misbehavior you report?
(That would include a private repo and any custom skin.)

(5) By Marcelo Huerta (richieadler) on 2021-09-03 16:40:42 in reply to 4 [link] [source]

Sorry, I omitted the most important part:

This error happened when I tried to run

fossil diff -tk

not the web version. Apologies for the confusion. (I fixed the message above also.)

This is for tclsh version 8.6.9.

(6) By Richard Hipp (drh) on 2021-09-03 16:43:15 in reply to 5 [link] [source]

Can you please send me (via private email to drh at sqlite dot org) the output of "fossil diff --tcl". NB: "--tcl" not "--tk". That text will be the content that the TCL script is attempting to parse when it encounters the error.

(8) By Marcelo Huerta (richieadler) on 2021-09-03 16:48:16 in reply to 6 [link] [source]

Sent

(9) By Richard Hipp (drh) on 2021-09-03 17:52:29 in reply to 8 [link] [source]

Please try again with the latest check-in from the branch. Let us know whether or not this clears your problem.

(10) By Marcelo Huerta (richieadler) on 2021-09-03 19:40:43 in reply to 9 [link] [source]

Running with fossil version 2.17 [3e08b15858] still gives me the same error.

Another output of the previously ran command has been sent to your email.

(11) By Richard Hipp (drh) on 2021-09-03 20:53:03 in reply to 10 [link] [source]

Can you send me the input files so that I can run the diff myself?

(16) By Richard Hipp (drh) on 2021-09-04 10:29:08 in reply to 10 [link] [source]

Another update has been uploaded (check-in b4c961e8fb788d2b). Please try the latest and let me know if that clears the problem.

(17) By Marcelo Huerta (richieadler) on 2021-09-04 14:33:45 in reply to 16 [link] [source]

Now it runs properly. Thank you.

(7.1) By Richard Hipp (drh) on 2021-09-03 16:45:52 edited from 7.0 in reply to 5 [link] [source]

Deleted

(12) By sean (jungleboogie) on 2021-09-03 22:29:27 in reply to 1.0 [link] [source]

Hi,

Thanks for the hard work you've put into improving the diff logic and display of the diffs.

Couple things I've noticed...

Because the side-by-side diffs are larger (which is welcomed by me), do yall think it makes sense to draw a border around the diffs? That happens with unified diffs now. Maybe it would make it easier to separate out files visually. Zooming my browser out to 80% makes things still very legible for me, but

Side by side with many changes: https://fossil-scm.org/fossil-ex/info/ea52b7d06cf7c049

Unified diff with border: https://fossil-scm.org/fossil-ex/vinfo/ea52b7d06cf7c049?diff=1

Using that same side by side, there are 13 horizontal scroll bars on the right side diffs, but only a few are needed to see the rest of the text. Is that just a browser thing putting the scroll bars there?

Also interestingly, if there are two (or probably more) scroll bars for the same file, the bottom scroll bar will move three of the four files.

Use src/diff.c as an example and slide the bottom right bar for that file. Notice that portion of the file moves, the file above moves and the top left file moves as well, not the left side of the file, though.

(13) By Richard Hipp (drh) on 2021-09-03 23:49:48 in reply to 1.1 [link] [source]

Diff Test Cases

There is a small collection of diff test cases in the test/diff-test-1.wiki file. Please point out lines on these tests that need improvement, and/or suggest new tricky diff cases that should be added to the test document. Thanks for your help in making Fossil better!

(14) By sean (jungleboogie) on 2021-09-04 02:43:05 in reply to 1.1 [link] [source]

Thanks for taking my feedback and putting it in. I think the borders enclose things much better and hopefully others don’t mind, either.

Another consideration….adjacent to or below the modified file name, display how many lines were modified:+/- stuff for the given file.

(15) By Stephan Beal (stephan) on 2021-09-04 02:47:13 in reply to 14 [link] [source]

Another consideration….adjacent to or below the modified file name, display how many lines were modified:+/- stuff for the given file.

That's another case of the header being output before the diff and the number of changes not being known (or not being counted) until the diff is completed. In order to display that info, the diff would effectively have to be traversed twice or we'll have to inject that info after diff generation using JS.

(19) By sean (jungleboogie) on 2021-09-05 00:39:43 in reply to 15 [link] [source]

mea culpa

(18) By Marcelo Huerta (richieadler) on 2021-09-04 15:29:46 in reply to 1.1 [link] [source]

I found a display problem in the new web diff.

In some diffs the +/- signs appear shifted.

I have added a file in the same test repository whose location I sent you privately before. (If you diff from "import_num.py: before" to "import_num.py: after", the problem shows, but if you do it in reverse, it doesn't. Hiding whitespace doesn't seem to make a difference.)

(20) By Richard Hipp (drh) on 2021-09-06 14:38:58 in reply to 1.1 [link] [source]

Update 2021-09-06:

The diff enhancements have been merged onto trunk. New precompiled binaries are available on the download page. Please report any problems.

(21) By Ralf Hoermann (ralf1024) on 2021-09-07 21:49:54 in reply to 20 [source]

One of the latest diff enhancements seem to have broken the vpatch page (example). Unless I'm missing something, you can no longer tell where another file starts in a multi-file diff. A bisect identified this as the breaking commit.