Patchwork [4,of,6] hgweb: CSS change to allow correct numbering for multiple code blocks

login
register
mail settings
Submitter Alexander Plavin
Date July 12, 2013, 9:07 p.m.
Message ID <5f28ecff8fd418349183.1373663261@debian-alexander.dolgopa>
Download mbox | patch
Permalink /patch/1849/
State Superseded, archived
Headers show

Comments

Alexander Plavin - July 12, 2013, 9:07 p.m.
# HG changeset patch
# User Alexander Plavin <me@aplavin.ru>
# Date 1373658651 -14400
#      Fri Jul 12 23:50:51 2013 +0400
# Node ID 5f28ecff8fd41834918308628462e0b1a97772b2
# Parent  8458ee0b0df78ae9ff3fbd809e49e290959fcbc7
hgweb: CSS change to allow correct numbering for multiple code blocks
Martin Geisler - July 15, 2013, 7:36 a.m.
Alexander Plavin <me@aplavin.ru> writes:

> # HG changeset patch
> # User Alexander Plavin <me@aplavin.ru>
> # Date 1373658651 -14400
> #      Fri Jul 12 23:50:51 2013 +0400
> # Node ID 5f28ecff8fd41834918308628462e0b1a97772b2
> # Parent  8458ee0b0df78ae9ff3fbd809e49e290959fcbc7
> hgweb: CSS change to allow correct numbering for multiple code blocks
>
> diff -r 8458ee0b0df7 -r 5f28ecff8fd4 mercurial/templates/static/style-paper.css
> --- a/mercurial/templates/static/style-paper.css	Fri Jul 12 23:47:56 2013 +0400
> +++ b/mercurial/templates/static/style-paper.css	Fri Jul 12 23:50:51 2013 +0400
> @@ -216,6 +216,7 @@
>  .sourcelines {
>    font-size: 90%;
>    position: relative;
> +  counter-reset: lineno;
>  }
>  
>  .wrap > span {
> @@ -226,6 +227,9 @@
>      float: right;
>  }
>  
> +.reset-lineno { counter-reset: lineno; }
> +.inc-lineno { counter-increment: lineno; }

You don't seem to be using these newly introduced classes? It is often
easier to review a change when functions/classes are introduced together
with their usage.

Also, when reading the class names I was wondering if they are
necessary? I mean, it would be strange if you had a 'reset-subsection'
class that you add to every h1 element to reset the subsection number in
a document like:

  <h1 class="reset-subsection">
    <h2>
    <h2>
  <h1 class="reset-subsection">
    <h2>
    <h2>
    <h2>
  <h1 class="reset-subsection">
    <h2>

In other words, the logical structure of the document allows you to
reset the subsection counter on all h1 elements. I would have assumed
that something similar was possible here, but without seeing how the
classes are used, I cannot tell yet :)

>  .sourcelines > span {
>    display: inline-block;
>    width: 100%;
> @@ -245,7 +249,7 @@
>    font-size: smaller;
>    color: #999;
>    text-align: right;
> -  content: counter(lineno);
> +  content: counters(lineno, ".");

That might be an unrelated change -- probably a good change, but not
something what would "*allow* correct numbering for multiple code
blocks" as the commit message promises.
Alexander Plavin - July 15, 2013, 4:21 p.m.
2013/7/15 Martin Geisler <martin@geisler.net>:
> Alexander Plavin <me@aplavin.ru> writes:
>
>> # HG changeset patch
>> # User Alexander Plavin <me@aplavin.ru>
>> # Date 1373658651 -14400
>> #      Fri Jul 12 23:50:51 2013 +0400
>> # Node ID 5f28ecff8fd41834918308628462e0b1a97772b2
>> # Parent  8458ee0b0df78ae9ff3fbd809e49e290959fcbc7
>> hgweb: CSS change to allow correct numbering for multiple code blocks
>>
>> diff -r 8458ee0b0df7 -r 5f28ecff8fd4 mercurial/templates/static/style-paper.css
>> --- a/mercurial/templates/static/style-paper.css      Fri Jul 12 23:47:56 2013 +0400
>> +++ b/mercurial/templates/static/style-paper.css      Fri Jul 12 23:50:51 2013 +0400
>> @@ -216,6 +216,7 @@
>>  .sourcelines {
>>    font-size: 90%;
>>    position: relative;
>> +  counter-reset: lineno;
>>  }
>>
>>  .wrap > span {
>> @@ -226,6 +227,9 @@
>>      float: right;
>>  }
>>
>> +.reset-lineno { counter-reset: lineno; }
>> +.inc-lineno { counter-increment: lineno; }
>
> You don't seem to be using these newly introduced classes? It is often
> easier to review a change when functions/classes are introduced together
> with their usage.

Yes, will fold this with the patch where they are used.

>
> Also, when reading the class names I was wondering if they are
> necessary? I mean, it would be strange if you had a 'reset-subsection'
> class that you add to every h1 element to reset the subsection number in
> a document like:
>
>   <h1 class="reset-subsection">
>     <h2>
>     <h2>
>   <h1 class="reset-subsection">
>     <h2>
>     <h2>
>     <h2>
>   <h1 class="reset-subsection">
>     <h2>
>
> In other words, the logical structure of the document allows you to
> reset the subsection counter on all h1 elements. I would have assumed
> that something similar was possible here, but without seeing how the
> classes are used, I cannot tell yet :)

Tried to make this more semantical (and with only one extra class) in
the next version of the patch.

>
>>  .sourcelines > span {
>>    display: inline-block;
>>    width: 100%;
>> @@ -245,7 +249,7 @@
>>    font-size: smaller;
>>    color: #999;
>>    text-align: right;
>> -  content: counter(lineno);
>> +  content: counters(lineno, ".");
>
> That might be an unrelated change -- probably a good change, but not
> something what would "*allow* correct numbering for multiple code
> blocks" as the commit message promises.

It's actually strongly related to the commit name, as this line gives
the ability to get '<n1>.<n2>' numbering (2 numbers separated with a
dot), which is used in hgweb for multiple code blocks.

>
> --
> Martin Geisler

Patch

diff -r 8458ee0b0df7 -r 5f28ecff8fd4 mercurial/templates/static/style-paper.css
--- a/mercurial/templates/static/style-paper.css	Fri Jul 12 23:47:56 2013 +0400
+++ b/mercurial/templates/static/style-paper.css	Fri Jul 12 23:50:51 2013 +0400
@@ -216,6 +216,7 @@ 
 .sourcelines {
   font-size: 90%;
   position: relative;
+  counter-reset: lineno;
 }
 
 .wrap > span {
@@ -226,6 +227,9 @@ 
     float: right;
 }
 
+.reset-lineno { counter-reset: lineno; }
+.inc-lineno { counter-increment: lineno; }
+
 .sourcelines > span {
   display: inline-block;
   width: 100%;
@@ -245,7 +249,7 @@ 
   font-size: smaller;
   color: #999;
   text-align: right;
-  content: counter(lineno);
+  content: counters(lineno, ".");
 }
 
 .sourcelines > span:target {