Patchwork [3,of,4] hgweb: add ancestors blame info in annotate hgweb template

login
register
mail settings
Submitter Denis Laxalde
Date June 6, 2016, 8:03 a.m.
Message ID <ecadc1da55f84fd77110.1465200199@sh77.tls.logilab.fr>
Download mbox | patch
Permalink /patch/15422/
State Changes Requested
Headers show

Comments

Denis Laxalde - June 6, 2016, 8:03 a.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1464943305 -7200
#      Fri Jun 03 10:41:45 2016 +0200
# Node ID ecadc1da55f84fd77110ca23341e12949110d46c
# Parent  632a34ed3b63420e1444cb4c3a09a0692b274615
hgweb: add ancestors blame info in annotate hgweb template

In gitweb style, display this ancestors information in a new column on the
left of the revision's one.

Also add a table head in annotate view (in gitweb style) so that it's clearer
what each column refers to.
Yuya Nishihara - June 8, 2016, 1:14 p.m.
On Mon, 06 Jun 2016 10:03:19 +0200, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1464943305 -7200
> #      Fri Jun 03 10:41:45 2016 +0200
> # Node ID ecadc1da55f84fd77110ca23341e12949110d46c
> # Parent  632a34ed3b63420e1444cb4c3a09a0692b274615
> hgweb: add ancestors blame info in annotate hgweb template
> 
> In gitweb style, display this ancestors information in a new column on the
> left of the revision's one.

I know it is useful to navigate to the parent revision, but this seems a bit
verbose. The parent revision has no relation to the line where it is displayed.

> diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
> --- a/mercurial/hgweb/webcommands.py
> +++ b/mercurial/hgweb/webcommands.py
> @@ -863,6 +863,16 @@ def annotate(web, req, tmpl):
>      diffopts = patch.difffeatureopts(web.repo.ui, untrusted=True,
>                                       section='annotate', whitespace=True)
>  
> +    def ancestors(f):
> +        for p in f.parents():
> +            yield {
> +                "node": p.hex(),
> +                "rev": p.rev(),
> +                "author": p.user(),
> +                "desc": p.description(),
> +                "file": p.path(),
> +            }
> +
>      def annotate(**map):
>          if util.binary(fctx.data()):
>              mt = (mimetypes.guess_type(fctx.path())[0]
> @@ -877,6 +887,7 @@ def annotate(web, req, tmpl):
>                     "node": f.hex(),
>                     "rev": f.rev(),
>                     "author": f.user(),
> +                   "ancestors": ancestors(f),

Nit: "ancestors" sounds as if it would return all ancestors.
Denis Laxalde - June 9, 2016, 12:26 p.m.
Yuya Nishihara a écrit :
> On Mon, 06 Jun 2016 10:03:19 +0200, Denis Laxalde wrote:
>> # HG changeset patch
>> # User Denis Laxalde <denis.laxalde@logilab.fr>
>> # Date 1464943305 -7200
>> #      Fri Jun 03 10:41:45 2016 +0200
>> # Node ID ecadc1da55f84fd77110ca23341e12949110d46c
>> # Parent  632a34ed3b63420e1444cb4c3a09a0692b274615
>> hgweb: add ancestors blame info in annotate hgweb template
>>
>> In gitweb style, display this ancestors information in a new column on the
>> left of the revision's one.
>
> I know it is useful to navigate to the parent revision, but this seems a bit
> verbose. The parent revision has no relation to the line where it is displayed.

That's true. For the record, I'm trying to address item "hgweb links to 
ancestor's blame" of https://www.mercurial-scm.org/wiki/BlamePlan. (Cc 
Gregory would originally wrote this plan.)

In fact, I later realized that there already was a link to parent's 
annotate view in the header section of the gitweb template that serves 
quite the same purpose. So to avoid "cluttering" the interface, maybe we 
could suggest a 2-steps navigation:
1) click on some link in the first column of the annotate view, leading 
the annotate view at this revision, then,
2) navigate to parent(s) annotate view from the header of the page.

What do you think?


PS.: discarding this patch for now.
Gregory Szorc - June 11, 2016, 2:07 a.m.
> On Jun 9, 2016, at 05:26, Denis Laxalde <denis.laxalde@logilab.fr> wrote:
> 
> Yuya Nishihara a écrit :
>>> On Mon, 06 Jun 2016 10:03:19 +0200, Denis Laxalde wrote:
>>> # HG changeset patch
>>> # User Denis Laxalde <denis.laxalde@logilab.fr>
>>> # Date 1464943305 -7200
>>> #      Fri Jun 03 10:41:45 2016 +0200
>>> # Node ID ecadc1da55f84fd77110ca23341e12949110d46c
>>> # Parent  632a34ed3b63420e1444cb4c3a09a0692b274615
>>> hgweb: add ancestors blame info in annotate hgweb template
>>> 
>>> In gitweb style, display this ancestors information in a new column on the
>>> left of the revision's one.
>> 
>> I know it is useful to navigate to the parent revision, but this seems a bit
>> verbose. The parent revision has no relation to the line where it is displayed.
> 
> That's true. For the record, I'm trying to address item "hgweb links to ancestor's blame" of https://www.mercurial-scm.org/wiki/BlamePlan. (Cc Gregory would originally wrote this plan.)
> 
> In fact, I later realized that there already was a link to parent's annotate view in the header section of the gitweb template that serves quite the same purpose. So to avoid "cluttering" the interface, maybe we could suggest a 2-steps navigation:
> 1) click on some link in the first column of the annotate view, leading the annotate view at this revision, then,
> 2) navigate to parent(s) annotate view from the header of the page.
> 
> What do you think?

The specific workflow I'd like to see optimized is navigating to earlier annotate views faster. Currently you have to go to a line then find a link possibly multiple pages away to the parent then find the original source line in that rendered view. Having a link to the parent's annotation for that line on a line/block removes 2 potentially time consuming scrolling steps and would be a massive win for traveling back in time in the annotation view.

This unfortunately requires some kind of clutter. Perhaps we could hide links unless there is mouseover/hover?
Yuya Nishihara - June 12, 2016, 5:25 a.m.
On Fri, 10 Jun 2016 19:07:16 -0700, Gregory Szorc wrote:
> > On Jun 9, 2016, at 05:26, Denis Laxalde <denis.laxalde@logilab.fr> wrote:
> > 
> > Yuya Nishihara a écrit :  
> >>> On Mon, 06 Jun 2016 10:03:19 +0200, Denis Laxalde wrote:
> >>> # HG changeset patch
> >>> # User Denis Laxalde <denis.laxalde@logilab.fr>
> >>> # Date 1464943305 -7200
> >>> #      Fri Jun 03 10:41:45 2016 +0200
> >>> # Node ID ecadc1da55f84fd77110ca23341e12949110d46c
> >>> # Parent  632a34ed3b63420e1444cb4c3a09a0692b274615
> >>> hgweb: add ancestors blame info in annotate hgweb template
> >>> 
> >>> In gitweb style, display this ancestors information in a new column on the
> >>> left of the revision's one.  
> >> 
> >> I know it is useful to navigate to the parent revision, but this seems a bit
> >> verbose. The parent revision has no relation to the line where it is displayed.  
> > 
> > That's true. For the record, I'm trying to address item "hgweb links to ancestor's blame" of https://www.mercurial-scm.org/wiki/BlamePlan. (Cc Gregory would originally wrote this plan.)
> > 
> > In fact, I later realized that there already was a link to parent's annotate view in the header section of the gitweb template that serves quite the same purpose. So to avoid "cluttering" the interface, maybe we could suggest a 2-steps navigation:
> > 1) click on some link in the first column of the annotate view, leading the annotate view at this revision, then,
> > 2) navigate to parent(s) annotate view from the header of the page.
> > 
> > What do you think?  
> 
> The specific workflow I'd like to see optimized is navigating to earlier annotate views faster. Currently you have to go to a line then find a link possibly multiple pages away to the parent then find the original source line in that rendered view. Having a link to the parent's annotation for that line on a line/block removes 2 potentially time consuming scrolling steps and would be a massive win for traveling back in time in the annotation view.
> 
> This unfortunately requires some kind of clutter. Perhaps we could hide links unless there is mouseover/hover?

Or make links less informative.

  mpm@22046 [^]  1 #require git

instead of

  ede@12992  mpm@22046  1 #require git

I don't like the latter because ede@12992 looks like saying something related
to the line 1, but actually it is only the parent of 22046 in filelog.
Denis Laxalde - June 14, 2016, 8:49 a.m.
Gregory Szorc a écrit :
> The specific workflow I'd like to see optimized is navigating to
> earlier annotate views faster. Currently you have to go to a line
> then find a link possibly multiple pages away to the parent then find
> the original source line in that rendered view. Having a link to the
> parent's annotation for that line on a line/block removes 2
> potentially time consuming scrolling steps and would be a massive win
> for traveling back in time in the annotation view.
>
> This unfortunately requires some kind of clutter. Perhaps we could
> hide links unless there is mouseover/hover?
>

I'll give your "hover" idea a try. I could also add the diff/rev links 
in here so that the annotate table would stay as it is now.

Patch

diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
--- a/mercurial/hgweb/webcommands.py
+++ b/mercurial/hgweb/webcommands.py
@@ -863,6 +863,16 @@  def annotate(web, req, tmpl):
     diffopts = patch.difffeatureopts(web.repo.ui, untrusted=True,
                                      section='annotate', whitespace=True)
 
+    def ancestors(f):
+        for p in f.parents():
+            yield {
+                "node": p.hex(),
+                "rev": p.rev(),
+                "author": p.user(),
+                "desc": p.description(),
+                "file": p.path(),
+            }
+
     def annotate(**map):
         if util.binary(fctx.data()):
             mt = (mimetypes.guess_type(fctx.path())[0]
@@ -877,6 +887,7 @@  def annotate(web, req, tmpl):
                    "node": f.hex(),
                    "rev": f.rev(),
                    "author": f.user(),
+                   "ancestors": ancestors(f),
                    "desc": f.description(),
                    "extra": f.extra(),
                    "file": f.path(),
diff --git a/mercurial/templates/gitweb/fileannotate.tmpl b/mercurial/templates/gitweb/fileannotate.tmpl
--- a/mercurial/templates/gitweb/fileannotate.tmpl
+++ b/mercurial/templates/gitweb/fileannotate.tmpl
@@ -64,6 +64,15 @@  annotate |
 </div>
 <div class="page_body">
 <table>
+<thead>
+<tr>
+ <th style="text-align: right;">ancestor(s)</th>
+ <th>rev</th>
+ <th></th>
+ <th>line no</th>
+ <th>source</th>
+</tr>
+</thead>
 {annotate%annotateline}
 </table>
 </div>
diff --git a/mercurial/templates/gitweb/map b/mercurial/templates/gitweb/map
--- a/mercurial/templates/gitweb/map
+++ b/mercurial/templates/gitweb/map
@@ -97,6 +97,9 @@  fileline = '
 annotateline = '
   <tr id="{lineid}" style="font-family:monospace" class="parity{parity}">
     <td class="linenr" style="text-align: right;">
+      {join(ancestors%annotateentry, "/")}
+    </td>
+    <td class="linenr" style="text-align: left;">
       <a href="{url|urlescape}annotate/{node|short}/{file|urlescape}{sessionvars%urlparameter}#l{targetline}"
          title="{node|short}: {desc|escape|firstline}">{author|user}@{rev}</a>
     </td>
@@ -109,6 +112,13 @@  annotateline = '
     <td><pre><a class="linenr" href="#{lineid}">{linenumber}</a></pre></td>
     <td><pre>{line|escape}</pre></td>
   </tr>'
+annotateentry = '
+  <a href="{url|urlescape}annotate/{node|short}/{file|urlescape}{sessionvars%urlparameter}"
+     title="{node|short}: {desc|escape|firstline}"
+     class="revision-ancestor">
+    {author|user}@{rev}
+  </a>'
+
 difflineplus = '
   <a href="#{lineid}"></a><span id="{lineid}" class="difflineplus">{strip(line|escape, '\r\n')}</span>'
 difflineminus = '
diff --git a/mercurial/templates/static/style-gitweb.css b/mercurial/templates/static/style-gitweb.css
--- a/mercurial/templates/static/style-gitweb.css
+++ b/mercurial/templates/static/style-gitweb.css
@@ -28,6 +28,7 @@  div.list_head { padding:6px 8px 4px; bor
 a.list { text-decoration:none; color:#000000; }
 a.list:hover { text-decoration:underline; color:#880000; }
 table { padding:8px 4px; }
+thead { background-color: #edece6; }
 th { padding:2px 5px; font-size:12px; text-align:left; }
 tr.dark, .parity1, pre.sourcelines.stripes > :nth-child(4n+4) { background-color:#f6f6f0; }
 tr.light:hover, .parity0:hover, tr.dark:hover, .parity1:hover,
@@ -52,6 +53,7 @@  div.pre { font-family:monospace; font-si
 div.diff_info { font-family:monospace; color:#000099; background-color:#edece6; font-style:italic; }
 div.index_include { border:solid #d9d8d1; border-width:0px 0px 1px; padding:12px 8px; }
 div.search { margin:4px 8px; position:absolute; top:56px; right:12px }
+a.revision-ancestor { font-style: italic; text-decoration: none }
 .linenr { color:#999999; text-decoration:none }
 div.rss_logo { float: right; white-space: nowrap; }
 div.rss_logo a {
diff --git a/tests/test-hgweb-symrev.t b/tests/test-hgweb-symrev.t
--- a/tests/test-hgweb-symrev.t
+++ b/tests/test-hgweb-symrev.t
@@ -619,6 +619,7 @@  Set up the repo
   <a href="/annotate/43c799df6e75/foo?style=gitweb#l1"
   <a href="/diff/43c799df6e75/foo?style=gitweb#l1"
   <a href="/rev/43c799df6e75?style=gitweb"
+  <a href="/annotate/43c799df6e75/foo?style=gitweb"
   <a href="/annotate/a7c1559b7bba/foo?style=gitweb#l2"
   <a href="/diff/a7c1559b7bba/foo?style=gitweb#l2"
   <a href="/rev/a7c1559b7bba?style=gitweb"
diff --git a/tests/test-hgweb.t b/tests/test-hgweb.t
--- a/tests/test-hgweb.t
+++ b/tests/test-hgweb.t
@@ -340,7 +340,7 @@  static file
 
   $ get-with-headers.py --twice localhost:$HGPORT 'static/style-gitweb.css' - date etag server
   200 Script output follows
-  content-length: 6521
+  content-length: 6624
   content-type: text/css
   
   body { font-family: sans-serif; font-size: 12px; border:solid #d9d8d1; border-width:1px; margin:10px; background: white; color: black; }
@@ -373,6 +373,7 @@  static file
   a.list { text-decoration:none; color:#000000; }
   a.list:hover { text-decoration:underline; color:#880000; }
   table { padding:8px 4px; }
+  thead { background-color: #edece6; }
   th { padding:2px 5px; font-size:12px; text-align:left; }
   tr.dark, .parity1, pre.sourcelines.stripes > :nth-child(4n+4) { background-color:#f6f6f0; }
   tr.light:hover, .parity0:hover, tr.dark:hover, .parity1:hover,
@@ -397,6 +398,7 @@  static file
   div.diff_info { font-family:monospace; color:#000099; background-color:#edece6; font-style:italic; }
   div.index_include { border:solid #d9d8d1; border-width:0px 0px 1px; padding:12px 8px; }
   div.search { margin:4px 8px; position:absolute; top:56px; right:12px }
+  a.revision-ancestor { font-style: italic; text-decoration: none }
   .linenr { color:#999999; text-decoration:none }
   div.rss_logo { float: right; white-space: nowrap; }
   div.rss_logo a {