moment.fn.format escape multiple characters is partially broken by Localized date format

[yelvert]

It appears that the substitution for localized date format doesn't respect the multiple character escape.

moment().format('[Last] dddd') #> "MM/DD/YYYYast Monday"
moment().format('[LLL] LLL') #> "MMMM D YYYY h:mm A July 30 2012 8:47 AM"

[rockymeza]

The problem is with how moment expands localFormattingTokens on line 381.

while (localFormattingTokens.test(format)) {
    format = format.replace(localFormattingTokens, replaceLongDateFormatTokens);
}

@timrwood, I'm not sure about how to fix this.

[timrwood]

So the way this works in 1.7.0 is that while there are still local formatting tokens in the string, it will replace them with the correct replacement. This is done to handle the recursive tokens like below.

LLL > MMMM D YYYY LT > MMMM D YYYY h:mm A

However, this also opens the possibility of an infinite loop if moment.longDateFormat.LLLL = 'LLLL'. The fix I made was to match anything inside braces in the localFormattingTokens regex, and only recurse twice through replacing local formatting tokens. This will cause things like the following not to work, but I don't think they are good practice anyway.

moment.longDateFormat.LT = "h:mm A";
moment.longDateFormat.L = "MM/DD/YYYY";
moment.longDateFormat.LL = "MMMM D YYYY";
moment.longDateFormat.LLL = "LL LT";
moment.longDateFormat.LLLL = "dddd, LLL";

If we think the above should be supported we'll have to think of another way to limit the while loop recursion. Maybe like this?

var lang = getLangDefinition(m), i = 5;

...

while (i-- && localFormattingTokens.test(format)) {
    format = format.replace(localFormattingTokens, replaceLongDateFormatTokens);
}

[yelvert]

Why would the localDate formats not be implemented the same as other formats?

P.S. I'm on my phone and don't have the code in front of me, so this question may sound uninformed.

On Aug 3, 2012, at 11:28 AM, Tim Woodreply@reply.github.com wrote:

So the way this works in 1.7.0 is that while there are still local formatting tokens in the string, it will replace them with the correct replacement. This is done to handle the recursive tokens like below.

LLL > MMMM D YYYY LT > MMMM D YYYY h:mm A

However, this also opens the possibility of an infinite loop if moment.longDateFormat.LLLL = 'LLLL'. The fix I made was to match anything inside braces in the localFormattingTokens regex, and only recurse twice through replacing local formatting tokens. This will cause things like the following not to work, but I don't think they are good practice anyway.

moment.longDateFormat.LT = "h:mm A";
moment.longDateFormat.L = "MM/DD/YYYY";
moment.longDateFormat.LL = "MMMM D YYYY";
moment.longDateFormat.LLL = "LL LT";
moment.longDateFormat.LLLL = "dddd, LLL";

If we think the above should be supported we'll have to think of another way to limit the while loop recursion. Maybe like this?

var lang = getLangDefinition(m), i = 5;

...

while (i-- && localFormattingTokens.test(format)) {
   format = format.replace(localFormattingTokens, replaceLongDateFormatTokens);
}

---

Reply to this email directly or view it on GitHub:
#380 (comment)

[rockymeza]

The localDate formats are actually more like macros than formats. They don't correspond to values, they correspond to other formats.

[yelvert]

Ah ok, makes sense. In that case, would it be possible to apply the same escaping logic as the others?

[timrwood]

Yeah, the commit I added fixes this, but it doesn't handle more than 2 levels of recursion as shown in my comment above.

[timrwood]

Oops, forgot to tag this commit. 12aad5f

I added up to 5 levels of recursion and an early escape if it doesn't match L LL LLL LLLL LT. This should handle all the use cases outlined above.

[timrwood]

This has been released with 1.7.1. #384