Patchwork [1,of,3,RFC,V4] hgweb: code selection without line numbers in file source view

login
register
mail settings
Submitter Alexander Plavin
Date July 4, 2013, 10:36 a.m.
Message ID <a1de30293d0e140b14e9.1372934188@debian-alexander.dolgopa>
Download mbox | patch
Permalink /patch/1790/
State Superseded, archived
Headers show

Comments

Alexander Plavin - July 4, 2013, 10:36 a.m.
# HG changeset patch
# User Alexander Plavin <me@aplavin.ru>
# Date 1372933124 -14400
#      Thu Jul 04 14:18:44 2013 +0400
# Node ID a1de30293d0e140b14e9d8ed5f2e7540907a81b1
# Parent  d7b4aa1049d3fbbc2b8d24114afad82586c49db3
hgweb: code selection without line numbers in file source view
Martin Geisler - July 4, 2013, 10:26 p.m.
Alexander Plavin <me@aplavin.ru> writes:

> # HG changeset patch
> # User Alexander Plavin <me@aplavin.ru>
> # Date 1372933124 -14400
> #      Thu Jul 04 14:18:44 2013 +0400
> # Node ID a1de30293d0e140b14e9d8ed5f2e7540907a81b1
> # Parent  d7b4aa1049d3fbbc2b8d24114afad82586c49db3
> hgweb: code selection without line numbers in file source view

Thanks for turning my demo into a full patch!

I would use the commit message to explain a few things:

- The requirements you have evaluated the code against:leading,
  embedded, and trailing whitespace/tabs in both the first and other
  lines; the ability to select part of lines (that ruled out using a
  elements around the entire line).

- which browsers you have tested with and their versions.

- How the new code degrades if browsers don't support this or that
  feature. How many users will this affect?

- Performance of the new solution. Does it work with a 10,000 line file?

- A note saying that the patch is based on my work :)

Basically a summary of the discussion we've had on the list so that
people can see why you picked this solution over the others.

Finally: does this not affect any tests? If the tests need updating,
then you should put that into this commit. Otherwise the commit
introduces test failures.
Alexander Plavin - July 4, 2013, 10:32 p.m.
Firstly, thanks for the idea and implementation!

2013/7/5 Martin Geisler <martin@geisler.net>:
> Alexander Plavin <me@aplavin.ru> writes:
>
>> # HG changeset patch
>> # User Alexander Plavin <me@aplavin.ru>
>> # Date 1372933124 -14400
>> #      Thu Jul 04 14:18:44 2013 +0400
>> # Node ID a1de30293d0e140b14e9d8ed5f2e7540907a81b1
>> # Parent  d7b4aa1049d3fbbc2b8d24114afad82586c49db3
>> hgweb: code selection without line numbers in file source view
>
> Thanks for turning my demo into a full patch!
>
> I would use the commit message to explain a few things:

This is RFC, as a flag says, so commit message will be extended for
sure. I've sent it to be sure everyone agrees with this solution.

>
> - The requirements you have evaluated the code against:leading,
>   embedded, and trailing whitespace/tabs in both the first and other
>   lines; the ability to select part of lines (that ruled out using a
>   elements around the entire line).
>
> - which browsers you have tested with and their versions.
>
> - How the new code degrades if browsers don't support this or that
>   feature. How many users will this affect?
>
> - Performance of the new solution. Does it work with a 10,000 line file?
>
> - A note saying that the patch is based on my work :)
>
> Basically a summary of the discussion we've had on the list so that
> people can see why you picked this solution over the others.
>
> Finally: does this not affect any tests? If the tests need updating,
> then you should put that into this commit. Otherwise the commit
> introduces test failures.

Tests are also not modified yet, for the same reason.

>
> --
> Martin Geisler
Pierre-Yves David - July 4, 2013, 10:58 p.m.
On 5 juil. 2013, at 00:32, Alexander Plavin wrote:

> Firstly, thanks for the idea and implementation!
> 
> 2013/7/5 Martin Geisler <martin@geisler.net>:
>> Alexander Plavin <me@aplavin.ru> writes:
>> 
>>> # HG changeset patch
>>> # User Alexander Plavin <me@aplavin.ru>
>>> # Date 1372933124 -14400
>>> #      Thu Jul 04 14:18:44 2013 +0400
>>> # Node ID a1de30293d0e140b14e9d8ed5f2e7540907a81b1
>>> # Parent  d7b4aa1049d3fbbc2b8d24114afad82586c49db3
>>> hgweb: code selection without line numbers in file source view
>> 
>> Thanks for turning my demo into a full patch!
>> 
>> I would use the commit message to explain a few things:
> 
> This is RFC, as a flag says, so commit message will be extended for
> sure. I've sent it to be sure everyone agrees with this solution.

But we can't comment something that you does explain. In order to evaluate this patch we need to understand how you solution achieve its goal, the pro and cons of the approach, etc.

One line commit description are be banned on a general basis.

Patch

diff -r d7b4aa1049d3 -r a1de30293d0e mercurial/templates/paper/filerevision.tmpl
--- a/mercurial/templates/paper/filerevision.tmpl	Sat Jun 29 14:36:51 2013 +0400
+++ b/mercurial/templates/paper/filerevision.tmpl	Thu Jul 04 14:18:44 2013 +0400
@@ -68,7 +68,7 @@ 
 
 <div class="overflow">
 <div class="sourcefirst"> line source</div>
-{text%fileline}
+<pre class="sourcelines">{text%fileline}</pre>
 <div class="sourcelast"></div>
 </div>
 </div>
diff -r d7b4aa1049d3 -r a1de30293d0e mercurial/templates/paper/map
--- a/mercurial/templates/paper/map	Sat Jun 29 14:36:51 2013 +0400
+++ b/mercurial/templates/paper/map	Thu Jul 04 14:18:44 2013 +0400
@@ -72,7 +72,7 @@ 
 filecomparison = filecomparison.tmpl
 filelog = filelog.tmpl
 fileline = '
-  <div class="parity{parity} source"><a href="#{lineid}" id="{lineid}">{linenumber}</a> {line|escape}</div>'
+  <span id="{lineid}">{strip(line|escape, '\r\n')}</span><a href="#{lineid}"></a>'
 filelogentry = filelogentry.tmpl
 
 annotateline = '
diff -r d7b4aa1049d3 -r a1de30293d0e mercurial/templates/static/style-paper.css
--- a/mercurial/templates/static/style-paper.css	Sat Jun 29 14:36:51 2013 +0400
+++ b/mercurial/templates/static/style-paper.css	Thu Jul 04 14:18:44 2013 +0400
@@ -209,6 +209,44 @@ 
 .source a { color: #999; font-size: smaller; font-family: monospace;}
 .bottomline { border-bottom: 1px solid #999; }
 
+.sourcelines {
+  font-size: 90%;
+  position: relative;
+}
+
+.sourcelines > span {
+  display: inline-block;
+  width: 100%;
+  padding: 1px 0px;
+  counter-increment: lineno;
+}
+
+.sourcelines > span:before {
+  -moz-user-select: -moz-none;
+  -khtml-user-select: none;
+  -webkit-user-select: none;
+  -ms-user-select: none;
+  user-select: none;
+  display: inline-block;
+  width: 4em;
+  margin-right: 1em;
+  font-size: smaller;
+  color: #999;
+  text-align: right;
+  content: counter(lineno);
+}
+
+.sourcelines > span:nth-child(4n+1) { background-color: #f0f0f0; }
+.sourcelines > span:nth-child(4n+3) { background-color: white; }
+
+.sourcelines > a {
+    display: inline-block;
+    position: absolute;
+    left: 0px;
+    width: 4em;
+    height: 1em;
+}
+
 .fileline { font-family: monospace; }
 .fileline img { border: 0; }