I updated Fossil from 2.21 to 2.25, knowing about the markup/custom skin changes (which actually had a pretty minimal impact with only minor repairs needed to my custom CSS).

However, there appears to be an unrelated change to the way tables are styled in unified and side-by-side diffs. It looks like the table widths are now set by a script which directly sets the pixel width using inline styles on the table/pre elements.

Whatever method the script is using to detect/calculate the table width, it gets the wrong answer if the content area’s width is not approximately 100%. You can see the problem in action by looking at https://joeldueck.com/code/jdcom/info/86818268e68f1c22, and even more if you try resizing your browser window. The problem goes away if I disable max-width: 1000px in my content area, but of course breaking my stylesheet is not a solution.

Why is Fossil using a script for this job as opposed to responsive CSS? The browser layout engine is already doing all of this work for us. And it makes overriding Fossil's styling in this area weirder than it is for any other area.

I checked the Change Log for any notes on this change and couldn’t find any. It doesn't seem to be mentioned on the custom skins page in the docs, and a quick best-effort non-exhaustive search of the forum also didn't turn anything up.

FYI, my custom skin/CSS doesn't touch any of the diff-related elements (except for styling fonts and font sizes in pre and code elements).

Patches thoughtfully considered.

This I am already aware of in general. It still makes sense to ask about the reasoning behind a change before working on a patch, as well as to supply any information on it to those who you would hope might offer a patch.

Why is Fossil using a script for this job as opposed to responsive CSS?

My vague recollection is that we attempted to do it with CSS and ran into the problem that the chunks of the diff would resize inconsistently. So the first chunk might be split 50/50 across the left/right, the next one 20/80, and the next one 70/30. It looked godawful.

JS was definitely not our first choice for that, and a working CSS-only solution would be strongly preferred if someone can figure out how to do it.

I'm somewhat familiar with the code from trying to fix some (modern) browser incompatibilities ("Hello darkness, my old friend ..."), and also from maintaining a backport to Vanilla JS ("Here comes the sun ...") for my IE-compatible Fossil build. The code is quite a beast, indeed.

I think another reason for the JS-based solution is the sync'ed scrolling. It's quite nifty, but then again ... as soon as I have to start scrolling side-by-side diffs when doing code review, I'm done with my attention.

In general, my slow moving away from side-by-side diffs has recently come to an end with fossil all set preferred-diff-type 1. I think they never have the right information density, on wide screens they require too large a saccade to make a visual connection between left and right, and on narrow screens the scrolling is disruptive (all subjective, of course).

My GUI diff tool¹ solves this by highlighting the background throughout left and right as a visual guideline, so that works for me and my fluttery eyes.

I've noticed GitHub also has side-by-side diffs² (maybe that's new, or maybe it was below my radar), and they ENABLE WORD WRAP TO GET HALF-WIDTH BLOCKS AND AVOID THE SCROLLING "PROBLEM"! But thinking of it twice, this is something I'd vote for: it increases information density on small screens without adding disruption (by scrolling), and might simplify the web UI JS as a CSS-only solution. (My vote is passive, it's not something I'd like to push.)

1. ^{^} https://winmerge.org/
2. ^{^} Random example on code I've written more than 20 years ago, when code formatting didn't matter to me: https://github.com/zufuliu/notepad4/commit/620df1601c?diff=split&w=0

I'm finding the inline diffs produced by fossil diff -b highly useful, particularly with Markdown prose written with editors that like to keep entire paragraphs on one line. Traditional diff shows the entire paragraph as changed when all you did was fix a one-character grammar mistake. (e.g. "a" → "an") Fossil's browser diff shows the useful truth: one new character in the middle of a very long line.

Here's what I have so far, see what you think. This CSS overrides the widths set by the javascript, gets consistent 50/50 splits on side-by-side diffs, and works with the scroll-syncing aspect of the javascript. The only possible problem anyone might point to is that it uses CSS Grid layout which has been supported by a mere 96% of browser installations globally since 2017 (source)

I’ve tested this both on the default skin (temporary example) and on my custom skin (temporary example).

Note, this CSS only works on the first two chunks of any given checkin. This is because Fossil does not add class names to the "chunk" td elements, only IDs like "chunk1" "chunk2" etc. — that would need to be patched in order to complete this fix. If the chunk were always the nth child element in its row I could use a selector for that, but such is not the case.

table.udiff tr#chunk1, table.udiff tr#chunk2 {
  display: grid;
  grid-template-columns: auto auto auto 1fr; 
  grid-template-rows: 1fr; 
  gap: 0px 0px; 
  grid-template-areas: 
    "difflnl difflnr diffsep difftxtu"; 
}

table.splitdiff tr#chunk1, table.splitdiff tr#chunk2 {
  display: grid;
  grid-template-columns: auto 1fr auto auto 1fr; 
  grid-template-rows: 1fr; 
  gap: 0px 0px; 
  grid-template-areas: 
    "difflnl difftxtl diffsep difflnr difftxtr"; 
}

td.difflnl { grid-area: difflnl; width: fit-content !important; }
td.difflnr { grid-area: difflnr; width: fit-content !important; }
td.diffsep { grid-area: diffsep; width: fit-content !important; }
td.difftxtu { grid-area: difftxtu; }
td.difftxtl  { grid-area: difftxtl; }
td.difftxtr  { grid-area: difftxtr; }

/* Override pixel widths added by javascript */
td.difftxt { width: 100% !important;}

td.difftxt > pre { 
  max-width: 100% !important;
  width: 100% !important;
}

Here's what I have so far, see what you think.

Excellent! Admittedly, the grid layout did not come to mind in the initial development, but it was also sufficiently new at the time that it probably would have been discounted anyway.

It is unfortunately so hot here that my computers have been turned off and i'm limited to a tablet, but i will work on integrating this as soon as sitting on the computer is not painfully uncomfortable. The local forecast currently says Monday will cool down.

Thank you for solving this!

Wonderful, thank you in advance! I'll be happy to help test of course, or whatever else I can do to help.

In case it helps, some additional notes/thoughts:

An additional caveat I thought of: on side-by-side diffs, the left/right splits might differ noticeably between chunks in cases where the line numbers of each chunk have a high delta: for example, when one chunk starts on line 11 of a file, and the following chunk starts on, say, line 10,372. To me this scenario is acceptable but I thought I’d mention it. I haven't tested this scenario under either the existing implementation or the new one; I would guess perhaps they both behave roughly the same in this area.

I've refactored the CSS a bit to remove rendundant rules, and to divide it more cleanly into two sections: the part that would replace the sizing logic in the javascript, and the part that only exists to override the current javascript sizing (and which could be discarded once the javacript is no longer interfering here).

Obviously the tr#chunk1 and tr#chunk2 selectors would be replaced by tr.chunk or whatever class selector you decide to add.

tr#chunk1, tr#chunk2 {
  display: grid;
  grid-template-rows: 1fr; 
  gap: 0px 0px; 
}

table.udiff tr#chunk1, table.udiff tr#chunk2 {
  grid-template-columns: auto auto auto 1fr; 
  grid-template-areas: 
    "difflnl difflnr diffsep difftxtu"; 
}

table.splitdiff tr#chunk1, table.splitdiff tr#chunk2 {
  grid-template-columns: auto 1fr auto auto 1fr; 
  grid-template-areas: 
    "difflnl difftxtl diffsep difflnr difftxtr"; 
}

td.difflnl { grid-area: difflnl; }
td.difflnr { grid-area: difflnr; }
td.diffsep { grid-area: diffsep; }
td.difftxtu { grid-area: difftxtu; }
td.difftxtl  { grid-area: difftxtl; }
td.difftxtr  { grid-area: difftxtr; }

/* This part can be discarded if resizing logic is removed from the javascript */
td.difftxt {
  width: 100% !important;
}

td.difftxt > pre { 
  max-width: 100% !important;
  width: 100% !important;
}

/* I needed to add this in order to see the line numbers and separators during testing,
   because of a rule elsewhere in the default CSS that sets them to 1px wide. I suspect
   those rules would/could be updated to use fit-content without the !important flag. */
td.diffln, td.diffsep {
  width: fit-content !important; 
}

Finally — unrelated to the sizing issue, there's a quirk of Safari (macOS and iOS) that clips the radius corner on the border drawn around diff chunks in the default theme. It can be fixed by setting the border radius on all the <td> elements that might touch the corners of the table, so that they don’t “paint over” the corner. I can fix it for myself with the custom CSS below, but perhaps as long as you’re in there, consider adding this to the default CSS as well:

/* Fix quirk that clips the corner of the drawn border on Safari */
td.difftxt, td.diffln, td.chunkctrl { border-radius: 5px; } 

Here's what I have so far...

My apologies for the delay on this, but we now have a a branch with a CSS-based SBS diff. This eliminates tons of JS code we currently use for that.

Caveats, as mentioned in above posts but summarized here:

• Two adjacent blocks which have a large gap in their line numbers, e.g. a block with lines 1-10 followed by a block for lines 1000-1010, will not line up quite as nicely as the JS impl does. IMO, no big deal.

• We lose synchronized side-by-side scrolling. We can re-add that, but... do we need to? Does that really gain us anything?

If we can accept these limitations as-is, we can strip out a large chunk of currently-disabled JS and merge it. If, on the other hand, we really want to keep synchronized scrolling, i'll need to go dissect the corresponding JS and get those parts working on this impl.

Opinions and suggestions are, of course, welcomed.

@Florian: this uses CSS grid, which won't, AFAIK, work on MSIE.

@Florian: this uses CSS grid, which won't, AFAIK, work on MSIE.

No worries, I'm depleted of stress hormones for the moment and luckily it takes my adrenal glands some time to refill ... ;-)

Opinions and suggestions are, of course, welcomed.

I've used the sync'ed scrolling and have seen it as a plus for the not always optimal (regarding browser window width, too wide or too narrow) side-by-side diffs. But I'm completely switched to unified diffs, and today I think I'd be more likely to resize the browser window instead of horizontally scrolling (possibly multiple) side-by-side diff blocks. So I don't have an opinion, here.

The only thing I'd personally consider is whether to completely abandon vertical scrolling in favor of word wrap, similar to GitHub¹ (and also GitLab²). I think that's quite nice, but when I mentioned the idea, there was no feedback that other people also might like this, so it's probably not worth pursuing.

1. ^{^} https://github.com/zufuliu/notepad4/commit/620df1601c?diff=split&w=0
2. ^{^} https://gitlab.freedesktop.org/mstoeckl/waypipe/-/commit/2545e8100393660d37d90b7fc20958617d6cbf0d?view=parallel

Just noticed both GH and GL also enable word wrap for unified diffs (so my browser windows obviously happened to be wide enough for the projects I'm interested in).

... the client-side diff chunk loader would need to be reworked ...

Didn't think of that!

The synchronized horizontal scrolling is "nice" but can also be considered harmful.

FWIW, that's also my current take.

I'm inclined to think that the improvement in the handling of the table width surpasses the loss of the synchronized scrolling.

Ditto.

i'll give it a few more days for objections before merging it. Which reminds me: a caveat not spelled out in the post above:

• This uses CSS grid, and the first browsers to support that were released in 2017. i.e. it won't work on I.E., nor on Android 4.4. Though i have no obvious way to test such an old browser, simply disabling the CSS grid in a modern browser causes it to fall back gracefully: the only obvious differences in Firefox/Chrome are:
  • Each column gets an implicit minimum size which the CSS grid does not impose. To reproduce that, simply narrow the browser window until the columns stop shrinking: that threshold is much lower with CSS grid than without.
  • The columns of each separate chunk of the diff do not line up vertically with each other (as described in one of the first responses in this thread). That's unsightly but it works.

While testing that fallback, however, a new bug was found which went unnoticed so far: the two line-number columns both overlap along far left edge. Fixed.

@Joel do you have any idea of how to get the right-side line number column back into place? My CSSGridFu is sorely lacking. Nevermind - found it! It was a CSS transcription error on my part.

Looking great so far, thanks so much for working on this!

There is a minor bug in the unified diffs caused by some missing CSS.. The CSS in the latest checkin for this branch reads as follows:

table.splitdiff tr.diffchunk {
  display: grid;
  gap: 0px 0px;
  grid-template-rows: 1fr;
  grid-template-columns: auto 1fr auto auto 1fr;
  grid-template-areas: "difflnl difftxtl diffsep difflnr difftxtr";
}
table.udiff tr.diffchunk {
  grid-template-columns: auto auto auto 1fr;
  grid-template-areas: "difflnl difflnr diffsep difftxtu";
}

The first three lines in the table.splitdiff tr.diffchunk selector should also be made to apply to the table.udiff tr.diffchunk selector. If it were me I’d refactor it like this (or just copy the three lines in question from the first selector to the second):

tr.diffchunk {
  display: grid;
  grid-template-rows: 1fr; 
  gap: 0px 0px; 
}

table.splitdiff tr.diffchunk {
  grid-template-columns: auto 1fr auto auto 1fr; 
  grid-template-areas: "difflnl difftxtl diffsep difflnr difftxtr"; 
}

table.udiff tr.diffchunk {
  grid-template-columns: auto auto auto 1fr; 
  grid-template-areas: "difflnl difflnr diffsep difftxtu"; 
}

With this change the column widths line up better between chunks when using the unified diffs (was this the “unsightly” part you mentioned?).

... the column widths line up better between chunks when using the unified diffs (was this the “unsightly” part you mentioned?).

i was referring to the sbs diffs, but this last change, much to my surprise, resolves that unsightliness in those too.

Very nice, thank you :).

... a few more days for objections before merging it.

The new CSS-grid-based side-by-side diff has been merged. Many thanks to Joel for getting that to work.

Summary of the differences:

• Eliminates all JS code related to resizing the side-by-side diff columns and synchronizing the scrolling those diff chunks.
• This requires a 2017-ish-era browser or newer but it degrades moderately well on browsers which don't support CSS grid: particularly wide diffs may get truncated out of view along the right edge, whereas compliant browsers will shrink the columns to fit.

These changes apply to the diffs in the web UI but do not apply to the diffs generated by fossil diff -by. Maybe they should?

has been merged

Containers rebuilt and pushed, then tested on a few of my repos, with macOS and iOS clients.

Maybe they should?

No opinion. I use that feature without the -y, primarily on docs with very long lines, where the inline diffs it gives are most helpful.

(Contrasting many other diff tools, where a single-character change in a 1000-character line is marked as "this line differs." Yes, very helpful, $TOOL. 🙄)

Awesome, thank you so much! This is working great.