Fossil SCM

Implemented fetching and injection of chunks which are smaller than the configured load size, but the results do not play well with our scrolling workaround and need to be revisited after some sleep, perhaps appending/prepending the results directly to the previous/next TR instead of injecting a new one.

stephan 2021-09-09 21:36 diff-js-refactoring
Commit da8a0f82b5d5638a39bb72206a2a9c6ccaadc4b8ef392297cefaafb72847caea
--- src/default.css
+++ src/default.css
@@ -544,10 +544,13 @@
544544
table.diff td {
545545
vertical-align: top;
546546
}
547547
table.diff pre {
548548
margin: 0 0 0 0;
549
+}
550
+table.diff tr.fetched {
551
+ opacity: 0.7;
549552
}
550553
tr.diffskip.jchunk {
551554
/* jchunk gets added from JS to diffskip rows when they are
552555
plugged into the /jchunk route and removed after that data
553556
is fetched. */
554557
--- src/default.css
+++ src/default.css
@@ -544,10 +544,13 @@
544 table.diff td {
545 vertical-align: top;
546 }
547 table.diff pre {
548 margin: 0 0 0 0;
 
 
 
549 }
550 tr.diffskip.jchunk {
551 /* jchunk gets added from JS to diffskip rows when they are
552 plugged into the /jchunk route and removed after that data
553 is fetched. */
554
--- src/default.css
+++ src/default.css
@@ -544,10 +544,13 @@
544 table.diff td {
545 vertical-align: top;
546 }
547 table.diff pre {
548 margin: 0 0 0 0;
549 }
550 table.diff tr.fetched {
551 opacity: 0.7;
552 }
553 tr.diffskip.jchunk {
554 /* jchunk gets added from JS to diffskip rows when they are
555 plugged into the /jchunk route and removed after that data
556 is fetched. */
557
+108 -24
--- src/fossil.diff.js
+++ src/fossil.diff.js
@@ -217,14 +217,17 @@
217217
Each instance corresponds to a single TR.diffskip element.
218218
*/
219219
Diff.ChunkLoadControls = function(isSplit, tr){
220220
this.isSplit = isSplit;
221221
this.e = {/*DOM elements*/
222
- tr: tr
222
+ tr: tr,
223
+ table: tr.parentElement/*TBODY*/.parentElement
223224
};
225
+ this.fileHash = this.e.table.dataset.lefthash;
224226
tr.$chunker = this /* keep GC from reaping this */;
225227
this.pos = {
228
+ //hash: F.hashDigits(this.fileHash),
226229
/* These line numbers correspond to the LHS file. Because the
227230
contents are common to both sides, we have the same number
228231
for the RHS, but need to extract those line numbers from the
229232
neighboring TR blocks */
230233
startLhs: +tr.dataset.startln,
@@ -260,15 +263,22 @@
260263
endLhs: extractLineNo(true, false, tr.previousElementSibling, isSplit),
261264
endRhs: extractLineNo(false, false, tr.previousElementSibling, isSplit)
262265
};
263266
}
264267
let btnUp = false, btnDown = false;
265
- if(this.pos.prev && this.pos.next
266
- && ((this.pos.next.startLhs - this.pos.prev.endLhs)
267
- <= Diff.config.chunkLoadLines)){
268
+ /**
269
+ this.pos.next refers to the line numbers in the next TR's chunk.
270
+ this.pos.prev refers to the line numbers in the previous TR's chunk.
271
+ */
272
+ if((this.pos.startLhs + Diff.config.chunkLoadLines
273
+ >= this.pos.endLhs )
274
+ || (this.pos.prev && this.pos.next
275
+ && ((this.pos.next.startLhs - this.pos.prev.endLhs)
276
+ <= Diff.config.chunkLoadLines))){
268277
/* Place a single button to load the whole block, rather
269278
than separate up/down buttons. */
279
+ //delete this.pos.next;
270280
btnDown = false;
271281
btnUp = D.append(
272282
D.addClass(D.span(), 'button', 'up', 'down'),
273283
D.span(/*glyph holder*/)
274284
//D.append(D.span(), this.config.glyphDown, this.config.glyphUp)
@@ -319,51 +329,125 @@
319329
}
320330
return this;
321331
},
322332
323333
destroy: function(){
324
- delete this.tr.$chunker;
325334
D.remove(this.e.tr);
335
+ delete this.e.tr.$chunker;
336
+ delete this.e.tr;
326337
delete this.e;
327338
delete this.pos;
328339
},
329340
/**
330
- Creates and returns a new TR element, including its TD elements (depending
331
- on this.isSplit), but does not fill it with any information nor inject it
332
- into the table (it doesn't know where to do so).
341
+ Creates a new TR element, including its TD elements (depending
342
+ on this.isSplit), but does not fill it with any information nor
343
+ inject it into the table (it doesn't know where to do
344
+ so). Returns an object containing the TR element and various TD
345
+ elements which will likely be needed by the routine which
346
+ called this. See this code for details.
333347
*/
334348
newTR: function(){
335
- const tr = D.tr();
336
- if(this.isSplit){
337
- D.append(D.addClass( D.td(tr), 'diffln', 'difflnl' ), D.pre());
338
- D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtl' ), D.pre());
339
- D.addClass( D.td(tr), 'diffsep' );
340
- D.append(D.addClass( D.td(tr), 'diffln', 'difflnr' ), D.pre());
341
- D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtr' ), D.pre());
342
- }else{
343
- D.append(D.addClass( D.td(tr), 'diffln', 'difflnl' ), D.pre());
344
- D.append(D.addClass( D.td(tr), 'diffln', 'difflnr' ), D.pre());
345
- D.addClass( D.td(tr), 'diffsep' );
346
- D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtu' ), D.pre());
347
- }
348
- return tr;
349
+ const tr = D.addClass(D.tr(),'fetched'), rc = {
350
+ tr,
351
+ preLnL: D.pre(),
352
+ preLnR: D.pre()
353
+ };
354
+ if(this.isSplit){
355
+ D.append(D.addClass( D.td(tr), 'diffln', 'difflnl' ), rc.preLnL);
356
+ rc.preTxtL = D.pre();
357
+ D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtl' ), rc.preTxtL);
358
+ D.addClass( D.td(tr), 'diffsep' );
359
+ D.append(D.addClass( D.td(tr), 'diffln', 'difflnr' ), rc.preLnR);
360
+ rc.preTxtR = D.pre();
361
+ D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtr' ), rc.preTxtR);
362
+ }else{
363
+ D.append(D.addClass( D.td(tr), 'diffln', 'difflnl' ), rc.preLnL);
364
+ D.append(D.addClass( D.td(tr), 'diffln', 'difflnr' ), rc.preLnR);
365
+ D.addClass( D.td(tr), 'diffsep' );
366
+ rc.preTxtU = D.pre();
367
+ D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtu' ), rc.preTxtU);
368
+ }
369
+ return rc;
370
+ },
371
+
372
+ injectResponse: function(direction/*as for fetchChunk()*/,
373
+ urlParam/*from fetchChunk()*/,
374
+ lines/*response lines*/){
375
+ console.debug("Loading line range ",urlParam.from,"-",urlParam.to);
376
+ const row = this.newTR();
377
+ const lineno = [];
378
+ let i;
379
+ for( i = urlParam.from; i <= urlParam.to; ++i ){
380
+ /* TODO: space-pad numbers, but we don't know the proper length from here. */
381
+ lineno.push(i);
382
+ }
383
+ row.preLnL.innerText = lineno.join('\n')+'\n';
384
+ if(row.preTxtU){//unified diff
385
+ row.preTxtU.innerText = lines.join('\n')+'\n';
386
+ }else{//split diff
387
+ const code = lines.join('\n')+'\n';
388
+ row.preTxtL.innerText = code;
389
+ row.preTxtR.innerText = code;
390
+ }
391
+ if(0===direction){
392
+ /* Closing the whole gap between two chunks or a whole gap
393
+ at the start or end of a diff. */
394
+ let startLnR = this.pos.prev
395
+ ? this.pos.prev.endRhs+1 /* Closing the whole gap between two chunks
396
+ or end-of-file gap. */
397
+ : this.pos.next.startRhs - lines.length /* start-of-file gap */;
398
+ lineno.length = 0;
399
+ for( i = startLnR; i < startLnR + lines.length; ++i ){
400
+ /* TODO? space-pad numbers, but we don't know the proper length from here. */
401
+ lineno.push(i);
402
+ }
403
+ row.preLnR.innerText = lineno.join('\n')+'\n';
404
+ this.e.tr.parentNode.insertBefore(row.tr, this.e.tr);
405
+ Diff.initTableDiff(this.e.table/*fix scrolling*/).checkTableWidth(true);
406
+ this.destroy();
407
+ return this;
408
+ }else{
409
+ console.debug("TODO: handle load of partial next/prev");
410
+ this.updatePosDebug();
411
+ }
349412
},
350413
351414
fetchChunk: function(direction/*-1=prev, 1=next, 0=both*/){
352415
/* Forewarning, this is a bit confusing: when fetching the
353416
previous lines, we're doing so on behalf of the *next* diff
354417
chunk (this.pos.next), and vice versa. */
355
- if(this.isFetching) return this;
418
+ if(this.$isFetching){
419
+ console.debug("Cannot load chunk while a load is pending.");
420
+ return this;
421
+ }
356422
if(direction<0/*prev chunk*/ && !this.pos.next){
357423
console.error("Attempt to fetch previous diff lines but don't have any.");
358424
return this;
359425
}else if(direction>0/*next chunk*/ && !this.pos.prev){
360426
console.error("Attempt to fetch next diff lines but don't have any.");
361427
return this;
362428
}
363429
console.debug("Going to fetch in direction",direction);
364
- this.updatePosDebug();
430
+ const fOpt = {
431
+ urlParams:{
432
+ name: this.fileHash, from: 0, to: 0
433
+ },
434
+ aftersend: ()=>delete this.$isFetching
435
+ };
436
+ const self = this;
437
+ if(direction!=0){
438
+ console.debug("Skipping fetch for now.");
439
+ return this;
440
+ }else{
441
+ fOpt.urlParams.from = this.pos.startLhs;
442
+ fOpt.urlParams.to = this.pos.endLhs;
443
+ fOpt.onload = function(list){
444
+ self.injectResponse(direction,fOpt.urlParams,list);
445
+ };
446
+ }
447
+ this.$isFetching = true;
448
+ Diff.fetchArtifactChunk(fOpt);
365449
return this;
366450
}
367451
};
368452
369453
Diff.addDiffSkipHandlers = function(){
370454
--- src/fossil.diff.js
+++ src/fossil.diff.js
@@ -217,14 +217,17 @@
217 Each instance corresponds to a single TR.diffskip element.
218 */
219 Diff.ChunkLoadControls = function(isSplit, tr){
220 this.isSplit = isSplit;
221 this.e = {/*DOM elements*/
222 tr: tr
 
223 };
 
224 tr.$chunker = this /* keep GC from reaping this */;
225 this.pos = {
 
226 /* These line numbers correspond to the LHS file. Because the
227 contents are common to both sides, we have the same number
228 for the RHS, but need to extract those line numbers from the
229 neighboring TR blocks */
230 startLhs: +tr.dataset.startln,
@@ -260,15 +263,22 @@
260 endLhs: extractLineNo(true, false, tr.previousElementSibling, isSplit),
261 endRhs: extractLineNo(false, false, tr.previousElementSibling, isSplit)
262 };
263 }
264 let btnUp = false, btnDown = false;
265 if(this.pos.prev && this.pos.next
266 && ((this.pos.next.startLhs - this.pos.prev.endLhs)
267 <= Diff.config.chunkLoadLines)){
 
 
 
 
 
 
268 /* Place a single button to load the whole block, rather
269 than separate up/down buttons. */
 
270 btnDown = false;
271 btnUp = D.append(
272 D.addClass(D.span(), 'button', 'up', 'down'),
273 D.span(/*glyph holder*/)
274 //D.append(D.span(), this.config.glyphDown, this.config.glyphUp)
@@ -319,51 +329,125 @@
319 }
320 return this;
321 },
322
323 destroy: function(){
324 delete this.tr.$chunker;
325 D.remove(this.e.tr);
 
 
326 delete this.e;
327 delete this.pos;
328 },
329 /**
330 Creates and returns a new TR element, including its TD elements (depending
331 on this.isSplit), but does not fill it with any information nor inject it
332 into the table (it doesn't know where to do so).
 
 
 
333 */
334 newTR: function(){
335 const tr = D.tr();
336 if(this.isSplit){
337 D.append(D.addClass( D.td(tr), 'diffln', 'difflnl' ), D.pre());
338 D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtl' ), D.pre());
339 D.addClass( D.td(tr), 'diffsep' );
340 D.append(D.addClass( D.td(tr), 'diffln', 'difflnr' ), D.pre());
341 D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtr' ), D.pre());
342 }else{
343 D.append(D.addClass( D.td(tr), 'diffln', 'difflnl' ), D.pre());
344 D.append(D.addClass( D.td(tr), 'diffln', 'difflnr' ), D.pre());
345 D.addClass( D.td(tr), 'diffsep' );
346 D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtu' ), D.pre());
347 }
348 return tr;
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
349 },
350
351 fetchChunk: function(direction/*-1=prev, 1=next, 0=both*/){
352 /* Forewarning, this is a bit confusing: when fetching the
353 previous lines, we're doing so on behalf of the *next* diff
354 chunk (this.pos.next), and vice versa. */
355 if(this.isFetching) return this;
 
 
 
356 if(direction<0/*prev chunk*/ && !this.pos.next){
357 console.error("Attempt to fetch previous diff lines but don't have any.");
358 return this;
359 }else if(direction>0/*next chunk*/ && !this.pos.prev){
360 console.error("Attempt to fetch next diff lines but don't have any.");
361 return this;
362 }
363 console.debug("Going to fetch in direction",direction);
364 this.updatePosDebug();
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
365 return this;
366 }
367 };
368
369 Diff.addDiffSkipHandlers = function(){
370
--- src/fossil.diff.js
+++ src/fossil.diff.js
@@ -217,14 +217,17 @@
217 Each instance corresponds to a single TR.diffskip element.
218 */
219 Diff.ChunkLoadControls = function(isSplit, tr){
220 this.isSplit = isSplit;
221 this.e = {/*DOM elements*/
222 tr: tr,
223 table: tr.parentElement/*TBODY*/.parentElement
224 };
225 this.fileHash = this.e.table.dataset.lefthash;
226 tr.$chunker = this /* keep GC from reaping this */;
227 this.pos = {
228 //hash: F.hashDigits(this.fileHash),
229 /* These line numbers correspond to the LHS file. Because the
230 contents are common to both sides, we have the same number
231 for the RHS, but need to extract those line numbers from the
232 neighboring TR blocks */
233 startLhs: +tr.dataset.startln,
@@ -260,15 +263,22 @@
263 endLhs: extractLineNo(true, false, tr.previousElementSibling, isSplit),
264 endRhs: extractLineNo(false, false, tr.previousElementSibling, isSplit)
265 };
266 }
267 let btnUp = false, btnDown = false;
268 /**
269 this.pos.next refers to the line numbers in the next TR's chunk.
270 this.pos.prev refers to the line numbers in the previous TR's chunk.
271 */
272 if((this.pos.startLhs + Diff.config.chunkLoadLines
273 >= this.pos.endLhs )
274 || (this.pos.prev && this.pos.next
275 && ((this.pos.next.startLhs - this.pos.prev.endLhs)
276 <= Diff.config.chunkLoadLines))){
277 /* Place a single button to load the whole block, rather
278 than separate up/down buttons. */
279 //delete this.pos.next;
280 btnDown = false;
281 btnUp = D.append(
282 D.addClass(D.span(), 'button', 'up', 'down'),
283 D.span(/*glyph holder*/)
284 //D.append(D.span(), this.config.glyphDown, this.config.glyphUp)
@@ -319,51 +329,125 @@
329 }
330 return this;
331 },
332
333 destroy: function(){
 
334 D.remove(this.e.tr);
335 delete this.e.tr.$chunker;
336 delete this.e.tr;
337 delete this.e;
338 delete this.pos;
339 },
340 /**
341 Creates a new TR element, including its TD elements (depending
342 on this.isSplit), but does not fill it with any information nor
343 inject it into the table (it doesn't know where to do
344 so). Returns an object containing the TR element and various TD
345 elements which will likely be needed by the routine which
346 called this. See this code for details.
347 */
348 newTR: function(){
349 const tr = D.addClass(D.tr(),'fetched'), rc = {
350 tr,
351 preLnL: D.pre(),
352 preLnR: D.pre()
353 };
354 if(this.isSplit){
355 D.append(D.addClass( D.td(tr), 'diffln', 'difflnl' ), rc.preLnL);
356 rc.preTxtL = D.pre();
357 D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtl' ), rc.preTxtL);
358 D.addClass( D.td(tr), 'diffsep' );
359 D.append(D.addClass( D.td(tr), 'diffln', 'difflnr' ), rc.preLnR);
360 rc.preTxtR = D.pre();
361 D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtr' ), rc.preTxtR);
362 }else{
363 D.append(D.addClass( D.td(tr), 'diffln', 'difflnl' ), rc.preLnL);
364 D.append(D.addClass( D.td(tr), 'diffln', 'difflnr' ), rc.preLnR);
365 D.addClass( D.td(tr), 'diffsep' );
366 rc.preTxtU = D.pre();
367 D.append(D.addClass( D.td(tr), 'difftxt', 'difftxtu' ), rc.preTxtU);
368 }
369 return rc;
370 },
371
372 injectResponse: function(direction/*as for fetchChunk()*/,
373 urlParam/*from fetchChunk()*/,
374 lines/*response lines*/){
375 console.debug("Loading line range ",urlParam.from,"-",urlParam.to);
376 const row = this.newTR();
377 const lineno = [];
378 let i;
379 for( i = urlParam.from; i <= urlParam.to; ++i ){
380 /* TODO: space-pad numbers, but we don't know the proper length from here. */
381 lineno.push(i);
382 }
383 row.preLnL.innerText = lineno.join('\n')+'\n';
384 if(row.preTxtU){//unified diff
385 row.preTxtU.innerText = lines.join('\n')+'\n';
386 }else{//split diff
387 const code = lines.join('\n')+'\n';
388 row.preTxtL.innerText = code;
389 row.preTxtR.innerText = code;
390 }
391 if(0===direction){
392 /* Closing the whole gap between two chunks or a whole gap
393 at the start or end of a diff. */
394 let startLnR = this.pos.prev
395 ? this.pos.prev.endRhs+1 /* Closing the whole gap between two chunks
396 or end-of-file gap. */
397 : this.pos.next.startRhs - lines.length /* start-of-file gap */;
398 lineno.length = 0;
399 for( i = startLnR; i < startLnR + lines.length; ++i ){
400 /* TODO? space-pad numbers, but we don't know the proper length from here. */
401 lineno.push(i);
402 }
403 row.preLnR.innerText = lineno.join('\n')+'\n';
404 this.e.tr.parentNode.insertBefore(row.tr, this.e.tr);
405 Diff.initTableDiff(this.e.table/*fix scrolling*/).checkTableWidth(true);
406 this.destroy();
407 return this;
408 }else{
409 console.debug("TODO: handle load of partial next/prev");
410 this.updatePosDebug();
411 }
412 },
413
414 fetchChunk: function(direction/*-1=prev, 1=next, 0=both*/){
415 /* Forewarning, this is a bit confusing: when fetching the
416 previous lines, we're doing so on behalf of the *next* diff
417 chunk (this.pos.next), and vice versa. */
418 if(this.$isFetching){
419 console.debug("Cannot load chunk while a load is pending.");
420 return this;
421 }
422 if(direction<0/*prev chunk*/ && !this.pos.next){
423 console.error("Attempt to fetch previous diff lines but don't have any.");
424 return this;
425 }else if(direction>0/*next chunk*/ && !this.pos.prev){
426 console.error("Attempt to fetch next diff lines but don't have any.");
427 return this;
428 }
429 console.debug("Going to fetch in direction",direction);
430 const fOpt = {
431 urlParams:{
432 name: this.fileHash, from: 0, to: 0
433 },
434 aftersend: ()=>delete this.$isFetching
435 };
436 const self = this;
437 if(direction!=0){
438 console.debug("Skipping fetch for now.");
439 return this;
440 }else{
441 fOpt.urlParams.from = this.pos.startLhs;
442 fOpt.urlParams.to = this.pos.endLhs;
443 fOpt.onload = function(list){
444 self.injectResponse(direction,fOpt.urlParams,list);
445 };
446 }
447 this.$isFetching = true;
448 Diff.fetchArtifactChunk(fOpt);
449 return this;
450 }
451 };
452
453 Diff.addDiffSkipHandlers = function(){
454

Keyboard Shortcuts

Open search /
Next entry (timeline) j
Previous entry (timeline) k
Open focused entry Enter
Show this help ?
Toggle theme Top nav button