Fossil SCM

Fetching of /jchunk lines by clicking on the '...' separator of a diff is now working but the fetched lines still need to be integrated into the UI.

stephan 2021-09-09 03:23 diff-js-refactoring

Commit 41ef416e778ea9c3c85e9fa0e335823a600cf6b25b0a8033c8505032f96729ad

Parent 1fec5f4abc3b2a4…

4 files changed +1 -1 +7 +143 -15 +6 -1

~ src/builtin.c ~ src/default.css ~ src/fossil.diff.js ~ src/info.c

M src/builtin.c

+1 -1

		--- src/builtin.c
		+++ src/builtin.c
		@@ -702,11 +702,11 @@
702	702	** the final one! */
703	703	} fjs[] = {
704	704	/* This list ordering isn't strictly important. */
705	705	{"confirmer", 0, 0},
706	706	{"copybutton", 0, "dom\0"},
707		- {"diff", 0, "dom\0fetch\0"},
	707	+ {"diff", 0, "dom\0fetch\0popupwidget\0"},
708	708	{"dom", 0, 0},
709	709	{"fetch", 0, 0},
710	710	{"info-diff", 0, "dom\0"},
711	711	{"numbered-lines", 0, "popupwidget\0copybutton\0"},
712	712	{"pikchr", 0, "dom\0"},
713	713

	--- src/builtin.c
	+++ src/builtin.c
	@@ -702,11 +702,11 @@
702	** the final one! */
703	} fjs[] = {
704	/* This list ordering isn't strictly important. */
705	{"confirmer", 0, 0},
706	{"copybutton", 0, "dom\0"},
707	{"diff", 0, "dom\0fetch\0"},
708	{"dom", 0, 0},
709	{"fetch", 0, 0},
710	{"info-diff", 0, "dom\0"},
711	{"numbered-lines", 0, "popupwidget\0copybutton\0"},
712	{"pikchr", 0, "dom\0"},
713

	--- src/builtin.c
	+++ src/builtin.c
	@@ -702,11 +702,11 @@
702	** the final one! */
703	} fjs[] = {
704	/* This list ordering isn't strictly important. */
705	{"confirmer", 0, 0},
706	{"copybutton", 0, "dom\0"},
707	{"diff", 0, "dom\0fetch\0popupwidget\0"},
708	{"dom", 0, 0},
709	{"fetch", 0, 0},
710	{"info-diff", 0, "dom\0"},
711	{"numbered-lines", 0, "popupwidget\0copybutton\0"},
712	{"pikchr", 0, "dom\0"},
713

M src/default.css

		--- src/default.css
		+++ src/default.css
		@@ -542,10 +542,17 @@
542	542	table.diff td {
543	543	vertical-align: top;
544	544	}
545	545	table.diff pre {
546	546	margin: 0 0 0 0;
	547	+}
	548	+tr.diffskip.jchunk:hover {
	549	+ /* jchunk gets added from JS to diffskip rows when they are
	550	+ plugged into the /jchunk route and removed after that data
	551	+ is fetched. */
	552	+ background-color: rgba(127,127,127,0.5);
	553	+ cursor: pointer;
547	554	}
548	555	td.diffln {
549	556	width: 1px;
550	557	text-align: right;
551	558	padding: 0 1em 0 0;
552	559

	--- src/default.css
	+++ src/default.css
	@@ -542,10 +542,17 @@
542	table.diff td {
543	vertical-align: top;
544	}
545	table.diff pre {
546	margin: 0 0 0 0;







547	}
548	td.diffln {
549	width: 1px;
550	text-align: right;
551	padding: 0 1em 0 0;
552

	--- src/default.css
	+++ src/default.css
	@@ -542,10 +542,17 @@
542	table.diff td {
543	vertical-align: top;
544	}
545	table.diff pre {
546	margin: 0 0 0 0;
547	}
548	tr.diffskip.jchunk:hover {
549	/* jchunk gets added from JS to diffskip rows when they are
550	plugged into the /jchunk route and removed after that data
551	is fetched. */
552	background-color: rgba(127,127,127,0.5);
553	cursor: pointer;
554	}
555	td.diffln {
556	width: 1px;
557	text-align: right;
558	padding: 0 1em 0 0;
559

M src/fossil.diff.js

+143 -15

		--- src/fossil.diff.js
		+++ src/fossil.diff.js
		@@ -20,45 +20,173 @@
20	20	document.querySelectorAll('table.diff').forEach(addToggle);
21	21	});
22	22
23	23	window.fossil.onPageLoad(function(){
24	24	const F = window.fossil, D = F.dom;
25		-
	25	+ const Diff = F.diff = {
	26	+ config: {
	27	+ chunkLoadLines: 20,
	28	+ chunkFetch: {
	29	+ /* Default callack handlers for Diff.fetchArtifactChunk(),
	30	+ unless overridden by options passeed to that function. */
	31	+ beforesend: function(){},
	32	+ aftersend: function(){},
	33	+ onerror: function(e){
	34	+ F.toast.error("XHR error: ",e.message);
	35	+ }
	36	+ }
	37	+ }
	38	+ };
26	39	/**
27	40	Uses the /jchunk AJAX route to fetch specific lines of a given
28		- artifact. The first argument must be an Object with these
29		- properties:
	41	+ artifact. The argument must be an Object suitable for passing as
	42	+ the second argument to fossil.fetch(). Its urlParams property
	43	+ must be an object with these properties:
30	44
31	45	{
32	46	name: full hash of the target file,
33	47	from: first 1-based line number of the file to fetch (inclusive),
34	48	to: last 1-based line number of the file to fetch (inclusive)
35	49	}
36	50
37		- onload and onerror are optional callback functions to be called
38		- on success resp. error, as documented for window.fossil.fetch().
39		- Note that onload is ostensibly optional but this function is not
40		- of much use without an onload handler. Conversely, the default
41		- onerror handler is often customized on a per-page basis to send
42		- the error output somewhere where the user can see it.
	51	+ The fetchOpt object is NOT cloned for use by the call: it is used
	52	+ as-is and may be modified by this call. Thus callers "really
	53	+ should" pass a temporary object, not a long-lived one.
	54	+
	55	+ If fetchOpt does not define any of the (beforesend, aftersend,
	56	+ onerror) callbacks, the defaults from fossil.diff.config.chunkFetch
	57	+ are used, so any given client page may override those to provide
	58	+ page-level default handling.
	59	+
	60	+ Note that onload callback is ostensibly optional but this
	61	+ function is not of much use without an onload
	62	+ handler. Conversely, the default onerror handler is often
	63	+ customized on a per-page basis to send the error output somewhere
	64	+ where the user can see it.
43	65
44	66	The response, on success, will be an array of strings, each entry
45	67	being one line from the requested artifact. If the 'to' line is
46	68	greater than the length of the file, the array will be shorter
47	69	than (to-from) lines.
48	70
49	71	The /jchunk route reports errors via JSON objects with
50	72	an "error" string property describing the problem.
51	73
52		- This is an async operation. Returns this object.
	74	+ This is an async operation. Returns the fossil object.
	75	+ */
	76	+ Diff.fetchArtifactChunk = function(fetchOpt){
	77	+ if(!fetchOpt.beforesend) fetchOpt.beforesend = Diff.config.chunkFetch.beforesend;
	78	+ if(!fetchOpt.aftersend) fetchOpt.aftersend = Diff.config.chunkFetch.aftersend;
	79	+ if(!fetchOpt.onerror) fetchOpt.onerror = Diff.config.chunkFetch.onerror;
	80	+ fetchOpt.responseType = 'json';
	81	+ return F.fetch('jchunk', fetchOpt);
	82	+ };
	83	+
	84	+ /**
	85	+ Fetches /jchunk for the given TR element then replaces the TR's
	86	+ contents with data from the result of that request.
53	87	*/
54		- F.fetchArtifactLines = function(urlParams, onload, onerror){
55		- const opt = {urlParams};
56		- if(onload) opt.onload = onload;
57		- if(onerror) opt.onerror = onerror;
58		- return this.fetch('jchunk', opt);
	88	+ const fetchTrChunk = function(tr){
	89	+ if(tr.dataset.xfer /* already being fetched */) return;
	90	+ const table = tr.parentElement.parentElement;
	91	+ const hash = table.dataset.lefthash;
	92	+ if(!hash) return;
	93	+ const isSbs = table.classList.contains('splitdiff')/else udiff/;
	94	+ tr.dataset.xfer = 1 /* sentinel against multiple concurrent ajax requests */;
	95	+ const lineTo = +tr.dataset.endln;
	96	+ var lnFrom = +tr.dataset.startln;
	97	+ /* TODO: for the time being, for simplicity, we'll read the whole
	98	+ [startln, endln] chunk. "Later on" we'll maybe want to read it in
	99	+ chunks of, say, 20 lines or so, adjusting lnFrom to be 1 if it would
	100	+ be less than 1. */
	101	+ Diff.fetchArtifactChunk({
	102	+ urlParams:{
	103	+ name: hash,
	104	+ from: lnFrom,
	105	+ to: lineTo
	106	+ },
	107	+ aftersend: function(){
	108	+ delete tr.dataset.xfer;
	109	+ Diff.config.chunkFetch.aftersend.apply(
	110	+ this, Array.prototype.slice.call(arguments,0)
	111	+ );
	112	+ },
	113	+ onload: function(result){
	114	+ console.debug("Chunk result: ",result);
	115	+ D.clearElement(tr);
	116	+ D.append(
	117	+ D.attr(D.td(tr), 'colspan', isSbs ? 5 : 4),
	118	+ "Fetched chunk of ",result.length," line(s). TODO: insert it here."
	119	+ );
	120	+ /*
	121	+ At this point we need to:
	122	+
	123	+ - Read the previous TR, if any, to get the preceeding LHS/RHS
	124	+ line numbers so that we know where to start counting.
	125	+
	126	+ - If there is no previous TR, we're at the top and we
	127	+ instead need to get the LHS/RHS line numbers from the
	128	+ following TR's children.
	129	+
	130	+ - D.clearElement(tr) and insert columns appropriate for the
	131	+ parent table's diff type.
	132	+
	133	+ We can fish the line numbers out of the PRE columns with something
	134	+ like this inefficient but effective hack:
	135	+
	136	+ theElement.innerText.split(/\n+/)
	137	+
	138	+ (need /\n+/ instead of '\n' b/c of INS/DEL elements)
	139	+
	140	+ Noting that the result array will end with an empty element
	141	+ due to the trailing \n character, so a call to pop() will be
	142	+ needed.
	143	+
	144	+ SBS diff col layout:
	145	+ <td><pre>...LHS line numbers...</pre></td>
	146	+ <td>...code lines...</td>
	147	+ <td></td> empty for this case.
	148	+ <td><pre>...RHS line numbers...</pre></td>
	149	+ <td>...dupe of col 2</td>
	150	+
	151	+ Unified diff col layout:
	152	+ <td>LHS line numbers</td>
	153	+ <td>RHS line numbers</td>
	154	+ <td>blank in this case</td>
	155	+ <td>code line</td>
	156	+ */
	157	+ }
	158	+ });
	159	+ };
	160	+
	161	+ Diff.addDiffSkipHandlers = function(){
	162	+ const tables = document.querySelectorAll('table.diff[data-lefthash]');
	163	+ if(!tables.length) return F;
	164	+ const addDiffSkipToTr = function f(tr){
	165	+ D.addClass(tr, 'jchunk');
	166	+ if(!f._handler){
	167	+ f._handler = function ff(event){
	168	+ var e = event.target;
	169	+ while(e && 'TR' !== e.tagName) e = e.parentElement;
	170	+ if(!e){
	171	+ console.error("Internal event-handling error: didn't find TR target.");
	172	+ return;
	173	+ }
	174	+ e.removeEventListener('click',ff);
	175	+ D.removeClass(e, 'jchunk');
	176	+ //console.debug("addDiffSkipToTr() Event:",e, event);
	177	+ fetchTrChunk(e);
	178	+ };
	179	+ }
	180	+ tr.addEventListener('click', f._handler, false);
	181	+ };
	182	+ tables.forEach(function(t){
	183	+ t.querySelectorAll('tr.diffskip[data-startln]').forEach(addDiffSkipToTr);
	184	+ });
59	185	};
	186	+
	187	+ F.diff.addDiffSkipHandlers();
60	188	});
61	189
62	190	/**
63	191	2021-09-07: refactoring the following for use in the higher-level
64	192	fossil.*.js framework is pending. For now it's a copy/paste copy
65	193

	--- src/fossil.diff.js
	+++ src/fossil.diff.js
	@@ -20,45 +20,173 @@
20	document.querySelectorAll('table.diff').forEach(addToggle);
21	});
22
23	window.fossil.onPageLoad(function(){
24	const F = window.fossil, D = F.dom;
25













26	/**
27	Uses the /jchunk AJAX route to fetch specific lines of a given
28	artifact. The first argument must be an Object with these
29	properties:

30
31	{
32	name: full hash of the target file,
33	from: first 1-based line number of the file to fetch (inclusive),
34	to: last 1-based line number of the file to fetch (inclusive)
35	}
36
37	onload and onerror are optional callback functions to be called
38	on success resp. error, as documented for window.fossil.fetch().
39	Note that onload is ostensibly optional but this function is not
40	of much use without an onload handler. Conversely, the default
41	onerror handler is often customized on a per-page basis to send
42	the error output somewhere where the user can see it.








43
44	The response, on success, will be an array of strings, each entry
45	being one line from the requested artifact. If the 'to' line is
46	greater than the length of the file, the array will be shorter
47	than (to-from) lines.
48
49	The /jchunk route reports errors via JSON objects with
50	an "error" string property describing the problem.
51
52	This is an async operation. Returns this object.












53	*/
54	F.fetchArtifactLines = function(urlParams, onload, onerror){
55	const opt = {urlParams};
56	if(onload) opt.onload = onload;
57	if(onerror) opt.onerror = onerror;
58	return this.fetch('jchunk', opt);




























































































59	};


60	});
61
62	/**
63	2021-09-07: refactoring the following for use in the higher-level
64	fossil.*.js framework is pending. For now it's a copy/paste copy
65

	--- src/fossil.diff.js
	+++ src/fossil.diff.js
	@@ -20,45 +20,173 @@
20	document.querySelectorAll('table.diff').forEach(addToggle);
21	});
22
23	window.fossil.onPageLoad(function(){
24	const F = window.fossil, D = F.dom;
25	const Diff = F.diff = {
26	config: {
27	chunkLoadLines: 20,
28	chunkFetch: {
29	/* Default callack handlers for Diff.fetchArtifactChunk(),
30	unless overridden by options passeed to that function. */
31	beforesend: function(){},
32	aftersend: function(){},
33	onerror: function(e){
34	F.toast.error("XHR error: ",e.message);
35	}
36	}
37	}
38	};
39	/**
40	Uses the /jchunk AJAX route to fetch specific lines of a given
41	artifact. The argument must be an Object suitable for passing as
42	the second argument to fossil.fetch(). Its urlParams property
43	must be an object with these properties:
44
45	{
46	name: full hash of the target file,
47	from: first 1-based line number of the file to fetch (inclusive),
48	to: last 1-based line number of the file to fetch (inclusive)
49	}
50
51	The fetchOpt object is NOT cloned for use by the call: it is used
52	as-is and may be modified by this call. Thus callers "really
53	should" pass a temporary object, not a long-lived one.
54
55	If fetchOpt does not define any of the (beforesend, aftersend,
56	onerror) callbacks, the defaults from fossil.diff.config.chunkFetch
57	are used, so any given client page may override those to provide
58	page-level default handling.
59
60	Note that onload callback is ostensibly optional but this
61	function is not of much use without an onload
62	handler. Conversely, the default onerror handler is often
63	customized on a per-page basis to send the error output somewhere
64	where the user can see it.
65
66	The response, on success, will be an array of strings, each entry
67	being one line from the requested artifact. If the 'to' line is
68	greater than the length of the file, the array will be shorter
69	than (to-from) lines.
70
71	The /jchunk route reports errors via JSON objects with
72	an "error" string property describing the problem.
73
74	This is an async operation. Returns the fossil object.
75	*/
76	Diff.fetchArtifactChunk = function(fetchOpt){
77	if(!fetchOpt.beforesend) fetchOpt.beforesend = Diff.config.chunkFetch.beforesend;
78	if(!fetchOpt.aftersend) fetchOpt.aftersend = Diff.config.chunkFetch.aftersend;
79	if(!fetchOpt.onerror) fetchOpt.onerror = Diff.config.chunkFetch.onerror;
80	fetchOpt.responseType = 'json';
81	return F.fetch('jchunk', fetchOpt);
82	};
83
84	/**
85	Fetches /jchunk for the given TR element then replaces the TR's
86	contents with data from the result of that request.
87	*/
88	const fetchTrChunk = function(tr){
89	if(tr.dataset.xfer /* already being fetched */) return;
90	const table = tr.parentElement.parentElement;
91	const hash = table.dataset.lefthash;
92	if(!hash) return;
93	const isSbs = table.classList.contains('splitdiff')/else udiff/;
94	tr.dataset.xfer = 1 /* sentinel against multiple concurrent ajax requests */;
95	const lineTo = +tr.dataset.endln;
96	var lnFrom = +tr.dataset.startln;
97	/* TODO: for the time being, for simplicity, we'll read the whole
98	[startln, endln] chunk. "Later on" we'll maybe want to read it in
99	chunks of, say, 20 lines or so, adjusting lnFrom to be 1 if it would
100	be less than 1. */
101	Diff.fetchArtifactChunk({
102	urlParams:{
103	name: hash,
104	from: lnFrom,
105	to: lineTo
106	},
107	aftersend: function(){
108	delete tr.dataset.xfer;
109	Diff.config.chunkFetch.aftersend.apply(
110	this, Array.prototype.slice.call(arguments,0)
111	);
112	},
113	onload: function(result){
114	console.debug("Chunk result: ",result);
115	D.clearElement(tr);
116	D.append(
117	D.attr(D.td(tr), 'colspan', isSbs ? 5 : 4),
118	"Fetched chunk of ",result.length," line(s). TODO: insert it here."
119	);
120	/*
121	At this point we need to:
122
123	- Read the previous TR, if any, to get the preceeding LHS/RHS
124	line numbers so that we know where to start counting.
125
126	- If there is no previous TR, we're at the top and we
127	instead need to get the LHS/RHS line numbers from the
128	following TR's children.
129
130	- D.clearElement(tr) and insert columns appropriate for the
131	parent table's diff type.
132
133	We can fish the line numbers out of the PRE columns with something
134	like this inefficient but effective hack:
135
136	theElement.innerText.split(/\n+/)
137
138	(need /\n+/ instead of '\n' b/c of INS/DEL elements)
139
140	Noting that the result array will end with an empty element
141	due to the trailing \n character, so a call to pop() will be
142	needed.
143
144	SBS diff col layout:
145	<td><pre>...LHS line numbers...</pre></td>
146	<td>...code lines...</td>
147	<td></td> empty for this case.
148	<td><pre>...RHS line numbers...</pre></td>
149	<td>...dupe of col 2</td>
150
151	Unified diff col layout:
152	<td>LHS line numbers</td>
153	<td>RHS line numbers</td>
154	<td>blank in this case</td>
155	<td>code line</td>
156	*/
157	}
158	});
159	};
160
161	Diff.addDiffSkipHandlers = function(){
162	const tables = document.querySelectorAll('table.diff[data-lefthash]');
163	if(!tables.length) return F;
164	const addDiffSkipToTr = function f(tr){
165	D.addClass(tr, 'jchunk');
166	if(!f._handler){
167	f._handler = function ff(event){
168	var e = event.target;
169	while(e && 'TR' !== e.tagName) e = e.parentElement;
170	if(!e){
171	console.error("Internal event-handling error: didn't find TR target.");
172	return;
173	}
174	e.removeEventListener('click',ff);
175	D.removeClass(e, 'jchunk');
176	//console.debug("addDiffSkipToTr() Event:",e, event);
177	fetchTrChunk(e);
178	};
179	}
180	tr.addEventListener('click', f._handler, false);
181	};
182	tables.forEach(function(t){
183	t.querySelectorAll('tr.diffskip[data-startln]').forEach(addDiffSkipToTr);
184	});
185	};
186
187	F.diff.addDiffSkipHandlers();
188	});
189
190	/**
191	2021-09-07: refactoring the following for use in the higher-level
192	fossil.*.js framework is pending. For now it's a copy/paste copy
193

M src/info.c

+6 -1

		--- src/info.c
		+++ src/info.c
		@@ -1899,11 +1899,11 @@
1899	1899	** diff page, to return unseen context to fill in additional context
1900	1900	** when the user clicks on the appropriate button. The response is
1901	1901	** always in JSON form and errors are reported as documented for
1902	1902	** ajax_route_error().
1903	1903	*/
1904		-void jtext_page(void){
	1904	+void jchunk_page(void){
1905	1905	int rid = 0;
1906	1906	const char *zName = PD("name", "");
1907	1907	int iFrom = atoi(PD("from","0"));
1908	1908	int iTo = atoi(PD("to","0"));
1909	1909	int ln;
		@@ -1911,10 +1911,15 @@
1911	1911	const char *zSep;
1912	1912	Blob content;
1913	1913	Blob line;
1914	1914	Blob *pOut;
1915	1915
	1916	+ if(0){
	1917	+ ajax_route_error(400, "Just testing client-side error handling.");
	1918	+ return;
	1919	+ }
	1920	+
1916	1921	login_check_credentials();
1917	1922	if( !g.perm.Read ){
1918	1923	ajax_route_error(403, "Access requires Read permissions.");
1919	1924	return;
1920	1925	}
1921	1926

	--- src/info.c
	+++ src/info.c
	@@ -1899,11 +1899,11 @@
1899	** diff page, to return unseen context to fill in additional context
1900	** when the user clicks on the appropriate button. The response is
1901	** always in JSON form and errors are reported as documented for
1902	** ajax_route_error().
1903	*/
1904	void jtext_page(void){
1905	int rid = 0;
1906	const char *zName = PD("name", "");
1907	int iFrom = atoi(PD("from","0"));
1908	int iTo = atoi(PD("to","0"));
1909	int ln;
	@@ -1911,10 +1911,15 @@
1911	const char *zSep;
1912	Blob content;
1913	Blob line;
1914	Blob *pOut;
1915





1916	login_check_credentials();
1917	if( !g.perm.Read ){
1918	ajax_route_error(403, "Access requires Read permissions.");
1919	return;
1920	}
1921

	--- src/info.c
	+++ src/info.c
	@@ -1899,11 +1899,11 @@
1899	** diff page, to return unseen context to fill in additional context
1900	** when the user clicks on the appropriate button. The response is
1901	** always in JSON form and errors are reported as documented for
1902	** ajax_route_error().
1903	*/
1904	void jchunk_page(void){
1905	int rid = 0;
1906	const char *zName = PD("name", "");
1907	int iFrom = atoi(PD("from","0"));
1908	int iTo = atoi(PD("to","0"));
1909	int ln;
	@@ -1911,10 +1911,15 @@
1911	const char *zSep;
1912	Blob content;
1913	Blob line;
1914	Blob *pOut;
1915
1916	if(0){
1917	ajax_route_error(400, "Just testing client-side error handling.");
1918	return;
1919	}
1920
1921	login_check_credentials();
1922	if( !g.perm.Read ){
1923	ajax_route_error(403, "Access requires Read permissions.");
1924	return;
1925	}
1926

Fossil SCM

Keyboard Shortcuts