Refreshing Number Parsing: ICU4J

http://bugs.icu-project.org/trac/ticket/13513

[sffc, 2017-12-08 09:12:23]

Adding AffixMatcher and SymbolMatcher.  AffixMatcher favors longer affixes over
shorter ones.  Not all possible features have been added to AffixMatcher.

[sffc, 2017-12-08 09:13:02]

Uploaded today's work.

[sffc, 2017-12-12 06:35:28]

Cleaning up code. Adding a few more test cases for the AffixMatcher.

[sffc, 2017-12-12 09:32:16]

Adding a matcher for scientific notation.  Renamed MantissaMatcher ->
DecimalMatcher.

[sffc, 2017-12-12 11:48:45]

More features added and code cleaned up a bit.  Please take another look.  Still
need currency parsing and testing.

[sffc, 2017-12-13 04:26:55]

Adding currency matcher.

[sffc, 2017-12-13 05:06:19]

Resetting file version history to new branch.

[sffc, 2017-12-13 05:08:48]

Updating base of code to new branch.

https://codereview.appspot.com/335150043/diff/100001/main/classes/core/.exter...
File main/classes/core/.externalToolBuilders/copy-data-core.launch (left):

https://codereview.appspot.com/335150043/diff/100001/main/classes/core/.exter...
main/classes/core/.externalToolBuilders/copy-data-core.launch:1: <?xml
version="1.0" encoding="UTF-8" standalone="no"?>
As usual, please ignore these eclipse config files.  I'm not committing them.

https://codereview.appspot.com/335150043/diff/100001/main/classes/core/.setti...
File main/classes/core/.settings/org.eclipse.jdt.core.prefs (left):

https://codereview.appspot.com/335150043/diff/100001/main/classes/core/.setti...
main/classes/core/.settings/org.eclipse.jdt.core.prefs:1:
eclipse.preferences.version=1
As usual, please ignore these eclipse config files.  I'm not committing them.

https://codereview.appspot.com/335150043/diff/100001/main/classes/core/.setti...
File main/classes/core/.settings/org.eclipse.jdt.ui.prefs (left):

https://codereview.appspot.com/335150043/diff/100001/main/classes/core/.setti...
main/classes/core/.settings/org.eclipse.jdt.ui.prefs:1:
eclipse.preferences.version=1
As usual, please ignore these eclipse config files.  I'm not committing them.

https://codereview.appspot.com/335150043/diff/100001/main/classes/core/core-b...
File main/classes/core/core-build.launch (left):

https://codereview.appspot.com/335150043/diff/100001/main/classes/core/core-b...
main/classes/core/core-build.launch:1: <?xml version="1.0" encoding="UTF-8"
standalone="no"?>
As usual, please ignore these eclipse config files.  I'm not committing them.

https://codereview.appspot.com/335150043/diff/100001/main/tests/core/.setting...
File main/tests/core/.settings/org.eclipse.jdt.core.prefs (left):

https://codereview.appspot.com/335150043/diff/100001/main/tests/core/.setting...
main/tests/core/.settings/org.eclipse.jdt.core.prefs:1:
eclipse.preferences.version=1
As usual, please ignore these eclipse config files.  I'm not committing them.

https://codereview.appspot.com/335150043/diff/100001/main/tests/core/.setting...
File main/tests/core/.settings/org.eclipse.jdt.ui.prefs (left):

https://codereview.appspot.com/335150043/diff/100001/main/tests/core/.setting...
main/tests/core/.settings/org.eclipse.jdt.ui.prefs:1:
eclipse.preferences.version=1
As usual, please ignore these eclipse config files.  I'm not committing them.

[sffc, 2017-12-13 10:05:13]

My branch is here:

http://source.icu-project.org/repos/icu/branches/shane/numberformat3/

[sffc, 2017-12-13 10:07:57]

Starting to tie in with upstream.

[sffc, 2017-12-14 10:28:18]

Most of the way through the data-driven file. Still thinking about how to handle
a few strange cases involving affixes and currencies.

[sffc, 2017-12-15 00:16:27]

Re-thinking affix matchers a bit. Affix matchers now contain only regular
characters, not symbols.

[sffc, 2017-12-16 10:08:15]

Data-driven unit tests are passing.

[andy.heninger, 2018-01-17 02:00:13]

https://codereview.appspot.com/335150043/diff/120001/main/classes/core/src/co...
File main/classes/core/src/com/ibm/icu/impl/number/parse/StringSegment.java
(right):

https://codereview.appspot.com/335150043/diff/120001/main/classes/core/src/co...
main/classes/core/src/com/ibm/icu/impl/number/parse/StringSegment.java:27:
assert start <= end;
For a class like this, it would probably be better to unconditionally throw
IllegalArgument or something similar, instead of assert(). Or, if you want to
stay with assert(), leave the object in a somewhat sane state when the asserts
are turned off.

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/.exter...
File main/classes/core/.externalToolBuilders/copy-data-core.launch (left):

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/.exter...
main/classes/core/.externalToolBuilders/copy-data-core.launch:1: <?xml
version="1.0" encoding="UTF-8" standalone="no"?>
This comment doesn't belong with this file, but I didn't see where else to leave
it.

When importing icu into Eclipse, the various icu projects are all named
icu4j-something. Importing the branch branches/shane/numberformat3, there new
projects currency-numeric and toolsj don't fit this pattern.

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
File main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParseMatcher.java
(right):

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParseMatcher.java:27:
* Should return a set representing all possible chars (UTF-16 code units) that
could be the first char that this
Would be better to use first code points, not code units.

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
File main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java
(right):

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java:242:
char leadChar = ignoreCase ? ParsingUtils.getCaseFoldedLeadingChar(segment) :
segment.charAt(0);
leadCodePoint might be simpler.

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java:252:
// The following line resets the offset so that the StringSegment says the same
across the function
says -> stays ?

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java:277:
for (int charsToConsume = 1; charsToConsume <= segment.length();
charsToConsume++) {
I was thinking about if this loop could be run the other direction, starting
with a long match and working towards short. Most matchers would consume way
less than initially given, and the loop would continue from the previous
consumption less one. But I don't think it gains anything - maybeMore should
also be effective in limiting how many times the loop runs.

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
File main/classes/core/src/com/ibm/icu/impl/number/parse/ParsingUtils.java
(right):

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
main/classes/core/src/com/ibm/icu/impl/number/parse/ParsingUtils.java:16: * Adds
all chars and lead surrogates from input into output.
I'm a little wary of dealing explicitly with separated surrogates, rather than
always getting code points out of the input and dealing only with them. The
logic shouldn't change if strings were UTF-32 or UTF-8, except for the lowest
level functions that pull code points out of strings. Leading character
optimizations would be leading code point, not leading code unit.

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
File main/classes/core/src/com/ibm/icu/impl/number/parse/StringSegment.java
(right):

https://codereview.appspot.com/335150043/diff/180001/main/classes/core/src/co...
main/classes/core/src/com/ibm/icu/impl/number/parse/StringSegment.java:98: if
(ignoreCase) {
Simple case folding is OK? Not full Unicode folding, where SS matches the German
ß.

[sffc, 2018-01-18 11:03:14]

Replying to Andy feedback round one. See commit message.