| 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 <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.
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
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; }