Add support for E PluralOperand

[zbraniecki]

Fixes #228

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• components/plurals/src/operands.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[zbraniecki]

This seems to be ready for review.

The issue is performance. It impacts it a lot. Like, FromStr benchmarks get 70-100% slower, and the plurals/operands/overview benchmark which benchmarks all types of inputs is getting ~18% hit (due to the FromStr being slower).

I could microoptimize it by refactoring to do a single iteration over the string, but since we want to switch FromStr to use FixedDecimal then maybe there's not that much value in it.

I'll consider trying just for fun, but since any potential optimizations are local to the FromStr impl, I think this PR is ready for review.

[codecov-io]

Codecov Report

Merging #407 (df5d2e8) into master (d0ee68b) will increase coverage by 0.08%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master     #407      +/-   ##
==========================================
+ Coverage   74.83%   74.91%   +0.08%     
==========================================
  Files          90       90              
  Lines        4641     4661      +20     
==========================================
+ Hits         3473     3492      +19     
- Misses       1168     1169       +1     

Impacted Files	Coverage Δ
components/ecma402/src/pluralrules.rs	92.05% <ø> (ø)
components/plurals/src/rules/ast.rs	35.29% <ø> (ø)
components/plurals/src/rules/serializer.rs	62.61% <0.00%> (ø)
components/plurals/src/rules/lexer.rs	89.13% <50.00%> (-0.87%)	⬇️
components/plurals/src/rules/parser.rs	91.61% <87.50%> (+0.42%)	⬆️
components/plurals/src/operands.rs	91.95% <100.00%> (+0.92%)	⬆️
components/plurals/src/rules/resolver.rs	94.87% <100.00%> (+0.13%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0ee68b...df5d2e8. Read the comment docs.

[coveralls]

Pull Request Test Coverage Report for Build cefed3b5c91be645e7c1898167199fd1358bcf01-PR-407

• 21 of 25 (84.0%) changed or added relevant lines in 5 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.09%) to 74.111%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
components/plurals/src/rules/parser.rs	7	8	87.5%
components/plurals/src/rules/serializer.rs	0	3	0.0%

Files with Coverage Reduction	New Missed Lines	%
components/plurals/src/rules/serializer.rs	1	62.04%

Totals
Change from base Build d0ee68bf679bd5bb49cb1514025fcec0428ff618:	0.09%
Covered Lines:	3607
Relevant Lines:	4867

---

💛 - Coveralls

[echeran]

LGTM. Seems pretty straightforward, and as you said, it makes sense to not think about the performance impact yet since you'll be moving stuff around including some of this into FixedDecimal as a followup anyways.

[sffc]

Here's the official language on e vs c:

Added new operand 'c' (with a synonym 'e') for languages like French (CLDR-12010)

So, in other words, we should name the field "c", but we should accept an alias "e" when parsing the rules.

https://sites.google.com/site/cldr/index/downloads/cldr-38?pli=1#TOC-Migration

[filmil]

nit: consider being a bit more verbose about what the exponent means?

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• components/ecma402/src/pluralrules.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot