[PATCH v3 2/6] ui-shared: line range highlight: introduce javascript

[Andy Green]

On 06/24/2018 07:28 PM, John Keeping wrote:
> The subject is "ui-shared: ..." but should be "cgit.js" now I think.
> 
> On Sun, Jun 24, 2018 at 10:44:34AM +0800, Andy Green wrote:
>> diff --git a/cgit.js b/cgit.js
>> index e69de29..501c98f 100644
>> --- a/cgit.js
>> +++ b/cgit.js
>> @@ -0,0 +1,53 @@
>> +function cgit_line_range_highlight()
>> +{
>> +	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
>> +
>> +	l1 = parseInt(h.substring(2));
>> +	t = h.indexOf("-");
>> +	if (t >= 1)
>> +		l2 = parseInt(h.substring(t + 1));
>> +	else
>> +		l2 = l1;
>> +
>> +	if (!l1)
>> +		return;
>> +
>> +	var lh, t = 0, e1, e2, de;
>> +
>> +	e1 = e = document.getElementById('n' + l1);
>> +	if (!e)
>> +		return;
>> +
>> +	while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; }
>> +
>> +	de = document.createElement("DIV");
>> +
>> +	de.className = "selected-lines";
>> +	de.style.bottom = e.style.bottom;
>> +	de.style.top = t + 'px';
>> +
>> +	/* DOM structure is different by one level for /blame/ */
>> +	e1 = e.parentElement.parentElement.parentNode;
>> +	if (e1.offsetWidth < e1.parentNode.offsetWidth) /* blame */
>> +		e1 = e1.parentNode;
> 
> I guess this is trying to find the <tr> element, so why not look for
> that directly?
> 
> 	while (e1.tagName.toUpperCase() != 'TR')
> 		e1 = e1.parentNode;
> 
>> +	de.style.width = e1.offsetWidth + 'px';
>> +	de.style.left = e1.parentNode.parentNode.offsetLeft + 'px';
> 
> Again, if we're looking for the <table> here, can we make this more
> robust by looking for it explicitly?
> 
>> +	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
>> +
>> +	e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
> 
> If I'm following correctly, e.parentElement.parentElement is either <td>
> in tree view or <div> in blame view.  Is that expected?
> 
> My thinking is that it is correct, but what we're looking for is the
> parent of the <pre> element containing the <a> with the line number ID.
> Some comments would really help make sense of these chains of parent
> references!

OK... I added a couple of comments but I think changing the var names to 
"etr" and "etable" and following the comment about looking for those 
specific names has obviated the rest of the need.

-Andy

>> +	while (l1 <= l2) {
>> +		e1 = document.getElementById('n' + l1++);
>> +		e1.style.backgroundColor = "yellow";
>> +	}
>> +
>> +	e.scrollIntoView(true);
>> +}
>> +
>> +/* line range highlight */
>> +
>> +document.addEventListener("DOMContentLoaded", function() {
>> +	cgit_line_range_highlight();
>> +}, false);