Wrap ICU4C DecimalFormat around NumberFormatter and new parsing code

Description

Parsing code in ICU4J: #13513

Port of parsing code to ICU4C: #13574

Follow-up for post-merge changes: #13725

Activity

Show:
TracBot
July 1, 2018, 12:04 AM
Trac Comment 2 by —2018-04-24T02:37:40.741Z

Code review containing the diff between branch and trunk:

https://codereview.appspot.com/347730043/

Andy and I went over this diff to make sure that all files being checked-in contain intentional changes.

TracBot
July 1, 2018, 12:04 AM
Trac Comment 4 by —2018-06-01T23:51:49.550Z

Review Comments. Reviewing DecimalFormat as a whole, not trying to sort the changes committed on this ticket.

decimfmt.h:785
@param newvalue

newvalue -> newValue

decimfmt.h:1763 `virtual void setParseCaseSensitive(UBool value);`
missing @param value in the doc comments.

decimfmt.h:1780 `virtual void setFormatFailIfMoreThanMaxDigits(UBool value);`
missing @param value in the doc comments.

decimfmt.h:1728-1781
remove `{@icu}` from doc comments. Not relevant to C++.

Having noticed the above docs problems by hand, check the doxygen warnings in decimfmt.h ...

Error handling in DecimalFormat constructors is incomplete. With U_FAILURE on the incoming status, the object needs to be initialized enough that it will destruct cleanly, and nothing else should be done. The original status should be maintained. It's OK for other methods (aside from the destructor) to crash with the failed-to-construct object, unless they themselves are called with an incoming error, in which case they should just return.

Test case showing a problem:

result

(More comments to come.)

TracBot
July 1, 2018, 12:04 AM
Trac Comment 5.6 by —2018-06-08T21:24:05.533Z

Replying to (Comment 5 andy):

Markus suggests creating followup ticket(s) for post-release candidate fixes. Definitely want one for cleaning up the error code handling.

decimfmt.cpp:367 // !// TODO: What is parseError for? //

It returns the position of any syntax errors in the pattern string.
If that's not readily available, it should probably be set to the start of the string.

TracBot
July 1, 2018, 12:04 AM
Trac Comment 7 by —2018-06-13T17:09:30.139Z

Follow-up for doxygen: #13833

Follow-up for error code handling: #13828

Fixed

Assignee

Shane Carr

Reporter

Shane Carr

Components

Labels

None

Reviewer

None

Priority

major

Time Needed

None

Fix versions

Configure