-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Python ignores whitespace in format strings #6447
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
Conversation
WalkthroughThis PR adds three new spell-check dictionary entries (DICTFLAG, keeped, stginfo) and modifies the struct format string parser in the buffer module to skip whitespace after repeat counts and handle missing format specifiers based on whether a non-default repeat was specified. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.cspell.dict/cpython.txt(3 hunks)crates/vm/src/buffer.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/buffer.rs
🔇 Additional comments (2)
crates/vm/src/buffer.rs (2)
241-244: No changes required—Python whitespace handling is correctly implemented.The code at lines 241-244 correctly skips the standard ASCII whitespace characters (space, tab, newline, carriage return) that Python's struct module ignores in format strings. The comment and implementation are both accurate.
247-257: Logic is correct; add edge case tests for whitespace handling.The implementation correctly handles format parsing with whitespace based on the official Python struct documentation. The error logic for "repeat count given without format specifier" appropriately distinguishes between explicit repeat counts and default repeats when trailing whitespace is encountered.
Consider adding tests to extra_tests/snippets/stdlib_struct.py to explicitly verify edge cases:
- Format with trailing whitespace:
"i "- Explicit repeat with trailing whitespace:
"3i "- Whitespace between repeat and format:
"3 i"These scenarios are not currently covered in the test file.
| HIGHRES | ||
| IMMUTABLETYPE | ||
| Itertool | ||
| keeped |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for "keeped" in the codebase
rg -i "keeped" -C 2Repository: RustPython/RustPython
Length of output: 47
Remove "keeped" from the dictionary.
The word "keeped" does not appear anywhere in the codebase and is not valid English (the past tense of "keep" is "kept"). This entry should be removed from .cspell.dict/cpython.txt.
🤖 Prompt for AI Agents
.cspell.dict/cpython.txt around line 32: remove the invalid dictionary entry
"keeped" from the file — open .cspell.dict/cpython.txt, delete the line
containing "keeped" (ensuring no leftover blank lines break formatting), save
the file, and commit the change so the misspelled past-tense entry is no longer
in the cspell dictionary.
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.
@coderabbitai Please make a new PR to address this issue.
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.
@fanninpm, I cannot create pull requests directly to the repository. However, I can create a GitHub issue to track this task as a follow-up action.
Would you like me to open an issue for removing the invalid dictionary entry "keeped" from .cspell.dict/cpython.txt? The issue would include:
- The specific file and line that needs to be changed
- The reason for removal (not valid English, doesn't appear in codebase)
- A reference back to this PR and comment thread
Alternatively, if you prefer to handle this directly, the fix is straightforward:
- Open
.cspell.dict/cpython.txt - Remove line 32 containing "keeped"
- Save and commit the change
Please let me know how you'd like to proceed!
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.