Fredrik discovered another round-tripping issue related to ignorables parsing. My code had previously assumed that ...
6 years, 12 months ago
(2017-04-22 06:12:10 UTC)
#1
Fredrik discovered another round-tripping issue related to ignorables parsing.
My code had previously assumed that runs of ignorables at the beginning of a
string could be skipped because the main parse loop would be accepting those
ignorables. However, this trick doesn't work when you are in the middle of
consuming an affix pattern, because ignorables are no longer accepted in the
main parse loop.
This patch removes the branch of the code with special handling for the
beginning of strings and instead treats runs of ignorables at the beginning of a
string as it would those in the middle of the string. The main downside is that
this comes at a performance cost, because more parse paths have the potential to
be created and kept alive.
LGTM I can't really say anything very useful about the implementation, as I'm wholly unfamiliar ...
6 years, 11 months ago
(2017-04-26 21:45:38 UTC)
#2
LGTM
I can't really say anything very useful about the implementation, as I'm wholly
unfamiliar with this code, but luckily Andy is here to review that. ;-)
The test cases do however look good and I've also verified that the change fixes
the exact test case that caused me to find the problem in the first place.
https://codereview.appspot.com/320520043/diff/1/icu4j/main/tests/core/src/com...
File icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
(right):
https://codereview.appspot.com/320520043/diff/1/icu4j/main/tests/core/src/com...
icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java:4993:
// If the symbols ever change in locale data, please call the setters so that
this test
It would now be more logical if you had this first, before pattern1, instead of
having it between pattern1 and pattern2.
Issue 320520043: Fixing round-trip issue with updated Farsi data
(Closed)
Created 6 years, 12 months ago by sffc
Modified 6 years, 11 months ago
Reviewers: andy.heninger, roubert (google)
Base URL: svn+icussh://source.icu-project.org/repos/icu/trunk/
Comments: 1