928693 - Use string.repeat(n) instead of (new Array(n + 1)).join(string)

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v1

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/repeat

It's faster, the code is shorter and no temporary is being used.

Comment 1

[Magnus Melin [:mkmelin]]

Comment on attachment 819425 [details] [diff] [review]
patch, v1

Review of attachment 819425 [details] [diff] [review]:
-----------------------------------------------------------------

Looks goog to me! r=mkmelin

::: mailnews/extensions/newsblog/content/FeedItem.js
@@ +353,5 @@
>      let source =
>        openingLine +
>        'X-Mozilla-Status: 0000\n' +
>        'X-Mozilla-Status2: 00000000\n' +
> +      'X-Mozilla-Keys: ' + " ".repeat(79) + '\n' +

I think this one is actually supposed to be 80...

::: mailnews/extensions/newsblog/content/feed-subscriptions.js
@@ +2021,5 @@
>  
>    generateOutlineStruct: function(baseFolder, parent, indentLevel)
>    {
>      // Pretty printing.
> +    function indentString(len) { return " ".repeat(len - 2) };

add a ; after 2) while you're there

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v2, r=mkmelin

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/23afadef993f

Comment 4

[alta88]

(In reply to Magnus Melin from comment #1)
> Comment on attachment 819425 [details] [diff] [review]
> patch, v1
> 
> Review of attachment 819425 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks goog to me! r=mkmelin
> 
> ::: mailnews/extensions/newsblog/content/FeedItem.js
> @@ +353,5 @@
> >      let source =
> >        openingLine +
> >        'X-Mozilla-Status: 0000\n' +
> >        'X-Mozilla-Status2: 00000000\n' +
> > +      'X-Mozilla-Keys: ' + " ".repeat(79) + '\n' +
> 
> I think this one is actually supposed to be 80...

the length should be 80; repeat(80) makes it 81..  it may break something later.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

According to http://mxr.mozilla.org/comm-central/source/mailnews/base/public/nsMsgLocalFolderHdrs.h#36 , the part following "X-Mozilla-Keys: " should be 80 whitespaces long. This patch does that. The previous code only put 79 whitespaces there (joining an array of 80 items).

Comment 6

[Magnus Melin [:mkmelin]]

Yes, that's my understanding too.

Comment 7

[alta88]

hmm. prior to bug 761656 and the join, there was a : plus a space, plus 79 spaces.  so it seems to have been wrong from the beginning, as a pop3 item is : plus space plus 80.

in any case, just a note that it's now different than originally coded.

and it seems irrelevant, as tags overflowing 80 are merely stored elsewhere, so the hardcoded header length seems anachronistic and an artifact from when there were the 5 labels.