From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Sun, 24 Jun 2018 12:28:19 +0100 Subject: [PATCH v3 2/6] ui-shared: line range highlight: introduce javascript In-Reply-To: <152980827428.2873.18013613713339707000.stgit@mail.warmcat.com> References: <152980795804.2873.3939128717470466784.stgit@mail.warmcat.com> <152980827428.2873.18013613713339707000.stgit@mail.warmcat.com> Message-ID: <20180624112819.GJ6584@john.keeping.me.uk> 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 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 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
in tree view or
in blame view. Is that expected? My thinking is that it is correct, but what we're looking for is the parent of the
 element containing the  with the line number ID.
Some comments would really help make sense of these chains of parent
references!

> +	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);