[PATCH] hgweb: add line wrapping switch with javascript

Alexander Plavin me at aplavin.ru
Fri Jul 12 14:22:09 UTC 2013


2013/7/12 Martin Geisler <martin at geisler.net>:
> Alexander Plavin <me at aplavin.ru> writes:
>
>> # HG changeset patch
>> # User Alexander Plavin <me at aplavin.ru>
>> # Date 1373630293 -14400
>> #      Fri Jul 12 15:58:13 2013 +0400
>> # Node ID 161cf6af00a4187cf2ebc19fe95f1b990de84133
>> # Parent  6187d6255fd2a14d7d21e27cd0d86ce6f282a86c
>> hgweb: add line wrapping switch with javascript
>>
>> diff -r 6187d6255fd2 -r 161cf6af00a4 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,6 +67,7 @@
>>  </table>
>>
>>  <div class="overflow">
>> +<div class="sourcefirst" style="float: right;">line wrap: <a class="linewraplink" href="javascript:setLinewrap('off')">on</a></div>
>>  <div class="sourcefirst"> line source</div>
>>  <pre class="sourcelines">{text%fileline}</pre>
>>  <div class="sourcelast"></div>
>> diff -r 6187d6255fd2 -r 161cf6af00a4 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
>> @@ -269,3 +269,16 @@
>>       document.getElementById('diffstatdetails').style.display = flag ? 'inline' : 'none';
>>       document.getElementById('diffstatexpand').style.display = flag ? 'none' : 'inline';
>>  }
>> +
>> +function setLinewrap(flag) {
>> +    var nodes = document.querySelectorAll('.sourcelines > span');
>> +    for (var i = 0; i < nodes.length; i++) {
>> +        nodes[i].style.whiteSpace = flag == 'on' ? 'pre-wrap' : 'pre';
>> +    }
>
> Would it not be better to set a class (such as "wrap") on the pre
> element and then add
>
>   pre.wrap span {
>       white-spare: pre-wrap;
>   }
>
> to the stylesheet? That way you don't need to run through a potentially
> huge number of lines and we can collect the style information in the
> stylesheet.

Agree, changed this.

>
>> +    var links = document.getElementsByClassName('linewraplink');
>
> I would prefer something like
>
>        var not_flag = flag == 'on' ? 'off' : 'on';
>> +    for (var i = 0; i < links.length; i++) {
>            links[i].innerHTML = flag;
>            links[i].href = 'javascript:setLinewrap("' + not_flag + '");'
>> +    }
>> +}
>
> Alternatively, and probably better: avoid the flag parameter and let the
> DOM keep the state instead. That is, if the "wrap" class is there, then
> remove it and update the innerHTML appropriatedly -- there is then no
> need to update the link since toggleLinewrap (new name for the function)
> would work without any arguments.

Yes, this really looks better :)

Thanks for your comments here. Sent a new version considering these suggestions.

>
> --
> Martin Geisler



More information about the Mercurial-devel mailing list