[PATCH V2] hgweb: add line wrapping switch to file source view
Alexander Plavin
me at aplavin.ru
Mon Jul 15 09:11:43 CDT 2013
2013/7/15 Laurens Holst <laurens.nospam at grauw.nl>:
> Op 14-07-13 00:07, Alexander Plavin schreef:
>
>> # HG changeset patch
>> # User Alexander Plavin <me at aplavin.ru>
>> # Date 1373630293 -14400
>> # Fri Jul 12 15:58:13 2013 +0400
>> # Node ID ff3a665349adccb190786a81bdb201efce895b1d
>> # Parent 6c49903f66be1a7185d54e1e0df5419d64238e11
>> hgweb: add line wrapping switch to file source view
>>
>> diff -r 6c49903f66be -r ff3a665349ad
>> mercurial/templates/paper/filerevision.tmpl
>> --- a/mercurial/templates/paper/filerevision.tmpl Fri Jul 12
>> 16:01:11 2013 +0400
>> +++ b/mercurial/templates/paper/filerevision.tmpl Fri Jul 12
>> 15:58:13 2013 +0400
>> @@ -67,8 +67,9 @@
>> </table>
>> <div class="overflow">
>> +<div class="sourcefirst linewraptoggle">line wrap: <a
>> class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
>> <div class="sourcefirst"> line source</div>
>> -<pre class="sourcelines">{text%fileline}</pre>
>> +<pre class="sourcelines wrap">{text%fileline}</pre>
>> <div class="sourcelast"></div>
>> </div>
>> </div>
>> diff -r 6c49903f66be -r ff3a665349ad
>> mercurial/templates/static/mercurial.js
>> --- a/mercurial/templates/static/mercurial.js Fri Jul 12 16:01:11 2013
>> +0400
>> +++ b/mercurial/templates/static/mercurial.js Fri Jul 12 15:58:13 2013
>> +0400
>> @@ -271,3 +271,21 @@
>> document.getElementById('diffstatdetails').style.display =
>> curexpand;
>> document.getElementById('diffstatexpand').style.display =
>> curdetails;
>> }
>> +
>> +function toggleLinewrap() {
>> + var new_flag;
>
>
> Initialise new_flag to some value; if by chance the loop has 0 items, you
> will be operating on undefined.
>
> Regardless of whether it can happen in practice right now, it’s a matter of
> robustness; it’s an external factor that is not under control by this
> function, and someone might use the function in some place or alter the
> template without realising the function would need to be adjusted as well.
>
Probably we shouldn't do anything if there are no items to loop through, but ok.
>
>> + var nodes = document.querySelectorAll('.sourcelines');
>> + for (var i = 0; i < nodes.length; i++) {
>> + new_flag = !nodes[i].classList.contains('wrap');
>> + if (new_flag) {
>> + nodes[i].classList.add('wrap');
>> + } else {
>> + nodes[i].classList.remove('wrap');
>> + }
>> + }
>> +
>> + var links = document.getElementsByClassName('linewraplink');
>
>
> Why do you use getElementsByClassName() here but querySelectorAll() before?
> Seems inconsistent.
Yes, there was something different in the first selection, then I
edited it and forgot to change the function called.
>
>
>> + for (var i = 0; i < links.length; i++) {
>> + links[i].innerHTML = new_flag ? 'on' : 'off';
>> + }
>> +}
>
>
> I don’t like how you overwrite new_flag every time you loop over the item.
> Theoretically, if there are two sourcelines sections you could end up in a
> situation where the one is toggled on and the other is toggled off.
As (now) there can be only one switch on the page (not one per
section), I assumed that all the sections have the same value of this,
but your next suggestion looks better.
>
> I would split this up into three functions:
>
> 1. getLinewrap()
> 2. setLinewrap(enable)
> 3. toggleLinewrap()
>
> Where the latter invokes the former two.
>
> function toggleLinewrap() {
> setLinewrap(!getLinewrap());
> }
>
> That way you always just have one state. (You could pass the for the sake of
> optimisation.)
I like this solution. Through getLinewrap still needs to return
something if there are differences between sections, at least this
changes everything consistently.
>
> p.s. Is classList supported by all browsers? It didn’t even exist just a
> couple of years ago...
Of course not by all, but both Chromium 8.0+, FireFox 3.6+ and Opera
11.5+ support it, as well as relatively modern versions of other
browsers, including mobile ones.
>
> ~Laurens
>
>> diff -r 6c49903f66be -r ff3a665349ad
>> mercurial/templates/static/style-paper.css
>> --- a/mercurial/templates/static/style-paper.css Fri Jul 12
>> 16:01:11 2013 +0400
>> +++ b/mercurial/templates/static/style-paper.css Fri Jul 12
>> 15:58:13 2013 +0400
>> @@ -214,11 +214,18 @@
>> position: relative;
>> }
>> +.wrap > span {
>> + white-space: pre-wrap;
>> +}
>> +
>> +.linewraptoggle {
>> + float: right;
>> +}
>> +
>> .sourcelines > span {
>> display: inline-block;
>> width: 100%;
>> padding: 1px 0px;
>> - white-space: pre-wrap;
>> counter-increment: lineno;
>> }
>> diff -r 6c49903f66be -r ff3a665349ad tests/test-hgweb-commands.t
>> --- a/tests/test-hgweb-commands.t Fri Jul 12 16:01:11 2013 +0400
>> +++ b/tests/test-hgweb-commands.t Fri Jul 12 15:58:13 2013 +0400
>> @@ -667,8 +667,9 @@
>> </table>
>> <div class="overflow">
>> + <div class="sourcefirst linewraptoggle">line wrap: <a
>> class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
>> <div class="sourcefirst"> line source</div>
>> - <pre class="sourcelines">
>> + <pre class="sourcelines wrap">
>> <span id="l1">foo</span><a href="#l1"></a></pre>
>> <div class="sourcelast"></div>
>> </div>
>> diff -r 6c49903f66be -r ff3a665349ad tests/test-highlight.t
>> --- a/tests/test-highlight.t Fri Jul 12 16:01:11 2013 +0400
>> +++ b/tests/test-highlight.t Fri Jul 12 15:58:13 2013 +0400
>> @@ -136,8 +136,9 @@
>> </table>
>> <div class="overflow">
>> + <div class="sourcefirst linewraptoggle">line wrap: <a
>> class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
>> <div class="sourcefirst"> line source</div>
>> - <pre class="sourcelines">
>> + <pre class="sourcelines wrap">
>> <span id="l1"><span class="c">#!/usr/bin/env python</span></span><a
>> href="#l1"></a>
>> <span id="l2"></span><a href="#l2"></a>
>> <span id="l3"><span class="sd">"""Fun with generators.
>> Corresponding Haskell implementation:</span></span><a href="#l3"></a>
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
>>
>
More information about the Mercurial-devel
mailing list