[PATCH 2 of 2] hgweb: optimize process_dates function

[Laurens Holst]

Op 08-08-13 15:16, Alexander Plavin schreef:
> # HG changeset patch
> # User Alexander Plavin <alexander at plav.in>
> # Date 1374682432 -14400
> #      Wed Jul 24 20:13:52 2013 +0400
> # Node ID 2e3064b1a8b3719bc896e61a5804fd515a0bc9cb
> # Parent  d2e91a2d576deff3bb97fefc1fe71c2a64d64bb6
> hgweb: optimize process_dates function
>
> This function looped over every node due to getElementsByTagName('*'), instead
> of using selectors. In this patch we use querySelectorAll('.age') and process
> only these nodes, which is much faster and also doesn't require extra condition.
>
> Browser compatibility isn't sacrificed: IE 8+, FF 3.5+, Opera 10+.

How much do we care about compatibility with IE8?

> diff -r d2e91a2d576d -r 2e3064b1a8b3 mercurial/templates/static/mercurial.js
> --- a/mercurial/templates/static/mercurial.js	Wed Jul 24 18:53:55 2013 +0400
> +++ b/mercurial/templates/static/mercurial.js	Wed Jul 24 20:13:52 2013 +0400
> @@ -245,21 +245,18 @@
>   		}
>   	}
>   
> -    var nodes = document.getElementsByTagName('*');
> -    var ageclass = new RegExp('\\bage\\b');
> +    var nodes = document.querySelectorAll('.age');

Possibly getElementsByClassName() is more efficient.

Looks like it’s only supported in IE9 though. (Other browsers actually 
had it *before* querySelectorAll().)

>       var dateclass = new RegExp('\\bdate\\b');

I would use a literal /\bdate\b/ instead of new RegExp().

>       for (var i=0; i<nodes.length; ++i){
>           var node = nodes[i];
>           var classes = node.className;
> -        if (ageclass.test(classes)){
> -            var agevalue = age(node.textContent);
> -            if (dateclass.test(classes)){
> -                // We want both: date + (age)
> -                node.textContent += ' ('+agevalue+')';
> -            } else {
> -                node.title = node.textContent;
> -                node.textContent = agevalue;
> -            }
> +        var agevalue = age(node.textContent);
> +        if (dateclass.test(classes)){

As you’re using classList elsewhere, I think you could use 
classList.contains() instead of the regexp match?

Though MDN tells me it is only supported in IE10 onwards, but if that’s 
an argument then we shouldn’t be using it in the other place either.

> +            // We want both: date + (age)
> +            node.textContent += ' ('+agevalue+')';
> +        } else {
> +            node.title = node.textContent;
> +            node.textContent = agevalue;
>           }
>       }
>   }
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>