Open
Conversation
Collaborator
|
Hi and thanks for this! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The latest v8 (current Chrome and Node 25) introduces a few nasty changes:
nbsp; there's also now U+202F ("narrow non-breaking space"). This affects Luxon in two ways: a) thefromFormatparsing code will now expect those new kinds of spaces, and b) the test code that makes assertions on the the output oftoFormatshould ignore different-kind-of-whitespace differences.2. There's a bug in v8 (or maybe the ICU data) that affects theI misunderstood the output here. I undid the changes related to this.copticcalendar. It seems to provide "AM" and "PM" aseravalues. That's weird and I didn't debug it further, since it's easy to reproduce directly in intl. Instead, I switched the tests that usecopticto instead usebuddhistthlocale (Thai) no longer produces a localized meridiem string. I've simply removed the uses of it from the tests.You'll notice there's only one "real" code change: item 1a. Before we handled the NBSP character by replacing any whitespace literals in the token regexes with a character class that included both a regular space and the nbsp character (i.e.
[ ${nbsp}]). I've switched it to replace anything matching whitespace character in the regex with the whitespace character class, so for example,/foo bar/will become/foo\sbar/. I think this is the correct fix, and likely what we should have done all along.The other changes are all test-related