Skip to content

Fix locale region override#7337

Merged
robertbastian merged 3 commits into
unicode-org:mainfrom
robertbastian:urg
Dec 22, 2025
Merged

Fix locale region override#7337
robertbastian merged 3 commits into
unicode-org:mainfrom
robertbastian:urg

Conversation

@robertbastian

@robertbastian robertbastian commented Dec 22, 2025

Copy link
Copy Markdown
Member

Simpler version of #7335

#4413

FixedCalendarDateTimeNames<icu_calendar::Gregorian>,
typed_date_time_names_size,
328
336

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Observation: Unfortunate that it increases stack size by 8 bytes

let result = self
.region
.to_tinystr()
.to_ascii_lowercase()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: is this a bugfix?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, it wasn't lowercasing the region as required and your test failed

.keywords
.get(&crate::extensions::unicode::key!("sd"))
.and_then(|v| v.as_single_subtag().copied());
.and_then(|v| SubdivisionId::try_from_str(v.as_single_subtag()?.as_str()).ok());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggestion: Add comments about invalid subtags being silently ignored?

Suggestion/Follow-up: Check that the subdivision region matches the subtag region?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's existing behaviour, we need to do more work here anyway

@robertbastian robertbastian merged commit f16ea51 into unicode-org:main Dec 22, 2025
31 checks passed
@robertbastian robertbastian deleted the urg branch December 22, 2025 18:29
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.

2 participants