Skip to content

ICU-23345 Require Range to be a range in UTFStringCodePoints to improve error messages#3733

Merged
eggrobin merged 1 commit intounicode-org:mainfrom
eggrobin:range-range
Apr 13, 2026
Merged

ICU-23345 Require Range to be a range in UTFStringCodePoints to improve error messages#3733
eggrobin merged 1 commit intounicode-org:mainfrom
eggrobin:range-range

Conversation

@eggrobin
Copy link
Copy Markdown
Member

@eggrobin eggrobin commented Oct 22, 2025

Suggested by @jmr after seeing the error messages generated by UTFStringCodePoints<..., char> (which was how this API worked in an earlier draft).

TODO: Fill out the checklist below.

Checklist

  • Required: Issue filed: ICU-23345
  • Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
  • Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@eggrobin
Copy link
Copy Markdown
Member Author

@markusicu, should this target main on a new ticket, or 78 maintenance on a reopened ICU-23004?

@eggrobin eggrobin requested a review from markusicu October 22, 2025 14:12
Comment thread icu4c/source/common/unicode/utfiterator.h
@markusicu
Copy link
Copy Markdown
Member

markusicu commented Oct 23, 2025

@markusicu, should this target main on a new ticket, or 78 maintenance on a reopened ICU-23004?

@eggrobin is this PR fixing problems, or just "improv[ing] error messages"?
If it's an important bug fix, then it should go onto the maintenance branch, with the existing ticket.
If it's merely an improvement, then it should go onto main for ICU 79.

@mihnita @richgillam FYI

Copy link
Copy Markdown
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm as an improvement for ICU 79 -- please create an ICU 79 ticket for such improvements

Copy link
Copy Markdown
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please create an ICU 79 ticket for this

Comment thread icu4c/source/common/unicode/utfiterator.h Outdated
@eggrobin eggrobin changed the title Require Range to be a range in UTFStringCodePoints to improve error messages ICU-23345 Require Range to be a range in UTFStringCodePoints to improve error messages Mar 26, 2026
@eggrobin eggrobin marked this pull request as ready for review March 26, 2026 17:29
Comment thread icu4c/source/common/unicode/utfiterator.h
@richgillam
Copy link
Copy Markdown
Contributor

This LGTM, but it involves newer C++ idioms that I'm not familiar with, so take everything I say with a grain of salt.

Copy link
Copy Markdown
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to write a meaningful unit test for this?

@eggrobin
Copy link
Copy Markdown
Member Author

Is it possible to write a meaningful unit test for this?

If it doesn’t change the API, not within the language.

It might be useful to set up a test harness that puts expectations on error messages, but not today.

@jira-pull-request-webhook
Copy link
Copy Markdown

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@markusicu
Copy link
Copy Markdown
Member

FYI: CI checks finished & passed (and I approved last week)

@eggrobin eggrobin merged commit b04af5f into unicode-org:main Apr 13, 2026
98 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants