The current trunk now, on JS-aware browsers, alters the diff-viewing pages to include buttons to load extra lines of context. This is based roughly on github's similar feature, which is exemplified at:

https://github.com/msteveb/autosetup/commit/235925e914a52a542

This diff demonstrates it pretty well:

https://fossil-scm.org/home/vinfo/49a60a580de9f3c2

Because this feature requires a connection to the server, it does not get applied to the CLI-side output of the diff -b and -by options. Likewise, on non-JS-aware browsers it doesn't do anything: the diffs will appear like they always have, with a placeholder in the slot where the new UI controls are injected.

It will certainly undergo some tweaking but is essentially feature-complete. This is a brand new feature and is bound to have some bugs and feature requests. As always, i'll make every attempt to squash (or justify) any bugs and justify away any feature requests ;). (Even so, please submit them!)

Known issues:

• Bootstrap skin does "something weird" with the diff tables in at least one of its mediaquery (screen size) configurations, applying inconsistent sizes, while behaving properly in others. The other skins are believed to be working as expected.

Okay, I have another suggestion for consideration.

Within the context section of a commit, put the up/down arrows. https://fossil-scm.org/home/info/0f7740b6329e5338

As of now, you can click the check-in link to traverse the timeline while in diff viewing mode, but might be neat to see more commits.

Within the context section of a commit, put the up/down arrows.

That is an interesting idea. That would require a new remote API but shouldn't pose any particular problem. That said, i have multiple TODOs lined up already so won't immediately start on that one.

That sounds like feature creep to me. Unlike the diff context, an expanded view of the "Context" section is just one click away: click the date in "Overview" → "User & Date", or click the time stamp next to each commit (if "Timeline Display Preferences" → "Timestamp hyperlinks to /info" is disabled) to get a full timeline view with the current commit highlighted, optionally in a new tab (Ctrl+Click) or new window (Shift+Click).

The extra diff context is not that quickly accessible, and links to the involved files, even if they included the line numbers, don't give the same view (i.e. they're lacking the diff).

That sounds like feature creep to me.

Sure, it certainly could be considered that way.

However, I don't think I was expecting it to be an expanding commit context, but rather, a scrolling diff context - kind of like a mini timeline. Your suggestion of clicking the already existing link is legitimate. We already have that link and displays nicely on the timeline.

I recommended the mini timeline idea, since Stephan implemented the expanding diff and was thinking of other ways it could be re-used.

Thanks for your thoughts and I have no objections to it.

FYI…

On mobile, the slightly smaller font remains slightly smaller when going up the diff, but the font size increases going down the diff. Also when viewing down, the line numbers don’t match up with the font.

Also when viewing down, the line numbers don’t match up with the font.

i can reproduce that on mobile but not the desktop. It's independent of the new loading feature, though: it happens even without loading new content.

i'm looking into a solution but debugging mobile quirks like this is a bit of a pain.

It's easily visible in the bottom half of this chunk.

i'm looking into a solution but debugging mobile quirks like this is a bit of a pain.

It's reproducible on both FF and Chrome on Android, but all attempts to solve it so far have not done so. We had this same problem with the line numbering reimplementation and the solution was to ensure that the font sizes and line heights were all forcibly propagated down the DOM. That fix is not working here.

Unfortunately, it's not reproducible in Chrome's dev tools using mobile device emulation, so it's about impossible to debug exactly what's mismatched here. The best we can do, unless someone has a clear idea of what's wrong, is trial and error until it works.

i'm looking into remote debugging options.

... the line numbers don’t match up with the font.

That's now fixed on trunk. Thank you for the report!

Details: Larry and Richard noticed that the misalignment started with lines which contained Unicode arrows (which is why default.css demonstrated it so clearly). That observation made the solution obvious: increase the line-height. We just had to tinker with that value until we found a workable minimum.

Private mail sent with a picture of the issue I’m seeing.

Guess what...I got this to happen on Chrome v76.0.

When expanding the diff (in side-by-side or unified), I see this li.join(...).replaceAll is not a function.

Maybe it's expected with this version of Chrome, i don't know.

Maybe it's expected with this version of Chrome, i don't know.

Definitely not expected. i'm away from the computer for a few hours but will look for a replacement option when i get back.

I see this li.join(...).replaceAll is not a function.

That's been replaced with a more portable construct. Thank you for the report!

Now that I'm back on a computer, here's the picture I sent to Stephan. We've exchanged a few emails over the weekend and he's been unable to see what I continue to see, even after the fix.

https://imgur.com/a/t3peRN8

Does anyone else see this layout on their mobile device? I'm using an iphone 6 with Firefox installed. I've deleted cache/private data and I've tried on multiple different wifi networks and tried on my mobile network.

I'm not so concerned about me not having the proper layout, but I'd rather be sure it's only me and not a surprise once there's a new release.

Does anyone else see this layout on their mobile device? I'm using an iphone 6 with Firefox installed.

i'm still unable to reproduce that on both a tablet and phone with FF and Chrome, but i have a suspicion. Is it possible for you to test a patch from your phone? If so, please add this to the end of src/default.css:

table.diff tr, table.diff td,
table.diff td > pre,
table.diff td > pre > * {
  font-size: inherit;
  line-height: inherit;
}

That "shouldn't be necessary, but i suspect that the built-in style for that particular browser is overriding the font size and/or line-height for the elements via CSS rules which are more specific (better matches) than ours.

Edit: if you're unable to try that via your phone, let me know and i'll check that in for testing purposes. It won't have any effect on most clients but adds CSS we don't (or shouldn't) strictly need.

The font-size thing you reported in another thread is a separate issue i haven't yet looked into but have a suspicion about (with an easy solution if it turns out to be correct).

I made the changes to default.css on top of the latest Fossil commits and started my fossil server on the Fossil repo.

The most recent commit displays and expands correctly, but selecting other random commits causes the misalignment that only I have seen.

The eagle and bootstrap skins seem to have the similar issue, if that means anything to you.

This is a diff of the changes:

Index: src/default.css
==================================================================
--- src/default.css
+++ src/default.css
@@ -1917,5 +1917,12 @@
 @media screen and (max-width: 1200px) {
   .wideonly {
     display: none;
   }
 }
+
+table.diff tr, table.diff td,
+table.diff td > pre,
+table.diff td > pre > * {
+         font-size: inherit;
+           line-height: inherit;
+    }

The most recent commit displays and expands correctly, but selecting other random commits causes the misalignment that only I have seen.

The misalignment "should" only show up on checkins which include certain parts of default.css, as that's the only file which includes the characters which trigger the problem. For a reproducible case, see the bottom half of this chunk.

One more thing to try is to change this block in default.css:

table.diff pre {
  margin: 0 0 0 0;
  padding: 0 0.5em;
  line-height: 1.20 /* important for mobile:
                      see forum post e6f4ee7de98b55c0 */;
}

and increase that 1.2 to 1.3 or even 1.4. You will probably see blank gaps between the insertion/deletion lines with that change, but it might fix the misalignment (and i might have a way of filling those gaps - i'll try that while you're trying this).

and increase that 1.2 to 1.3 or even 1.4.

If 1.3 or 1.4 works for you, please try incrementally smaller values until you find where it breaks. The lower the value, the better, but 1.2 is the current minimum which we know works in most places, so don't go below that. If 1.3 works for you, i'd start next with 1.25.

i seem to have found a way to eliminate the color gaps which will appear with higher line-height values, which i'll apply if we can find that a given line-height value works for your browser.

The fossil.diff.js code is quite sophisticated, and I think it's really well done! Congratulations, nice work!

Here's my accumulated feedback from combing through the code in detail while backporting it to my IE-compatible version of Fossil.

(0) Backporting is surprisingly easy: replace const with var, prepend the replacement code for fossil.dom.js (see below) to the script, and rewrite the fetchArtifactChunk() routine to handle all the XHR stuff itself (about 20 lines of code).

var D = {
addClass:function(e){for(var i=1;i<arguments.length;i++)e.classList.add(arguments[i]);return e;},
append:function(p){for(var i=1;i<arguments.length;i++)p.appendChild(arguments[i]);return p;},
attr:function(e){if(arguments.length==2)return e.getAttribute(arguments[1]);for(var i=1;i<arguments.length-1;i+=2)e.setAttribute(arguments[i],arguments[i+1]);return e;},
clearElement:function(e){while(e.firstChild)e.removeChild(e.firstChild);return e;},
div:function(p){var e=document.createElement('div');if(p)p.appendChild(e);return e;},
pre:function(p){var e=document.createElement('pre');if(p)p.appendChild(e);return e;},
remove:function(e){e.parentNode.removeChild(e);return e},
span:function(p){var e=document.createElement('span');if(p)p.appendChild(e);return e;},
td:function(p){var e=document.createElement('td');if(p)p.appendChild(e);return e;}
};

Together, the dom and fetch rewrites amount to less than 1.5 KB of Javascript code to replace two 28 KB and 9 KB Javascript framework libraries. ;-)

(1) It looks like the fetchTrChunk() function in fossil.diff.js is unused, so at least (almost) 4 KB that could be stripped. ;-)

(2) On a desktop monitor, the "Cannot load chunk while a load is pending" (toast? notification?) message is not only (too) far away from the proceedings, but is usually (after a bit of scrolling to get to the diffs) blending over the "Context" area, which already has different background colors -- so it regularly gets unnoticed. I also think the message is unnecessary, and would like to suggest it to be replaced with some disabled-state look of the up/down buttons (for example, with CSS similar to the hover-state look, but in the opposite direction of brightness).

Together, the dom and fetch rewrites amount to less than 1.5 KB of Javascript code to replace two 28 KB and 9 KB Javascript framework libraries. ;-)

You're ignoring the caching and reuse effects, however: fossil.fetch and fossil.dom are used in all of the JS-heavy pages and have an extremely low amortized traffic cost. Also, code cost is measured not only in size but in developer/development time. Those extra kb are an insignificant cost compared to the time it takes me to write in plain DOM API code (and re-write it for every page/app).

addClass:function(e){for(var i=1;i<arguments.length;i++)e.classList.add(arguments[i]);return e;},

You've removed an important feature there which is actually used by some of our apps (multiple target DOM elements as the first argument). The fossil.dom functions are written they way they are because the person writing the client code finds their features useful ;).

If we did not have library-level code in place to eliminate stuff like DOM node creation and manipulation, most of our JS code would never get written because it's simply too tedious to develop any significant amount of JS that way. Any amount of time spent rewriting features costs me more than the double-digit kilobytes of reuse do.

It looks like the fetchTrChunk() function in fossil.diff.js is unused, so at least (almost) 4 KB that could be stripped. ;-)

i'll get that removed in a few moments - it was an early failed development direction.

On a desktop monitor, the "Cannot load chunk while a load is pending" (toast? notification?) message is not only (too) far away from the proceedings, but is usually (after a bit of scrolling to get to the diffs) blending over the "Context" area...

That's a fair point - a toast was used because i needed a solution and didn't (and still don't) have a more suitable one readily available except for simply sending the errors to console.error with no UI interaction.

Disabling the buttons would be overkill, though. The errors, if any, are very likely transient network problems. In all of development and testing, the only time the error toasts showed up was when there was a bug in the related code, not due to a network problem or XHR error response. If you were seeing errors frequently, it's possibly because you were working on the code?

I've deliberately ignored all the points you bring up -- I just wanted to let the world know that I have a private build of Fossil with dynamic diff context loading in IE ;-)

Disabling the buttons would be overkill, though. The errors, if any, are very likely transient network problems. ... If you were seeing errors frequently, it's possibly because you were working on the code?

No, I see them all the time with the Fossil canonical website, due to network latency.

Right now, my average ping time for the Fossil website is 175 ms. So when clicking one of the "Chunk Load" buttons, nothing seems to happen, there's no visual indication that any background work is in progress. And when I click again, I get that yellow "Cannot load chunk while a load is pending" bar far away at the top, often hiding behind the yellow background of the currently selected check-in (and possibly other colored branches) from the "Context" section.

With such network latency, I'd prefer some immediate visual feedback, to be sure the action was initiated and can be waited for. The clicked button changing to some disabled state seems like the most natural and least invasive approach, in my opinion?

Is that convincing to you? If not, I wonder why you might have better network latency for fossil-scm.org, when you are living on the same continent as I do? ;-)

With such network latency, I'd prefer some immediate visual feedback, to be sure the action was initiated and can be waited for. The clicked button changing to some disabled state seems like the most natural and least invasive approach, in my opinion?

That's easy to do, as far as the ajax parts go, but will be a bit fiddly for the UI elements because they are not true buttons, so the ability to disable them will need to be added. So... yet more JS code ;).

Is that convincing to you?

i'm not convinced that half a second of delay is unexpected or justifies adding full-fledged UI components capabilities to the buttons, but i will look into that option.

There's already a "busy" flag to trigger the toast? Just keep that to swallow repeated clicks, and also toggle the "disabled-look" class with the flag? Or is the flag not per-button? Can't look it up right now...

There's already a "busy" flag to trigger the toast? Just keep that to swallow repeated clicks, and also toggle the "disabled-look" class with the flag? Or is the flag not per-button? Can't look it up right now...

There is such a flag, and it's per "diff separator," not per button, but i found a nice way to provide a distinct visual indication of what's going on which keems me from having to implement the disabled buttons in a skin-friendly way, it just needs to be implemented.

I haven't considered the required skin updates, so adding CSS for a disabled style seemed simple.

Looking forward to see your approach, especially if it's even less obtrusive than disabling the buttons... ;-)

But don't get me wrong, if you think this feature is not worth your effort, then please don't do it (and share your idea, anyway). But in that case I'd rather remove the toast, and just let the user wait without any visual feedback. But there may be cases with much longer network latencies (access via mobile networks, for example; browser developer tools can simulate this). Also, there may be users clicking twice, for example if they already know they want twice the extra amount of context lines loaded -- but that's another story, and would require some kind of queueing.

I think your new solution is more robust, and the visual feedback is in the very area the user is currently focusing on.

I believe the toast is really the worst approach: aside from potentially already being far away, it can even be scrolled out of view. Whether or not the new solution is really more lightweight (or less "overkill" as you said) than disabling the buttons is probably a matter of opinion... ;-)

While porting the JS code to load the diff context to IE was straight forward, the scrolling code was more tricky. I had absolutely no idea what a construct like this might do! ;-)

After using it for a while, the still-enabled buttons doing nothing but immediately displaying the "Cannot load chunk while a load is pending" message seem a bit odd.

If disabling the buttons is not an option, what about queueing serial clicks? Please have a look at diff-js-fetchqueue if you find the time (no hurry, I'm online sporadically, and it may take a while for me to see your reply, anyway), and let me know what you think.

The feature works well with the "Next Up" buttons, for example if the file header to look up something is just a few clicks away from the current "Diff Separator", or if the gap between two diff blocks can be closed by a few clicks: just do these clicks, at any speed, for any network latency, and focus on the text again. No need to click "as slowly as network latency dictates"!

Tested with Chromium and Firefox, but only on Windows, so far.

In case this turns out to be useful, here's some possible TODOs:

• Is some "rate limit" needed (i.e. a maximum queue length)? Probably not, as usual SCM-managed files have finite amounts of lines, and injectResponse() slows down significantly after many rounds to "naturally prevent bursts".
• Coalesce multiple queued clicks (of the same "direction") into single (larger) XHR requests to `/jchunk'? Probably not, this may be too confusing, and also not worth the effort.

Looking forward to your feedback!

After using it for a while, the still-enabled buttons doing nothing but immediately displaying the "Cannot load chunk while a load is pending" message seem a bit odd.

On the one hand you're unhappy about the amount of JS code and on the other you're asking for more JS code to resolve a cosmetic non-issue. ;)

If disabling the buttons is not an option, what about queueing serial clicks? Please have a look at diff-js-fetchqueue ...

That's an interesting approach but we still have a huge number of people who habitually double-click everything and this would penalize those people. It also introduces error cases we don't currently have. For example, if there are 21 lines of context to fetch and a user triple-clicks, we've queued up 3 requests to load a total of 45 lines. The 3rd request will fail unless we proactively catch it and abort the request. (i think it will do nothing except send the message to console.error(), but i'm not 100% certain of that.) i'm not really keen on the idea of adding error handling for cases the UI should not be allowing to happen.

It may also introduce unpredictable click behaviour when one is working on a fast network connection, e.g. on a local machine: by the time i can get my third or fourth click in, the first click might already be loaded and shift the UI elements out from under me, leading me to click on something else.

In any case, it's a highly non-intuitive use of buttons (even if it does make some sense for this use case).

No need to click "as slowly as network latency dictates"!

i strongly suspect that context diff loading will rarely be used and will, more often than not, be used to load a single chunk. i don't expect people to be loading 5 chunks at a time. If they want that much context, they can open the whole file.

Looking forward to your feedback!

If you'd like to patch it for that, go ahead, keeping in mind the error handling for "no more data left to fetch" cases. i'm personally not thrilled about this behaviour, though, because it's a non-intuitive/unconventional use of buttons and introduces error cases we don't currently have.

i do like the idea of not having to wait for the network delay between clicks. i'm not convinced that that rare-case benefit justifies the rest, though. (The rare case being someone loading more than one context chunk. That's something we'll do a lot in testing/development but probably (i suspect) won't happen much in day-to-day usage.)

i'd rather see someone add visual indications of disabling, but the issue doesn't bother me nearly enough to do so myself ;).

Thanks for your feedback!

The error handling is there, give it a try ;-)

I've been using GUI systems since the beginning, when computers were slow, and programs single-threaded. Buttons remained enabled, even for lengthy operations, maybe because disabling and repainting was expensive, and because the OS provided input serialization, so clicks falling in the busy phase were queued and handled when ready. This was handy on old systems, as you could "click at your speed" N times to get N operations, without waiting for the sluggish display. Modern window managers still use the same serialization model, and many apps have kept this familiar pattern to date. Why slow down the user if they're faster then the system? Do you, as a terminal person never hit UP and ENTER to repeat a command before the running one has completed? You'd likely replace your shell if you couldn't do that ;-) That said, I find it highly unintuitive for a button to be enabled, but not accept new clicks.

But of course I don't want to push something you don't like. Thanks for your appreciated attention, anyway.

Why slow down the user if they're faster then the system?

1. queuing clicks is non-intuitive and unconventional, 2) many users habitually double-click anything which looks like it can be clicked, and 3) it adds error cases and handling which don't otherwise exist.

But of course I don't want to push something you don't like.

The moment i checked in the code it became the project's code, subject to the needs of the project, not limited to my personal tastes. If you feel that this is a valid need and improvement for you and others then feel free to check it in.

... context diff loading ... be used to load a single chunk.

I'm using the feature to look up the names and header comments for the modified functions. Not only with a mature project like Fossil, where there's a lot of local variables, this usually takes more than just one click.

It may also introduce unpredictable click behaviour ... and shift the UI elements out from under me ...

Your original version has the same problem, even more painful, because you have to wait for the button to jump away! In my version, you can at least try to avoid this by "clicking ahead".

But I agree it's not ideal, and a possible (quick) fix has been committed to the diff-js-fetchqueue branch.

... habitually double-click anything ...

I think the imposed "penalty" is negligible, in this case: an accidental double-click simply loads you two chunks.

... non-intuitive and unconventional ...

I don't think so, that's just how input processing has worked since the early times, maybe even since the beginning: when you press a key on your keyboard, the keystroke is written to a buffer (queue), and the application processes it as soon as it's in an idle state. (The OS may "asynchronously" read the keyboard input buffer during a hardware interrupt, but then just repost the input to it's internal queues.) Imagine keyboard buttons not working (without visual hint) until the buffer has been emptied!

... adds error cases and handling which don't otherwise exist ...

Not al all! I've carefully tested the feature with all possible network latencies and failures simulated, and with random delays (to test the effect of caching) and random generated errors by the jchunk() server-side handler. Also, it's no problem if the queue grows "really long" -- it's discarded before any "no more data" errors have a chance to appear. Things work really smooth -- thanks to your preparatory work, of course!

If you feel that this is a valid need and improvement for you and others then feel free to check it in.

I'm trying to convince you with technical arguments. But more than that, I hope you'll give it a try, and will like the behavior!

Also note that nothing stops you from using the feature in your preferred way, i.e. make a single click, wait for the requested lines to appear, and only then make the next click if needed. The inserted lines is the visual feedback that the operation is complete, and errors still appear next to the buttons.

My objections aren't really technical ...

You are of course free to have non-technical objections. But I think it's a bit of a pity you didn't at least try it out.

... except for the introduction of new error cases which don't otherwise exist.

This is the third time you have raised these concerns, and twice I have been unable to dispel them.

Let's recap:

• The click queue belongs to the "diff separator", so only events ("up", "down", "fill gap") that are allowed for the "diff separator" are posted to its queue.
• If the gap between two diff chunks gets small enough so the "up" and "down" buttons are replaced by the "fill gap" buttons, and the queue of the same "diff separator" is not empty, the queue is truncated and loaded with one single "fill gap" event.
• If the top or bottom of the file is reached, or if the gap between two "diff separators" is closed, the queue is deleted and any pending events discarded.
• In case of an XHR error, the queue is emptied and all pending events discarded, and an error message appears next to the button. The error message remains on screen until the user has fixed their network problem (or whatever) and they're ready to start over from the beginning, with an empty event queue.

After extensive testing and re-testing, I don't see any "new error cases which don't otherwise exist" in this logic. But I know must be missing something, please let me know what it is!

... if this helps you Get Stuff Done ...

Indeed, it does, it even helps me Get Stuff Done Faster!

As already mentioned, when following the Fossil development history on fossil-scm.org, i.e. without any local checkout and local tools, I sometimes would like to see the function name and declaration, as well as the comment atop the function modified by some diff block -- and then I always get these yellow bars.

It's not that "I know I need X clicks to get there."

It's more like "I know I'm below the local variable declaration block, so I need to skip that block, plus the comment atop the function to be able read it from the beginning. So I need at least X*(3-5) clicks to get there."

These units of 3-5 clicks correspond to the amount of extra diff context that fits on my screen -- so in fact I'm doing page-wise loading!

In my version, a rapid series of N clicks takes the following duration to completion:

    N * NETWORK_LATENCY

In your version, a series of N clicks takes the following duration to completion:

    N * NETWORK_LATENCY + (N-1) * HUMAN_REACTION_TIME

HUMAN_REACTION_TIME is the duration from the moment the yellow "Cannot load chunk while a load is pending" message disappears until I've made my next click.

So my version is faster, and yes, it helps me Get Stuff Done Faster!

Maybe it helps comparing the "chunk load buttons" more to "scrollbar buttons" instead of "conventional command buttons". For a window displaying a large amount of complex data (load https://fossil-scm.org/home/timeline?n=all and scroll around) the scrollbar buttons are never disabled, but always accept new clicks at any speed, and the queued scrolling events happen as soon as the application is ready to process them. If the scrolling reaches the top or bottom limit, any queued events to scroll further in the same direction become no-ops, and the appropriate scrollbar button is disabled.

The "display lag" in this example corresponds to the "network latency", the "disabled scrollbar button" corresponds to the removal of the "up" / "down" / "fill gap" buttons, and the scrolling events becoming no-ops corresponds to the deletion of the event queue upon hitting the limit.

... feel free to merge your patch in. It's collaborative code and is there to help those who contribute to it.

According to Contributing Code Or Enhancements To The Fossil Project:

The Fossil Architect (Richard Hipp) will merge changes onto the trunk.

I consider my small enhancement as non-invasive and non-harmful to your work (most people may not notice and/or not care about it). But if you, as one of the deputies of the Fossil Architect, and Owning Implementor of the JS UI part in question, can't give me your explicit and positive permission to merge it, I won't do so.

You are of course free to have non-technical objections. But I think it's a bit of a pity you didn't at least try it out.

Diff context loading was added at Richard's request, not because it scratched a personal itch. It's not something i'm likely to make much use of. (That applies to many things fossil can do: i've also never used it over ssh, nor used the "extras" or "clean" commands.)

But if you, as one of the deputies of the Fossil Architect, and Owning Implementor of the JS UI part in question, can't give me your explicit and positive permission to merge it, I won't do so.

Richard's traveling for a few days, with limited internet access. FWIW, you have my blessing to merge it whenever you're ready to do so and i have no reason at all to believe that Richard would object.

My hope was to get this in for technical reasons -- not for my tenacity! But thanks for your blessing, anyway.

At the moment it feels like I'm the only one who wants that feature (which may be okay), but also the only one who thinks it's at least "non-inferior" to the current implementation. But, people can still post feedback after merging, and backout is possible at any time, should it turn out to be a flop (I can't do mobile testing, for example).

I haven't done any tests on non-Windows webkit browsers, this is something I'd like to do, first (in the next days).

Also, is the approach to prevent the jumping buttons by duplicating them acceptable? (The jumping buttons are also in the current trunk version.) Technically/economically, this could be simplified to change just the CSS class and click handlers, instead of recreating the buttons -- but this would be more code, and I don't know how to replace an arrow function click handler?

My hope was to get this in for technical reasons

You did: no strong technical objections have been raised. If you like, i'll merge it, but it's better for you if you merge it so that the checkin counts towards your stats instead of mine.

Also, is the approach to prevent the jumping buttons by duplicating them acceptable? (The jumping buttons are also in the current trunk version.)

My instinct is to leave that as is, rather than complicate it, because it's arguably to be expected that the buttons move around in that case.

I don't know how to replace an arrow function click handler?

Replacing an event handler requires either keeping the original handler so that it can be removed later or swapping out the internals of the original handler depending on context. Both sound like more trouble than they're worth for this case. My recommendation is to simply leave that "bug" in place, and if it bothers people too much we can revisit it in "version 2."

My instinct is to leave that as is, rather than complicate it, because it's arguably to be expected that the buttons move around in that case.

What about adding one button during diff separator initialization to get the lighter look, but two buttons during maybeReplaceButtons(), while the user may be in the course of clicking?

Building and testing on Ubuntu is not something I'm (always) able to do quickly, unfortunately. I have my notes how to install the build tools, etc. but right now I see build errors that are new to me, with some pikchr math functions, so I may need to find another library, first.

Resolving this, and then installing Chromium and do the testing, seems to take longer than what I can quickly do right now.

I'm likely to get around to testing with Chromium on Ubuntu in the next days, however.

Anyway, I'd prefer you merging it, and then being the first mobile tester, so you can backout immediately if it's a showstopper on mobile. And -- you have more experience with Fossil JS distribution -- if you think the extra Ubuntu/Chromium testing is not strictly necessary, you can also merge without waiting for my test results.

What about adding one button during diff separator initialization to get the lighter look, but two buttons during maybeReplaceButtons(), while the user may be in the course of clicking?

i'm not currently convinced that the extra effort is justified. It seems like a lot of hassle for pedantic levels of usability benefit. That said: he who writes the code gets to decide :).

Anyway, I'd prefer you merging it, and then being the first mobile tester, ...

i will do so in just a few moments, as soon as i get libfossil back in a compileable state (porting in fossil's new diff builders). If you don't hear back from me in this thread within the next couple of hours then everything went just fine.

i'm going to leave out the button-jumping workaround for now because that solution currently feels weird to me and would look like a bug to me if i were to see it happen without understanding what it's for, but the click queue will be cherry-picked.

... but the click queue will be cherry-picked.

Just FYI, i'm going to change a couple of the bind() calls:

fOpt.onerror = function(err){
        this.msg(true,err.message);
        this.$fetchQueue = [];
      }.bind(this);

"The problem" (for lack of a better word) is that the API which will call onerror() is explicitly documented as passing a well-defined this to that routine, and bind() redefines/violates that, which can cause more confusion on the maintaining developer's part than the historical JS approach of:

const self = this;
fOpt.onerror = function(err){
        self.msg(true,err.message);
        self.$fetchQueue = [];
      };

Even though i write JS very often, i had to double-check which "this" was going to be applied in the bind() construct because i was not sure how bind() interacts/overrides calls made to the function via apply(). (IMO, bind's behavior is wrong and confusing, but EMCA didn't ask my opinion on it!)

For completeness's sake, here's the output of my double-checking it, with the console output preceded by >:

const onerror = function(){console.debug("this =",this)};
const opt = {x:1, onerror};
opt.onerror();

> this = Object { x: 1, onerror: onerror() }

opt.onerror = onerror.bind({x:2});
opt.onerror();

> this = Object { x: 2 }

// This is where bind is "wrong", in that it violates onerror()'s API:

opt.onerror.call(opt); // explicitly set 'this' to opt, but:

> Object { x: 2 }

opt.onerror.apply(opt); // explicitly set 'this' to opt, but:

> Object { x: 2 }

opt.x

> 1

This time building on Ubuntu worked smoothly as usual with Fossil, not sure what went wrong yesterday. Maybe I should add a disk drive to my Ubuntu VM, so I don't need start from scratch each time. ;-)

My tests with Chromium on Ubuntu were OK, so far. I'm always a bit anxious about pushing commits with JS code, as each and every device/OS/browser combination seems to have it's own 1'000 ways how things could go awry. So I will do more tests over the next days, for somehow I don't fully trust this JS world (regarding cross-platform compatibility, that is).

Just FYI, i'm going to change a couple of the bind() calls:

Me: ... [my code is] non-invasive and non-harmful to your work ...
You: Maintenance nightmare!

I'm sorry :(

I had to look up where the arrow function handler gets its this from, and noted the one passed to onerror() is ignored. I think there's no other choice with arrow functions, but doing this explicitly violates the API contract. Thanks for fixing this.

i'm going to leave out the button-jumping workaround for now because that solution currently feels weird to me and would look like a bug to me if i were to see it happen without understanding what it's for ...

Yeah, it's an "expected and friendly quirk", somehow. Maybe somebody has a nice idea how to solve this, one day.

I forgot to ask about the "Fetching diff chunk..." message, and noticed you restored it. I think that's fine, "the best of both worlds", i.e. immediate feedback that asynchronous work was started, and daredevils can deliver their clicks as fast as they want to get "page-wise" loading.

Thanks for your support!

I already found a reproducible failure :(

I found it with my modified version -- but it also happens with your original version, i.e. the one currently active on fossil-scm.org:

• https://fossil-scm.org/home/fdiff?v1=fd15b5f29e&v2=29561f5fee
• Click the very first "Up Next" button as many times as required to load all the available diff context, until the top of the file(s) is reached.
• The completely loaded diff context starts at line number 2, not 1.

I probably only can get to investigate this tomorrow ...

I've pushed some fixes for the diff context loading.

@Stephan: May I ask you to take a look at this when you get a chance, make any adjustments to match the Fossil JS framework API contracts and style, and merge it? Thanks!

@Stephan: May I ask you to take a look at this when you get a chance, make any adjustments to match the Fossil JS framework API contracts and style, and merge it? Thanks!

Done! Thank you for the fixes!

for somehow I don't fully trust this JS world (regarding cross-platform compatibility, that is).

It has improved a lot the past 10 or 12 years in that regard. Sneaky incompatibilities are relatively rare nowadays compared to 15 years ago.

You: Maintenance nightmare!

"Nightmare" is too strong of a word, but definitely a point of potential confusion later (very likely my own!). Your implementation did work exactly as it should have. There was not a definite problem there, just me being over-cautious about avoiding one. That said: i will make an effort to come to grips with the implications of bind().

Thanks for your support!

By all means - this is what i do for fun.

After using it for a while, I need to say the feature is really nifty! Good work, Stephan!

I glimpsed over to GitHub, and found their dynamic diff context loader buttons also have click-ahead queues ;-)

But I noticed I use the feature far less frequently on GitHub -- because they show the name of the current function next to the @@ ±w,x ±y,z @@ chunk headers, so I already have something to jump to. I've seen this with other systems (maybe some diff tool, or maybe some other VCS) too, with a (customizable) regular expression used to match the most recent function declaration. That may be a good use of the empty space where the "Fetching diff chunk..." message appears (but where to put the errors, then?) -- but this feature (automated function declaration lookup) seems somewhat fragile, and may not work with scripts, makefiles, and other non-C auxiliary files, at all.

i'm going to leave out the button-jumping workaround for now because that solution currently feels weird to me and would look like a bug to me if i were to see it happen without understanding what it's for ...

Yeah, it's an "expected and friendly quirk", somehow. Maybe somebody has a nice idea how to solve this, one day.

What about the following: swap positions (left/right) of the Up/Down buttons if they're both available. If the Up button is on the left, the combined Fill Gap button will appear in the same position, without jumping any more. For the Down button, this is not a problem, as that one is jumping away with every click, anyway. (On GitHub, this problem is mitigated by stacking the Up/Down buttons.)

I've seen this with other systems (maybe some diff tool, or maybe some other VCS) too, with a (customizable) regular expression used to match the most recent function declaration.

github has tons of backend services to handle that type of thing. The content fossil is showing is "opaque" to it - it has no idea that it's source code or what type of source code it may be.

What about the following: swap positions (left/right) of the Up/Down buttons if they're both available. If the Up button is on the left, the combined Fill Gap button will appear in the same position, without jumping any more.

That's a good idea and trivial to change. i'll try that out in a few moments.

(On GitHub, this problem is mitigated by stacking the Up/Down buttons.)

Ours initially used that layout but i didn't like it in practice so replaced it.

github has tons of backend services to handle that type of thing.

That's also my impression, and keeping track of this information during diff generation seems complex. It's pity I don't remember where I've seen this (I've tried all my diff viewers/tools at hand).

That's a good idea and trivial to change.

Thanks! I think it feels "natural" -- at least to people used to left-to-right flows.

because they show the name of the current function next to the @@ ±w,x ±y,z @@ chunk headers

That's the -p option to diff(1), and is something I want to add to fnc's diff view. It's really only accurate with C source code files, but that's what I spend the vast majority of my time in.

ETA: quoted post for reference

Wow, I'm no longer familiar with this utility, and missed the option from diff --help output. But from the linked man page, the algorithm sounds way simpler than I expected.

GitHub ... show[s] the name of the current function next to the @@ ±w,x ±y,z @@ chunk headers

In my experience, that "function name" is wrong about as often as it is right.

Together, the dom and fetch rewrites amount to less than 1.5 KB of Javascript code to replace two 28 KB and 9 KB Javascript framework libraries. ;-)

I just wanted to let the world know that I have a private build of Fossil with dynamic diff context loading in IE ;-)

You've piqued my interest, florian@

Out of curiosity from a former, longtime noscript user, would you mind
sharing your local patches?

Sure, but I'm away from my main machine for 1 or 2 days, where I have this stuff.

Perfect! Thanks. And no rush—whenever you can will be great.

That's what I've got so far. I put it aside when both diff context loading and sync'ed / keyboard scrolling worked, pending some TODOs and cleanup. It's based on Fossil version [9956fa6dde], so not the most recent trunk.

• https://pastebin.com/Cv1hbpZq
• https://pastebin.com/gzKJSXFu

The block comments have been removed, so it was easier to find errors displayed in the browser console, as Fossil also strips comments from HTTP-served JS files. I intend to bring back the comments for a simpler diff.

The JS part is about replacing some modern ECMAScript with traditional Javascript, so mostly const, forEach(), and arrow functions.

The C part is required for some support to pass the diff context chunk size to the client (this could be simplified), and to make sure the JS is loaded without any dependencies.

I hope you're not disappointed, it's far away from a <noscript> version, but just some backporting to make the JS work in IE.

Thanks, florian@. I'm presently occupied with work so won't have a chance to
patch my fossil till later tonight but I appreciate the diffs.

I hope you're not disappointed, it's far away from a <noscript> version, but just some backporting to make the JS work in IE.

It's impossible to disappoint in this respect; I've no JS skills to speak of.
You piqued my interest talking about running the new diff with less JS. And
given I might have to learn JS at some point, It'll be good to see different
approaches.