Skip to content

Implemented ICU4J#5953

Merged
kwvanderlinde merged 12 commits into
RPTools:developfrom
bubblobill:ICU4J-Implementation
Jul 8, 2026
Merged

Implemented ICU4J#5953
kwvanderlinde merged 12 commits into
RPTools:developfrom
bubblobill:ICU4J-Implementation

Conversation

@bubblobill

@bubblobill bubblobill commented Mar 26, 2026

Copy link
Copy Markdown
Collaborator

Identify the Bug or Feature request

progresses #5952

Description of the Change

Implemented ICU4J which allows the use of named parameters in message pattern strings.

This will improve the life of translators where parameters with useful names can provide context for the phrase.
Example:
Invalid condition in {0}({1}) roll option.
vs.
Invalid condition in {branchType}({condition}) roll option.

Added ParserExceptionBuilder class for easier construction of error messages using named parameters.

Additionally: Fixed issue with & replacement. & is now only removed if it is immediately followed by a alphanumeric character, and does not form part of a HTML entity. Strings like the following should render correctly:

States & Bars
Sights, Lights & Auras
Measures how opaque the "soft fog" overlay is drawn (0-255).

Possible Drawbacks

Having to create a map of name-value pairs can be tiresome. Thus the exception builder.

Documentation Notes

Implemented ICU4J

Release Notes

Improved context for future translation strings.


This change is Reviewable

Added ParserExceptionBuilder class for easy construction using named
parameters.
@bubblobill bubblobill requested a review from cwisniew March 26, 2026 08:31
Comment thread common/src/main/java/net/rptools/maptool/language/AbstractMessageBuilder.java Outdated
@bubblobill bubblobill requested a review from cwisniew May 13, 2026 02:20
@kwvanderlinde

Copy link
Copy Markdown
Collaborator

I don't see where ICU4J is put to use. Looks like the named parameters work just as well without it. And the AbstractMessageFactory and implementations aren't used anywhere either. It's just the changes in I18N itself that seem to be doing anything.

@bubblobill

bubblobill commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

@kwvanderlinde
Import changed from Java.text.MessageFormat. That's why it works with named values.

The message factory is a convenience tool. I'm not about to start writing new/rewriting translations without the change being adopted. Then you might see it used.

@kwvanderlinde

Copy link
Copy Markdown
Collaborator

@kwvanderlinde Import changed from Java.text.MessageFormat. That's why it works with named values.

See it now. Definitely off my game 😅

Comment thread common/src/main/java/net/rptools/maptool/language/AbstractMessageFactory.java Outdated
Comment thread common/src/main/java/net/rptools/maptool/language/I18N.java Outdated
@kwvanderlinde

Copy link
Copy Markdown
Collaborator

I think the changes look pretty good. Just a couple nitpicks.

@kwvanderlinde kwvanderlinde moved this to Awaiting-Review in MapTool 1.19 Jun 30, 2026
@github-project-automation github-project-automation Bot moved this from Awaiting-Review to In-Progress in MapTool 1.19 Jun 30, 2026
@bubblobill bubblobill requested a review from kwvanderlinde July 1, 2026 07:18
@kwvanderlinde

Copy link
Copy Markdown
Collaborator

@cwisniew Do you have any further thoughts on this one? I think it's good to go.

@kwvanderlinde kwvanderlinde dismissed cwisniew’s stale review July 8, 2026 23:26

Requested changes have been implemented

@kwvanderlinde

Copy link
Copy Markdown
Collaborator

I'll take silence as implicit permission 😅 We can always revisit if needed.

@kwvanderlinde kwvanderlinde added this pull request to the merge queue Jul 8, 2026
Merged via the queue into RPTools:develop with commit f6394be Jul 8, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from In-Progress to Merged in MapTool 1.19 Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

3 participants