<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 30, 2017 at 1:29 AM, Denis Laxalde <span dir="ltr"><<a href="mailto:denis@laxalde.org" target="_blank">denis@laxalde.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"># HG changeset patch<br>
# User Denis Laxalde <<a href="mailto:denis.laxalde@logilab.fr">denis.laxalde@logilab.fr</a>><br>
# Date 1490819176 -7200<br>
# Wed Mar 29 22:26:16 2017 +0200<br>
# Node ID 1c360ab5640bc326dd6ff70bdee78e<wbr>4da006dcef<br>
# Parent e540846c67e0f838bcdb1db567a57d<wbr>f28d92491c<br>
# Available At <a href="http://hg.logilab.org/users/dlaxalde/hg" rel="noreferrer" target="_blank">http://hg.logilab.org/users/<wbr>dlaxalde/hg</a><br>
# hg pull <a href="http://hg.logilab.org/users/dlaxalde/hg" rel="noreferrer" target="_blank">http://hg.logilab.org/users/<wbr>dlaxalde/hg</a> -r 1c360ab5640b<br>
# EXP-Topic linerange-log/hgweb-filelog<br>
hgweb: expose a followlines UI in filerevision view<br>
<br>
In filerevision view (/file/<rev>/<fname>) we add some event listeners on<br>
mouse clicks of <span> elements in the <pre class="sourcelines"> block.<br>
Those listeners will capture a range of lines selected between two mouse<br>
clicks and a box inviting to follow the history of selected lines will then<br>
show up. Selected lines (i.e. the block of lines) get a CSS class which make<br>
them highlighted. Selection can be cancelled (and restarted) by either<br>
clicking on the cancel ("x") button in the invite box or clicking on any other<br>
source line. Also clicking twice on the same line will abort the selection and<br>
reset event listeners to restart the process.<br>
<br>
As a first step, this action is only advertised by the "cursor: cell" CSS rule<br>
on source lines elements as any other mechanisms would make the code<br>
significantly more complicated. This might be improved later.<br>
<br>
All JavaScript code lives in a new "linerangelog.js" file, sourced in<br>
filerevision template (only in "paper" style for now).<br></blockquote><div><br></div><div>This patch is great! As far as functionality goes, I think it is almost good enough to queue. There are still some usability improvements (such as an aid to let users know that clicking a line will allow them to follow that line). But those features can be implemented as follow-ups IMO.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
diff --git a/contrib/wix/templates.wxs b/contrib/wix/templates.wxs<br>
--- a/contrib/wix/templates.wxs<br>
+++ b/contrib/wix/templates.wxs<br>
@@ -225,6 +225,7 @@<br>
<File Id="static.coal.file.png" Name="coal-file.png" /><br>
<File Id="static.coal.folder.png" Name="coal-folder.png" /><br>
<File Id="static.excanvas.js" Name="excanvas.js" /><br>
+ <File Id="static.linerangelog.js" Name="linerangelog.js" /><br>
<File Id="static.mercurial.js" Name="mercurial.js" /><br>
<File Id="static.hgicon.png" Name="hgicon.png" /><br>
<File Id="static.hglogo.png" Name="hglogo.png" /><br>
diff --git a/mercurial/templates/paper/<wbr>filerevision.tmpl b/mercurial/templates/paper/<wbr>filerevision.tmpl<br>
--- a/mercurial/templates/paper/<wbr>filerevision.tmpl<br>
+++ b/mercurial/templates/paper/<wbr>filerevision.tmpl<br>
@@ -71,8 +71,11 @@<br>
<div class="overflow"><br>
<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:<wbr>toggleLinewrap()">on</a></div><br>
<div class="sourcefirst"> line source</div><br>
-<pre class="sourcelines stripes4 wrap bottomline">{text%fileline}</<wbr>pre><br>
+<pre class="sourcelines stripes4 wrap bottomline" data-logurl="{url|urlescape}<wbr>log/{symrev}/{file|urlescape}"<wbr>>{text%fileline}</pre><br>
</div><br>
+<br>
+<script type="text/javascript" src="{staticurl|urlescape}<wbr>linerangelog.js"></script><br>
+<br>
</div><br>
</div><br>
<br>
diff --git a/mercurial/templates/static/<wbr>linerangelog.js b/mercurial/templates/static/<wbr>linerangelog.js<br>
new file mode 100644<br>
--- /dev/null<br>
+++ b/mercurial/templates/static/<wbr>linerangelog.js<br>
@@ -0,0 +1,137 @@<br>
+// Copyright 2017 Logilab SA <<a href="mailto:contact@logilab.fr">contact@logilab.fr</a>><br>
+//<br>
+// This software may be used and distributed according to the terms<br>
+// of the GNU General Public License, incorporated herein by reference.<br></blockquote><div><br></div><div>This license header is slightly different from what is used elsewhere. In other files, we use "version 2 or any later version." I see this file's version is used by .c files and a few other random places. But since it is in the minority, let's stick to the "version 2 or any later version" language, as seen in e.g. mercurial/__init__.py.<br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+//** Install event listeners for line block selection and followlines action */<br>
+function installLineSelect() {<br>
+ var sourcelines = document.<wbr>getElementsByClassName('<wbr>sourcelines')[0];<br>
+ if (typeof sourcelines === 'undefined') {<br>
+ return;<br>
+ }<br>
+ // URL to complement with "linerange" query parameter<br>
+ var targetUri = sourcelines.dataset.logurl;<br>
+ if (typeof targetUri === 'undefined') {<br>
+ return;<br>
+ }<br>
+<br>
+ var lineSelectedCSSClass = 'followlines-selected';<br>
+<br>
+ //** add CSS class on <span> element in `from`-`to` line range */<br>
+ function addSelectedCSSClass(from, to) {<br>
+ var spans = sourcelines.<wbr>getElementsByTagName('span');<br>
+ for (var i = from; i <= to; i++) {<br>
+ spans.item(i).classList.add(<wbr>lineSelectedCSSClass);<br>
+ }<br>
+ }<br>
+<br>
+ //** remove CSS class from previously selected lines */<br>
+ function removeSelectedCSSClass() {<br>
+ var nodes = sourcelines.<wbr>getElementsByClassName(<br>
+ lineSelectedCSSClass);<br>
+ while (nodes.length) {<br>
+ nodes[0].classList.remove(<wbr>lineSelectedCSSClass);<br>
+ }<br>
+ }<br>
+<br>
+ // add event listener to the whole <pre class="sourcelines"> block to have<br>
+ // it available in all children <span><br>
+ sourcelines.addEventListener('<wbr>click', lineSelectStart);<br></blockquote><div><br></div><div>I know this works, but could you please move this until after the definition of lineSelectStart() because it feels weird to see the reference to a symbol before it is declared.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ //** event handler for "click" on the first line of a block */<br>
+ function lineSelectStart(e) {<br>
+ var startElement = e.target;<br>
+ if (startElement.tagName !== 'SPAN') {<br>
+ // not a <span> (maybe <a>): abort, keeping event listener<br>
+ // registered for other click with <span> target<br>
+ return;<br></blockquote><div><br></div><div>I wonder what happens when Pygments is installed and the HTML is no longer as simple as we expect. It feels like we should traverse startElement.parentElement until we find an element with .parentElement.isSameNode(sourcelines) that is also a <span>. Alternatively, on startup we could iterate sourcelines.children and add a CSS class (I don't think we should add the class to the HTML because it would needlessly bloat the HTML).<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ }<br>
+ var startId = parseInt(startElement.id.<wbr>slice(1));<br>
+ startElement.classList.add(<wbr>lineSelectedCSSClass); // CSS<br>
+<br>
+ // remove this event listener<br>
+ sourcelines.<wbr>removeEventListener('click', lineSelectStart);<br>
+<br>
+ //** event handler for "click" on the last line of the block */<br>
+ function lineSelectEnd(e) {<br>
+ var endElement = e.target;<br>
+ if (endElement.tagName !== 'SPAN') {<br>
+ // not a <span> (maybe <a>): abort, keeping event listener<br>
+ // registered for other click with <span> target<br>
+ return;<br></blockquote><div><br></div><div>Same concern with not being robust enough.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ }<br>
+<br>
+ // remove this event listener<br>
+ sourcelines.<wbr>removeEventListener('click', lineSelectEnd);<br>
+<br>
+ // compute line range (startId, endId)<br>
+ var endId = parseInt(endElement.id.slice(<wbr>1));<br>
+ if (endId == startId) {<br>
+ // clicked twice the same line, cancel and reset initial state<br>
+ // (CSS and event listener for selection start)<br>
+ removeSelectedCSSClass();<br>
+ sourcelines.addEventListener('<wbr>click', lineSelectStart);<br>
+ return;<br>
+ }<br>
+ var inviteElement = endElement;<br>
+ if (endId < startId) {<br>
+ var tmp = endId;<br>
+ endId = startId;<br>
+ startId = tmp;<br>
+ inviteElement = startElement;<br>
+ }<br>
+<br>
+ addSelectedCSSClass(startId - 1, endId -1); // CSS<br>
+<br>
+ // append the <div id="followlines"> element to last line of the<br>
+ // selection block<br>
+ var divAndButton = followlinesBox(targetUri, startId, endId);<br>
+ var div = divAndButton[0],<br>
+ button = divAndButton[1];<br>
+ inviteElement.appendChild(div)<wbr>;<br>
+<br>
+ //** event handler for cancelling selection */<br>
+ function cancel() {<br>
+ // remove invite box<br>
+ div.parentNode.removeChild(<wbr>div);<br>
+ // restore initial event listeners<br>
+ sourcelines.addEventListener('<wbr>click', lineSelectStart);<br>
+ sourcelines.<wbr>removeEventListener('click', cancel);<br>
+ // remove styles on selected lines<br>
+ removeSelectedCSSClass();<br>
+ }<br>
+<br>
+ // bind cancel event to click on <button><br>
+ button.addEventListener('<wbr>click', cancel);<br>
+ // as well as on an click on any source line<br>
+ sourcelines.addEventListener('<wbr>click', cancel);<br>
+ }<br>
+<br>
+ sourcelines.addEventListener('<wbr>click', lineSelectEnd);<br>
+<br>
+ }<br>
+<br>
+}<br>
+<br>
+//** return a <div id="followlines"> and inner cancel <button> elements */<br>
+function followlinesBox(targetUri, startId, endId) {<br>
+ var div = document.createElement('div');<br>
+ <a href="http://div.id" rel="noreferrer" target="_blank">div.id</a> = 'followlines';<br>
+ var aDiv = document.createElement('div');<br>
+ aDiv.classList.add('<wbr>followlines-link');<br>
+ var a = document.createElement('a');<br>
+ var url = targetUri + '?patch=&linerange=' + startId + ':' + endId;<br>
+ a.setAttribute('href', url);<br>
+ a.textContent = 'follow lines ' + startId + ':' + endId;<br>
+ aDiv.appendChild(a);<br>
+ var buttonDiv = document.createElement('div');<br>
+ buttonDiv.classList.add('<wbr>followlines-cancel');<br>
+ var button = document.createElement('<wbr>button');<br>
+ button.textContent = 'x';<br>
+ buttonDiv.appendChild(button);<br>
+ div.appendChild(buttonDiv);<br>
+ div.appendChild(aDiv);<br>
+ return [div, button];<br></blockquote><div><br></div><div>This code is somewhat difficult to read. Could you please add some blank lines to separate the discrete elements/operations?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+}<br>
+<br>
+document.addEventListener('<wbr>DOMContentLoaded', installLineSelect, false);<br>
diff --git a/mercurial/templates/static/<wbr>style-paper.css b/mercurial/templates/static/<wbr>style-paper.css<br>
--- a/mercurial/templates/static/<wbr>style-paper.css<br>
+++ b/mercurial/templates/static/<wbr>style-paper.css<br>
@@ -280,6 +280,46 @@ td.annotate:hover div.annotate-info { di<br>
background-color: #bfdfff;<br>
}<br>
<br>
+div.overflow pre.sourcelines > span:hover {<br>
+ cursor: cell;<br>
+}<br>
+<br>
+pre.sourcelines > span.followlines-selected {<br>
+ background-color: #99C7E9;<br>
+}<br>
+<br>
+div#followlines {<br>
+ background-color: #B7B7B7;<br>
+ border: 1px solid #CCC;<br>
+ border-radius: 5px;<br>
+ padding: 4px;<br>
+ position: absolute;<br>
+}<br>
+<br>
+div.followlines-cancel {<br>
+ text-align: right;<br>
+}<br>
+<br>
+div.followlines-cancel > button {<br>
+ line-height: 80%;<br>
+ padding: 0;<br>
+ border: 0;<br>
+ border-radius: 2px;<br>
+ background-color: inherit;<br>
+ font-weight: bold;<br>
+}<br>
+<br>
+div.followlines-cancel > button:hover {<br>
+ color: #FFFFFF;<br>
+ background-color: #CF1F1F;<br>
+}<br>
+<br>
+div.followlines-link {<br>
+ margin: 2px;<br>
+ margin-top: 4px;<br>
+ font-family: sans-serif;<br>
+}<br>
+<br>
.sourcelines > a {<br>
display: inline-block;<br>
position: absolute;<br>
diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t<br>
--- a/tests/test-hgweb-commands.t<br>
+++ b/tests/test-hgweb-commands.t<br>
@@ -1343,9 +1343,12 @@ File-related<br>
<div class="overflow"><br>
<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:<wbr>toggleLinewrap()">on</a></div><br>
<div class="sourcefirst"> line source</div><br>
- <pre class="sourcelines stripes4 wrap bottomline"><br>
+ <pre class="sourcelines stripes4 wrap bottomline" data-logurl="/log/1/foo"><br>
<span id="l1">foo</span><a href="#l1"></a></pre><br>
</div><br>
+<br>
+ <script type="text/javascript" src="/static/linerangelog.js"><wbr></script><br>
+<br>
</div><br>
</div><br>
<br>
@@ -1468,9 +1471,12 @@ File-related<br>
<div class="overflow"><br>
<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:<wbr>toggleLinewrap()">on</a></div><br>
<div class="sourcefirst"> line source</div><br>
- <pre class="sourcelines stripes4 wrap bottomline"><br>
+ <pre class="sourcelines stripes4 wrap bottomline" data-logurl="/log/2/foo"><br>
<span id="l1">another</span><a href="#l1"></a></pre><br>
</div><br>
+<br>
+ <script type="text/javascript" src="/static/linerangelog.js"><wbr></script><br>
+<br>
</div><br>
</div><br>
<br>
______________________________<wbr>_________________<br>
Mercurial-devel mailing list<br>
<a href="mailto:Mercurial-devel@mercurial-scm.org">Mercurial-devel@mercurial-scm.<wbr>org</a><br>
<a href="https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel" rel="noreferrer" target="_blank">https://www.mercurial-scm.org/<wbr>mailman/listinfo/mercurial-<wbr>devel</a><br>
</blockquote></div><br></div></div>