> Please review this PR which handles an edge case pattern bug with 
> ChoiceFormat.
> 
> 
> var d = new ChoiceFormat("0#foo|1#bar|baz|") // creates cFmt equivalent to 
> "0.0#foo|1.0#bar|1.0#"
> d.format(1) // unexpectedly returns ""
> 
> 
> Not only does this lead to faulty formatting results, but breaks the 
> ChoiceFormat class invariant of duplicate limits.
> 
> It would have been preferable if either an exception was thrown when the 
> ChoiceFormat was initially created, or the ChoiceFormat formatting 1 returned 
> a value of "bar".
> 
> For comparison,
> 
> var d = new ChoiceFormat("0#foo|1#bar|baz") // creates cFmt equivalent to 
> "0.0#foo|1.0#bar"
> d.format(1) // returns "bar"
> 
> 
> After this change, the first code snippet now returns "bar" when formatting 1 
> and discards the "baz" as the second code snippet does.
> 
> The fix prevents a limit/format entry from being built when the current 
> parsing mode has not yet processed the limit and relation. The previous 
> implementation would build an additional limit/format of 1 with an empty 
> string. The `break` allows for the discarding of the incorrect portion and 
> continuing. Alternatively an exception could have been thrown. For 
> consistency with the second code snippet, the former was picked.
> 
> https://github.com/openjdk/jdk/pull/17856 may be of interest, which is a 
> semi-related specification fix.

Justin Lu has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains 11 additional commits since the 
last revision:

 - ammend comment on parsing
 - Merge branch 'master' into JDK-8325898-ChoiceFormat-trailingPipeBug
 - use enhanced switch in stringToNum
 - revert enum change
 - improve variable name for StringBuilder
 - null check at public level. spacing adjustment.
 - add additional longer test case
 - shorten relational symbol in format comment
 - preserve final modifier for next/previousDouble
 - reduce visibility of Segment enum
 - ... and 1 more: https://git.openjdk.org/jdk/compare/f8638598...05051975

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/17883/files
  - new: https://git.openjdk.org/jdk/pull/17883/files/de38a988..05051975

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17883&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17883&range=04-05

  Stats: 15147 lines in 427 files changed: 8844 ins; 3288 del; 3015 mod
  Patch: https://git.openjdk.org/jdk/pull/17883.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17883/head:pull/17883

PR: https://git.openjdk.org/jdk/pull/17883

Reply via email to