-
Notifications
You must be signed in to change notification settings - Fork 7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[misc] speedup month & era locale handling #5581
Conversation
} | ||
// Sorting makes sure if one month (or abbr) is a prefix of another it | ||
// will match the longer piece. | ||
shortPieces.sort(cmpLenRev); | ||
longPieces.sort(cmpLenRev); | ||
mixedPieces.sort(cmpLenRev); | ||
for (i = 0; i < 12; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could change functionality, correct?
It's first a sort, then escape. Your change escapes, then sorts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regexEscape
function may change the length of shortP, longP, and mixedP, leading a wrong sorting, so I changed the order of escapes and sorts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you ever want to sort before you escape the regex?
Does it make sense to test this behavior?
Merged in a3babc5 |
[feature] Optimize computeErasParse and computeMonthsParse
I tweaked a bit the eras code, was double-escaping the regex for the mixed case. |
summary
Optimize
computeErasParse
andcomputeMonthsParse
, reduce unnecessary regular escaping and loop traversal.