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

Fix zone parsing in potentially ambiguous cases (fixes #1336) #1347

Merged
merged 3 commits into from Dec 17, 2013

Conversation

Projects
None yet
2 participants
@pimterry
Contributor

pimterry commented Dec 11, 2013

This patch changes zone() and parseZone(), so that zone(tzString) now parses for timezones from the right, and parseZone() now uses _tmz data as specified by the format string, if available (and continues to fallback to zone(originalInput) if not).

@ichernev

This comment has been minimized.

Contributor

ichernev commented Dec 12, 2013

Can you please make sure that both cases of the parseZone are hit (from _tzm and from last tz-looking thing in the string) -- and put a comment in the test case. Also global regexes tend to remember where you matched them last. Is that why you switches the rexexp.exec(string) to string.match(regex) (RegExp MDN lastIndex property).

@pimterry

This comment has been minimized.

Contributor

pimterry commented Dec 15, 2013

I've added a test covering that in more detail, and rebased this branch back up to date. Does that look better?

And yes, the switch from regex.exec to string.match is entirely to get around the lastIndex state. Unlike regex.exec, string.match doesn't track any extra state, so the results are always independent and consistent; it gives you back the full array of all the possible matches every time.

@ichernev

This comment has been minimized.

Contributor

ichernev commented Dec 17, 2013

Looks great! Thank you very much!

ichernev added a commit that referenced this pull request Dec 17, 2013

Merge pull request #1347 from pimterry/fixZoneParsing
Fix zone parsing in potentially ambiguous cases (fixes #1336)

@ichernev ichernev merged commit bcec595 into moment:develop Dec 17, 2013

1 check passed

default The Travis CI build passed
Details

@pimterry pimterry deleted the pimterry:fixZoneParsing branch Dec 24, 2013

@theurere theurere referenced this pull request Jul 1, 2014

Merged

Update moment.js to 2.7.0 #58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment