GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Count the complete Windows locale encoding suffix by youknowdot · Pull Request #9065 · RustPython/RustPython

Conversation

youknowdot commented Oct 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Summary

Extract the Python 3.14-compatible Windows locale suffix guard from #8954.

Count every byte after the first dot toward the existing 15-byte encoding-name limit, including an @modifier. The existing LC_ALL helper still validates semicolon-separated components individually. This matches CPython 3.14.7 and 3.14.8, which reject before calling the CRT.

The original guard from #6416 incorrectly stopped counting at @. Existing canonical long-encoding tests cover other suffix lengths; the small Python snippet adds only the missing modifier cases for LC_CTYPE and LC_ALL. Both test strings exceed the guard and cannot reach the CRT with this fix.

Validation and platform limits

Independent source review confirms both inputs return the existing locale.Error before CString conversion or the host call.
Normal repository hooks and the Linux native build passed.
Both Linux snippet harness entries pass, but the Windows assertions are platform-skipped. This is not Windows runtime validation.
No defective Windows baseline was executed, and no local Windows toolchain is installed.
Linux stdlib and separately configured CAPI Clippy pass with -D warnings. Hosted Windows build and snippet results are required before treating platform validation as complete.

No locale setting is changed by the new regression cases. No canonical assertions, README wording, or implementation-name guards are changed.

Summary by CodeRabbit

Bug Fixes
- On Windows, setting LC_CTYPE or LC_ALL to a locale name with an overlong encoding portion now raises the expected “unsupported locale setting” error. The length check includes the full portion after the period, including any modifier suffix, so these invalid settings are rejected consistently.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Count the complete Windows locale encoding suffix

6e7e7e5

Include modifiers in the 15-byte bound before calling the CRT, matching Python 3.14.

Assisted-by: OpenAI Codex:model identifier unavailable

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: a5836347-4c08-44fb-951c-080c906b1760

📥 Commits

Reviewing files that changed from the base of the PR and between b63c3b0 and 6e7e7e5.

📒 Files selected for processing (2)

crates/stdlib/src/locale.rs
extra_tests/snippets/stdlib_locale.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

📝 Walkthrough

Walkthrough

Windows locale-name validation now checks the full substring after the period against MAX_CP_LEN. A Windows-only test checks that overlong names for LC_CTYPE and LC_ALL raise the expected error.

Changes

Windows locale validation

Layer / File(s)	Summary
Validate locale name length `crates/stdlib/src/locale.rs`, `extra_tests/snippets/stdlib_locale.py`	The Windows validation checks the full substring after the period against `MAX_CP_LEN`. Tests check that overlong names for `LC_CTYPE` and `LC_ALL` raise `locale.Error("unsupported locale setting")`.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to 6e7e7

Overlong Windows locale suffixes, including modifier bytes, are rejected for LC_CTYPE and LC_ALL. No concrete code blocker is evident; complete the stated hosted Windows validation before considering platform validation complete.

Pre-merge checks | 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: counting the complete Windows locale encoding suffix, including modifiers, within the length limit.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone approved these changes

Oct 10, 2026

View reviewed changes

youknowone marked this pull request as ready for review

October 10, 2026 12:57

youknowone enabled auto-merge (squash)

October 10, 2026 12:57

chatgpt-codex-connector Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T13:01:19.412407Z	`6e7e7e5`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chatgpt-codex-connector Bot reviewed

Oct 10, 2026

View reviewed changes

chatgpt-codex-connector Bot left a comment

Copy link

Copy Markdown

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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e7e7e5cfd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

extra_tests/snippets/stdlib_locale.py

Comment on lines +9 to +10

		(locale.LC_CTYPE, "English.1252@" + "x" * 16),
		(locale.LC_ALL, "LC_CTYPE=English.1252@" + "x" * 16),

chatgpt-codex-connector Bot Oct 10, 2026

Copy link

Copy Markdown

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.

Remove the prohibited new locale test

This new snippet adds test logic and data, but the root AGENTS.md restricts test-file changes to adding or removing expected-failure decorators and associated TODO comments; these new LC_CTYPE/LC_ALL cases therefore violate the repository's critical test-modification policy and should be removed from this change.

AGENTS.md reference: AGENTS.md:L273-L279

Useful? React with 👍 / 👎.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

codspeed Bot commented Oct 10, 2026

Copy link

Copy Markdown

Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks¹

_{Comparing youknowdot:fix-windows-locale-suffix-length (6e7e7e5) with main (b63c3b0)}

Footnotes

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone merged commit 9220438 into RustPython:main

Oct 10, 2026

31 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Reviewers

chatgpt-codex-connector[bot]

chatgpt-codex-connector[bot] left review comments

youknowone

youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

youknowdot commented Oct 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation and platform limits

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 10, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

youknowdot commented Oct 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

chatgpt-codex-connector Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.