ICU4C PluralRules - first implementation #222
Conversation
executors/cpp/plural_rules.cpp
Outdated
| input_is_double = true; | ||
| cout << "# DOUBLE STRING " << input_double_sample << endl; | ||
| } else | ||
| if (sample_string.find('c') != std::string::npos) { |
There was a problem hiding this comment.
The test for compact decimal should come before the test of a decimal point because 1.2c6 is a valid compact decimal number, but 3.1415926 isn't.
There was a problem hiding this comment.
And a value can be both a double and a compact. This will need more work to see if ICU4C can handle compact input.
There was a problem hiding this comment.
I think you still want to keep the handling of compact decimal numbers separate from regular floating point. The parsing for both is not the same, for starters.
So you want an if () { ...} else if () ... rather than if () {...} if () {...}
@sffc How do we want to handle compact input number strings for the ICU4C executor for plural rules? Because we have internal code to do so, but it's just not public.
executors/cpp/plural_rules.cpp
Outdated
| // Convert into an integer, decimal, or compact decimal | ||
| input_double_sample = std::stod(sample_string); | ||
| input_is_double = true; | ||
| cout << "# DOUBLE STRING " << input_double_sample << endl; |
There was a problem hiding this comment.
Remove low level stdout printing statements, or else use a logging library and convert to debug level log line.
update: based on the TODO statements, it seems like this PR is meant to have a followup PR, and the stdout printing is a best approximation of debugging for development purposes. If that is the case, then instead of removing the stdout printing prematurely, add a few TODO comments to remove the stdout printing.
There was a problem hiding this comment.
This is mostly ready for release now. There's a big gap in both Node and CPP implementation, i.e., plural ranges. That should be a separate PR.
There was a problem hiding this comment.
Update code, fixing many lint complaints.
executors/cpp/plural_rules.cpp
Outdated
| input_is_double = true; | ||
| cout << "# DOUBLE STRING " << input_double_sample << endl; | ||
| } else | ||
| if (sample_string.find('c') != std::string::npos) { |
There was a problem hiding this comment.
I think you still want to keep the handling of compact decimal numbers separate from regular floating point. The parsing for both is not the same, for starters.
So you want an if () { ...} else if () ... rather than if () {...} if () {...}
@sffc How do we want to handle compact input number strings for the ICU4C executor for plural rules? Because we have internal code to do so, but it's just not public.
|
I'm not sure what to do with Compact vs. Decimal. Here are some options:
@sffc, please advise. |
|
I'd appreciate more guidance on how to handle compact. Can we submit and then fix? |
|
Please mark the compact values as "unsupported". I think we established that they are currently supported in ICU4X only. |
|
I will set compact to unsupported. |
|
PTAL: linted... |
|
PTAL so we can add this component to ICU4C. |
No description provided.