Patchwork [1,of,3,V2] gitweb: highlight data of the current revision in annotate view

login
register
mail settings
Submitter Denis Laxalde
Date June 9, 2016, 12:34 p.m.
Message ID <3064963ddbc0b509c0f3.1465475696@sh77.tls.logilab.fr>
Download mbox | patch
Permalink /patch/15447/
State Changes Requested
Headers show

Comments

Denis Laxalde - June 9, 2016, 12:34 p.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1464877610 -7200
#      Thu Jun 02 16:26:50 2016 +0200
# Node ID 3064963ddbc0b509c0f3f8149ca2c6edb2edb568
# Parent  75a86bf04cb2a2b74e6ca26f742f4f8b97c4a008
gitweb: highlight data of the current revision in annotate view

* Distinguish the /annotate/<revision>/<file>#<linenumber> link when it would
  lead to the current page (i.e. <revision> is the current revision) (style it
  gray and undecorated). This indicates more clearly that this is a "dead-end"
  in blame navigation.

* Display lines changed in current revision in green.
Anton Shestakov - June 9, 2016, 1:37 p.m.
09.06.2016, 20:35, "Denis Laxalde" <denis.laxalde@logilab.fr>:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1464877610 -7200
> # Thu Jun 02 16:26:50 2016 +0200
> # Node ID 3064963ddbc0b509c0f3f8149ca2c6edb2edb568
> # Parent 75a86bf04cb2a2b74e6ca26f742f4f8b97c4a008
> gitweb: highlight data of the current revision in annotate view
>
> * Distinguish the /annotate/<revision>/<file>#<linenumber> link when it would
>   lead to the current page (i.e. <revision> is the current revision) (style it
>   gray and undecorated). This indicates more clearly that this is a "dead-end"
>   in blame navigation.
>
> * Display lines changed in current revision in green.
>
> diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
> --- a/mercurial/hgweb/webcommands.py
> +++ b/mercurial/hgweb/webcommands.py
> @@ -858,6 +858,7 @@ def annotate(web, req, tmpl):
>      The ``fileannotate`` template is rendered.
>      """
>      fctx = webutil.filectx(web.repo, req)
> + blamedrev = fctx.rev()
>      f = fctx.path()
>      parity = paritygen(web.stripecount)
>      diffopts = patch.difffeatureopts(web.repo.ui, untrusted=True,
> @@ -876,6 +877,7 @@ def annotate(web, req, tmpl):
>              yield {"parity": next(parity),
>                     "node": f.hex(),
>                     "rev": f.rev(),
> + "blamedrev": blamedrev,
>                     "author": f.user(),
>                     "desc": f.description(),
>                     "extra": f.extra(),
> diff --git a/mercurial/templates/gitweb/map b/mercurial/templates/gitweb/map
> --- a/mercurial/templates/gitweb/map
> +++ b/mercurial/templates/gitweb/map
> @@ -98,10 +98,11 @@ annotateline = '
>    <tr id="{lineid}" style="font-family:monospace" class="parity{parity}">
>      <td class="linenr" style="text-align: right;">
>        <a href="{url|urlescape}annotate/{node|short}/{file|urlescape}{sessionvars%urlparameter}#l{targetline}"
> + {ifeq(rev, blamedrev, 'class="blamedrev"', '')}
>           title="{node|short}: {desc|escape|firstline}">{author|user}@{rev}</a>
>      </td>
>      <td><pre><a class="linenr" href="#{lineid}">{linenumber}</a></pre></td>
> - <td><pre>{line|escape}</pre></td>
> + <td><pre{ifeq(rev, blamedrev, ' class="blamedrev"', '')}>{line|escape}</pre></td>
>    </tr>'

It's also possible to set the class to the whole <tr>, which already has a class, that would avoid the duplication.

Nit: I actually liked the name "thisrev" more, maybe because revision is more like a generic context in hgweb, and, technically, we don't blame the revision, we're blaming a file (but for this revision).

Also, while "blame" is an alias to "annotate", it's used very rarely in hgweb and in Mercurial in general (in some part because of the negative connotation).
Augie Fackler - June 10, 2016, 3:34 a.m.
On Thu, Jun 09, 2016 at 09:37:19PM +0800, Anton Shestakov wrote:
> 09.06.2016, 20:35, "Denis Laxalde" <denis.laxalde@logilab.fr>:
> > # HG changeset patch
> > # User Denis Laxalde <denis.laxalde@logilab.fr>
> > # Date 1464877610 -7200
> > # Thu Jun 02 16:26:50 2016 +0200
> > # Node ID 3064963ddbc0b509c0f3f8149ca2c6edb2edb568
> > # Parent 75a86bf04cb2a2b74e6ca26f742f4f8b97c4a008
> > gitweb: highlight data of the current revision in annotate view
> >
> > * Distinguish the /annotate/<revision>/<file>#<linenumber> link when it would
> >   lead to the current page (i.e. <revision> is the current revision) (style it
> >   gray and undecorated). This indicates more clearly that this is a "dead-end"
> >   in blame navigation.
> >
> > * Display lines changed in current revision in green.
> >
> > diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
> > --- a/mercurial/hgweb/webcommands.py
> > +++ b/mercurial/hgweb/webcommands.py
> > @@ -858,6 +858,7 @@ def annotate(web, req, tmpl):
> >      The ``fileannotate`` template is rendered.
> >      """
> >      fctx = webutil.filectx(web.repo, req)
> > + blamedrev = fctx.rev()

I agree with Anton - this would be better named "annotrev" or
something so it's consistent with the annotate terminology.

> >      f = fctx.path()
> >      parity = paritygen(web.stripecount)
> >      diffopts = patch.difffeatureopts(web.repo.ui, untrusted=True,
> > @@ -876,6 +877,7 @@ def annotate(web, req, tmpl):
> >              yield {"parity": next(parity),
> >                     "node": f.hex(),
> >                     "rev": f.rev(),
> > + "blamedrev": blamedrev,
> >                     "author": f.user(),
> >                     "desc": f.description(),
> >                     "extra": f.extra(),
> > diff --git a/mercurial/templates/gitweb/map b/mercurial/templates/gitweb/map

I assume patches for other styles are forthcoming once we get
consensus on the initial version for gitweb?


> > --- a/mercurial/templates/gitweb/map
> > +++ b/mercurial/templates/gitweb/map
> > @@ -98,10 +98,11 @@ annotateline = '
> >    <tr id="{lineid}" style="font-family:monospace" class="parity{parity}">
> >      <td class="linenr" style="text-align: right;">
> >        <a href="{url|urlescape}annotate/{node|short}/{file|urlescape}{sessionvars%urlparameter}#l{targetline}"
> > + {ifeq(rev, blamedrev, 'class="blamedrev"', '')}
> >           title="{node|short}: {desc|escape|firstline}">{author|user}@{rev}</a>
> >      </td>
> >      <td><pre><a class="linenr" href="#{lineid}">{linenumber}</a></pre></td>
> > - <td><pre>{line|escape}</pre></td>
> > + <td><pre{ifeq(rev, blamedrev, ' class="blamedrev"', '')}>{line|escape}</pre></td>
> >    </tr>'
>
> It's also possible to set the class to the whole <tr>, which already has a class, that would avoid the duplication.
>
> Nit: I actually liked the name "thisrev" more, maybe because revision is more like a generic context in hgweb, and, technically, we don't blame the revision, we're blaming a file (but for this revision).
>
> Also, while "blame" is an alias to "annotate", it's used very rarely in hgweb and in Mercurial in general (in some part because of the negative connotation).
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Denis Laxalde - June 10, 2016, 9:43 a.m.
Augie Fackler a écrit :
> I assume patches for other styles are forthcoming once we get
> consensus on the initial version for gitweb?

Done V3 series.
Yuya Nishihara - June 10, 2016, 1:18 p.m.
On Thu, 9 Jun 2016 23:34:50 -0400, Augie Fackler wrote:
> On Thu, Jun 09, 2016 at 09:37:19PM +0800, Anton Shestakov wrote:
> > 09.06.2016, 20:35, "Denis Laxalde" <denis.laxalde@logilab.fr>:  
> > > # HG changeset patch
> > > # User Denis Laxalde <denis.laxalde@logilab.fr>
> > > # Date 1464877610 -7200
> > > # Thu Jun 02 16:26:50 2016 +0200
> > > # Node ID 3064963ddbc0b509c0f3f8149ca2c6edb2edb568
> > > # Parent 75a86bf04cb2a2b74e6ca26f742f4f8b97c4a008
> > > gitweb: highlight data of the current revision in annotate view
> > >
> > > * Distinguish the /annotate/<revision>/<file>#<linenumber> link when it would
> > >   lead to the current page (i.e. <revision> is the current revision) (style it
> > >   gray and undecorated). This indicates more clearly that this is a "dead-end"
> > >   in blame navigation.
> > >
> > > * Display lines changed in current revision in green.
> > >
> > > diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
> > > --- a/mercurial/hgweb/webcommands.py
> > > +++ b/mercurial/hgweb/webcommands.py
> > > @@ -858,6 +858,7 @@ def annotate(web, req, tmpl):
> > >      The ``fileannotate`` template is rendered.
> > >      """
> > >      fctx = webutil.filectx(web.repo, req)
> > > + blamedrev = fctx.rev()  
> 
> I agree with Anton - this would be better named "annotrev" or
> something so it's consistent with the annotate terminology.
> 
> > >      f = fctx.path()
> > >      parity = paritygen(web.stripecount)
> > >      diffopts = patch.difffeatureopts(web.repo.ui, untrusted=True,
> > > @@ -876,6 +877,7 @@ def annotate(web, req, tmpl):
> > >              yield {"parity": next(parity),
> > >                     "node": f.hex(),
> > >                     "rev": f.rev(),
> > > + "blamedrev": blamedrev,
> > >                     "author": f.user(),
> > >                     "desc": f.description(),
> > >                     "extra": f.extra(),

I meant "originalnode", literally. It's sort of black magic, but can avoid
introducing new keyword since fctx.node() is passed to the templater.

https://selenic.com/repo/hg/file/3.8.3/mercurial/templater.py#l399

Patch

diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
--- a/mercurial/hgweb/webcommands.py
+++ b/mercurial/hgweb/webcommands.py
@@ -858,6 +858,7 @@  def annotate(web, req, tmpl):
     The ``fileannotate`` template is rendered.
     """
     fctx = webutil.filectx(web.repo, req)
+    blamedrev = fctx.rev()
     f = fctx.path()
     parity = paritygen(web.stripecount)
     diffopts = patch.difffeatureopts(web.repo.ui, untrusted=True,
@@ -876,6 +877,7 @@  def annotate(web, req, tmpl):
             yield {"parity": next(parity),
                    "node": f.hex(),
                    "rev": f.rev(),
+                   "blamedrev": blamedrev,
                    "author": f.user(),
                    "desc": f.description(),
                    "extra": f.extra(),
diff --git a/mercurial/templates/gitweb/map b/mercurial/templates/gitweb/map
--- a/mercurial/templates/gitweb/map
+++ b/mercurial/templates/gitweb/map
@@ -98,10 +98,11 @@  annotateline = '
   <tr id="{lineid}" style="font-family:monospace" class="parity{parity}">
     <td class="linenr" style="text-align: right;">
       <a href="{url|urlescape}annotate/{node|short}/{file|urlescape}{sessionvars%urlparameter}#l{targetline}"
+         {ifeq(rev, blamedrev, 'class="blamedrev"', '')}
          title="{node|short}: {desc|escape|firstline}">{author|user}@{rev}</a>
     </td>
     <td><pre><a class="linenr" href="#{lineid}">{linenumber}</a></pre></td>
-    <td><pre>{line|escape}</pre></td>
+    <td><pre{ifeq(rev, blamedrev, ' class="blamedrev"', '')}>{line|escape}</pre></td>
   </tr>'
 difflineplus = '
   <a href="#{lineid}"></a><span id="{lineid}" class="difflineplus">{strip(line|escape, '\r\n')}</span>'
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
@@ -52,6 +52,8 @@  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.blamedrev { color:#999999; text-decoration: none }
+pre.blamedrev { color:#009900; }
 .linenr { color:#999999; text-decoration:none }
 div.rss_logo { float: right; white-space: nowrap; }
 div.rss_logo a {
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: 6607
   content-type: text/css
   
   body { font-family: sans-serif; font-size: 12px; border:solid #d9d8d1; border-width:1px; margin:10px; background: white; color: black; }
@@ -397,6 +397,8 @@  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.blamedrev { color:#999999; text-decoration: none }
+  pre.blamedrev { color:#009900; }
   .linenr { color:#999999; text-decoration:none }
   div.rss_logo { float: right; white-space: nowrap; }
   div.rss_logo a {