-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JDK-6285888: ChoiceFormat can support unescaped relational symbols in the Format segment #17640
Conversation
👋 Welcome back jlu! A progress list of the required criteria for merging this PR into |
@justin-curtis-lu The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
* <p>If a <i>Relation</i> symbol is to be used within a <i>Format</i> pattern, | ||
* it must be single quoted. For example, | ||
* {@code new ChoiceFormat("1# '#'1 ").format(1)} returns {@code " #1 "}. | ||
* Use two single quotes in a row to produce a literal single quote. For example, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds right to remove this restriction, but if we were to use |
inside Format
, shouldn't it still need to be single-quoted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the restriction shouldn't be removed, but rather updated. Fixed in 740686b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@justin-curtis-lu This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 20 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
/integrate |
Going to push as commit d3c3194.
Your commit was automatically rebased without conflicts. |
@justin-curtis-lu Pushed as commit d3c3194. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Please review this PR and CSR which allows for the Format segment of a ChoiceFormat pattern to use the ChoiceFormat Relational symbols without the need to escape them using quotes. Previously, using a non escaped Relational symbol within a Format segment would throw an IAE.
Implementation wise, this can be achieved by adding an additional check of
part == 0
to the relational symbol conditional. This is safe to do, as any subsequent relational symbols should only come after a '|' is parsed, which sets part back to 0. This ensures that for the duration ofpart = 1
(Format segment), the relational symbols can be used without syntactic meaning.For example, this change allows behavior such as:
new ChoiceFormat("1#The code is #7281")
. Previously, the workaround would have been newChoiceFormat("1#The code is '#'7281")
to achieve the same formatted behavior. Please see the CSR for more detail.Additionally, this change also includes an unrelated grouping of the ChoiceFormat examples under a single header: Usage Information.
Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17640/head:pull/17640
$ git checkout pull/17640
Update a local copy of the PR:
$ git checkout pull/17640
$ git pull https://git.openjdk.org/jdk.git pull/17640/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 17640
View PR using the GUI difftool:
$ git pr show -t 17640
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17640.diff
Webrev
Link to Webrev Comment