Patchwork [4,of,4] hgweb: add a link to followlines in descending direction

login
register
mail settings
Submitter Denis Laxalde
Date April 11, 2017, 1:09 p.m.
Message ID <4895e777df5cc64eccd0.1491916151@sh77.tls.logilab.fr>
Download mbox | patch
Permalink /patch/20103/
State Accepted
Headers show

Comments

Denis Laxalde - April 11, 2017, 1:09 p.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1491838600 -7200
#      Mon Apr 10 17:36:40 2017 +0200
# Node ID 4895e777df5cc64eccd038d71a9f9a5b969a5ea0
# Parent  a24bf350ea723273831d6531a1345ed458312126
# Available At http://hg.logilab.org/users/dlaxalde/hg
#              hg pull http://hg.logilab.org/users/dlaxalde/hg -r 4895e777df5c
hgweb: add a link to followlines in descending direction

We change the content of the followlines popup to display two links inviting
to follow the history of selected lines in ascending (as before) and
descending directions. The popup now renders as:

  follow history of lines <fromline>:<toline>:
  <a href=...>ascending</a> / <a href=...>descending</a>
Augie Fackler - April 12, 2017, 9:14 p.m.
On Tue, Apr 11, 2017 at 03:09:11PM +0200, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1491838600 -7200
> #      Mon Apr 10 17:36:40 2017 +0200
> # Node ID 4895e777df5cc64eccd038d71a9f9a5b969a5ea0
> # Parent  a24bf350ea723273831d6531a1345ed458312126
> # Available At http://hg.logilab.org/users/dlaxalde/hg
> #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r 4895e777df5c
> hgweb: add a link to followlines in descending direction

queued, thanks
Gregory Szorc - April 15, 2017, 8:09 p.m.
On Tue, Apr 11, 2017 at 6:09 AM, Denis Laxalde <denis@laxalde.org> wrote:

> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1491838600 -7200
> #      Mon Apr 10 17:36:40 2017 +0200
> # Node ID 4895e777df5cc64eccd038d71a9f9a5b969a5ea0
> # Parent  a24bf350ea723273831d6531a1345ed458312126
> # Available At http://hg.logilab.org/users/dlaxalde/hg
> #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r
> 4895e777df5c
> hgweb: add a link to followlines in descending direction
>
> We change the content of the followlines popup to display two links
> inviting
> to follow the history of selected lines in ascending (as before) and
> descending directions. The popup now renders as:
>
>   follow history of lines <fromline>:<toline>:
>   <a href=...>ascending</a> / <a href=...>descending</a>
>

Cool feature!

I do wonder if "ascending" and "descending" are the best terms to expose to
end-users in the HTML interface. For the revset it makes sense since it is
operating on the DAG and various revsets need you to know the concept of a
DAG. But for the web interface, I wonder if something like "older" and
"newer" would be better. And perhaps the links could contain verbs. e.g.
"show older" and "show newer."

Anyone else have any thoughts?


>
> diff --git a/mercurial/templates/static/followlines.js
> b/mercurial/templates/static/followlines.js
> --- a/mercurial/templates/static/followlines.js
> +++ b/mercurial/templates/static/followlines.js
> @@ -204,13 +204,23 @@ document.addEventListener('DOMContentLoa
>          //   <div class="followlines-link">
>          var aDiv = document.createElement('div');
>          aDiv.classList.add('followlines-link');
> -
> -        //     <a href="/log/<rev>/<file>?patch=&linerange=...">
> -        var a = document.createElement('a');
> +        aDiv.textContent = 'follow history of lines ' + fromline + ':' +
> toline + ':';
> +        var linesep = document.createElement('br');
> +        aDiv.appendChild(linesep);
> +        //     link to "ascending" followlines
> +        var aAsc = document.createElement('a');
>          var url = targetUri + '?patch=&linerange=' + fromline + ':' +
> toline;
> -        a.setAttribute('href', url);
> -        a.textContent = 'follow lines ' + fromline + ':' + toline;
> -        aDiv.appendChild(a);
> +        aAsc.setAttribute('href', url);
> +        aAsc.textContent = 'ascending';
> +        aDiv.appendChild(aAsc);
> +        var sep = document.createTextNode(' / ');
> +        aDiv.appendChild(sep);
> +        //     link to "descending" followlines
> +        var aDesc = document.createElement('a');
> +        aDesc.setAttribute('href', url + '&descend=');
> +        aDesc.textContent = 'descending';
> +        aDiv.appendChild(aDesc);
> +
>          div.appendChild(aDiv);
>
>          return [div, button];
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Gregory Szorc - April 15, 2017, 8:11 p.m.
On Tue, Apr 11, 2017 at 6:09 AM, Denis Laxalde <denis@laxalde.org> wrote:

> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1491838600 -7200
> #      Mon Apr 10 17:36:40 2017 +0200
> # Node ID 4895e777df5cc64eccd038d71a9f9a5b969a5ea0
> # Parent  a24bf350ea723273831d6531a1345ed458312126
> # Available At http://hg.logilab.org/users/dlaxalde/hg
> #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r
> 4895e777df5c
> hgweb: add a link to followlines in descending direction
>
> We change the content of the followlines popup to display two links
> inviting
> to follow the history of selected lines in ascending (as before) and
> descending directions. The popup now renders as:
>
>   follow history of lines <fromline>:<toline>:
>   <a href=...>ascending</a> / <a href=...>descending</a>
>

How difficult would it be to not render the "descending" link when we're
already looking at the latest changeset / file revision? As it is today,
the "descending" link goes to a blank page.


>
> diff --git a/mercurial/templates/static/followlines.js
> b/mercurial/templates/static/followlines.js
> --- a/mercurial/templates/static/followlines.js
> +++ b/mercurial/templates/static/followlines.js
> @@ -204,13 +204,23 @@ document.addEventListener('DOMContentLoa
>          //   <div class="followlines-link">
>          var aDiv = document.createElement('div');
>          aDiv.classList.add('followlines-link');
> -
> -        //     <a href="/log/<rev>/<file>?patch=&linerange=...">
> -        var a = document.createElement('a');
> +        aDiv.textContent = 'follow history of lines ' + fromline + ':' +
> toline + ':';
> +        var linesep = document.createElement('br');
> +        aDiv.appendChild(linesep);
> +        //     link to "ascending" followlines
> +        var aAsc = document.createElement('a');
>          var url = targetUri + '?patch=&linerange=' + fromline + ':' +
> toline;
> -        a.setAttribute('href', url);
> -        a.textContent = 'follow lines ' + fromline + ':' + toline;
> -        aDiv.appendChild(a);
> +        aAsc.setAttribute('href', url);
> +        aAsc.textContent = 'ascending';
> +        aDiv.appendChild(aAsc);
> +        var sep = document.createTextNode(' / ');
> +        aDiv.appendChild(sep);
> +        //     link to "descending" followlines
> +        var aDesc = document.createElement('a');
> +        aDesc.setAttribute('href', url + '&descend=');
> +        aDesc.textContent = 'descending';
> +        aDiv.appendChild(aDesc);
> +
>          div.appendChild(aDiv);
>
>          return [div, button];
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Denis Laxalde - April 17, 2017, 8:39 a.m.
Gregory Szorc a écrit :
>> # HG changeset patch
>> # User Denis Laxalde <denis.laxalde@logilab.fr>
>> # Date 1491838600 -7200
>> #      Mon Apr 10 17:36:40 2017 +0200
>> # Node ID 4895e777df5cc64eccd038d71a9f9a5b969a5ea0
>> # Parent  a24bf350ea723273831d6531a1345ed458312126
>> # Available At http://hg.logilab.org/users/dlaxalde/hg
>> #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r
>> 4895e777df5c
>> hgweb: add a link to followlines in descending direction
>>
>> We change the content of the followlines popup to display two links
>> inviting
>> to follow the history of selected lines in ascending (as before) and
>> descending directions. The popup now renders as:
>>
>>   follow history of lines <fromline>:<toline>:
>>   <a href=...>ascending</a> / <a href=...>descending</a>
>>
>
> How difficult would it be to not render the "descending" link when we're
> already looking at the latest changeset / file revision? As it is today,
> the "descending" link goes to a blank page.
>

Because we're on the client side, that would require a fair amount of
extra machinery I'd say. Namely, we'd need to query a dedicated server
endpoint with the information provided by the user (file, revision, line
range) before displaying the link. We may avoid displaying the link when
at the last revision of the filelog but that would also require
additional logic on the web command and that would not work when at a
revision from which the selected line range has not be touched.

Maybe we can display an informative message in the target page when
there's no revision to display? (Similar to what you get when a search
query yields no result.)
Gregory Szorc - April 17, 2017, 5:26 p.m.
On Mon, Apr 17, 2017 at 1:39 AM, Denis Laxalde <denis@laxalde.org> wrote:

> Gregory Szorc a écrit :
>
>> # HG changeset patch
>>> # User Denis Laxalde <denis.laxalde@logilab.fr>
>>> # Date 1491838600 -7200
>>> #      Mon Apr 10 17:36:40 2017 +0200
>>> # Node ID 4895e777df5cc64eccd038d71a9f9a5b969a5ea0
>>> # Parent  a24bf350ea723273831d6531a1345ed458312126
>>> # Available At http://hg.logilab.org/users/dlaxalde/hg
>>> #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r
>>> 4895e777df5c
>>> hgweb: add a link to followlines in descending direction
>>>
>>> We change the content of the followlines popup to display two links
>>> inviting
>>> to follow the history of selected lines in ascending (as before) and
>>> descending directions. The popup now renders as:
>>>
>>>   follow history of lines <fromline>:<toline>:
>>>   <a href=...>ascending</a> / <a href=...>descending</a>
>>>
>>>
>> How difficult would it be to not render the "descending" link when we're
>> already looking at the latest changeset / file revision? As it is today,
>> the "descending" link goes to a blank page.
>>
>>
> Because we're on the client side, that would require a fair amount of
> extra machinery I'd say. Namely, we'd need to query a dedicated server
> endpoint with the information provided by the user (file, revision, line
> range) before displaying the link. We may avoid displaying the link when
> at the last revision of the filelog but that would also require
> additional logic on the web command and that would not work when at a
> revision from which the selected line range has not be touched.
>

We definitely want to do this computation on the server, where it already
has access to the full repo data.

You raise some good points. But thinking about this some, I think perfect
is the enemy of good and we can address many occurrences with a simple fix:
examining if the current changesets is a DAG head. If the current changeset
is a head, then no descendant filelog revisions for this line of work exist
and we shouldn't display the link. Otherwise if we're not a head,
calculating whether a filelog has descendants could be expensive. Unless we
want to check the special case that the filelog revision is also a DAG
head, we should display the link.


>
> Maybe we can display an informative message in the target page when
> there's no revision to display? (Similar to what you get when a search
> query yields no result.)
>
>
Yes, that's a good idea.

Patch

diff --git a/mercurial/templates/static/followlines.js b/mercurial/templates/static/followlines.js
--- a/mercurial/templates/static/followlines.js
+++ b/mercurial/templates/static/followlines.js
@@ -204,13 +204,23 @@  document.addEventListener('DOMContentLoa
         //   <div class="followlines-link">
         var aDiv = document.createElement('div');
         aDiv.classList.add('followlines-link');
-
-        //     <a href="/log/<rev>/<file>?patch=&linerange=...">
-        var a = document.createElement('a');
+        aDiv.textContent = 'follow history of lines ' + fromline + ':' + toline + ':';
+        var linesep = document.createElement('br');
+        aDiv.appendChild(linesep);
+        //     link to "ascending" followlines
+        var aAsc = document.createElement('a');
         var url = targetUri + '?patch=&linerange=' + fromline + ':' + toline;
-        a.setAttribute('href', url);
-        a.textContent = 'follow lines ' + fromline + ':' + toline;
-        aDiv.appendChild(a);
+        aAsc.setAttribute('href', url);
+        aAsc.textContent = 'ascending';
+        aDiv.appendChild(aAsc);
+        var sep = document.createTextNode(' / ');
+        aDiv.appendChild(sep);
+        //     link to "descending" followlines
+        var aDesc = document.createElement('a');
+        aDesc.setAttribute('href', url + '&descend=');
+        aDesc.textContent = 'descending';
+        aDiv.appendChild(aDesc);
+
         div.appendChild(aDiv);
 
         return [div, button];