Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Dec 16, 2025

Summary by CodeRabbit

  • Chores

    • Updated spell-check dictionary with new entries.
  • Bug Fixes

    • Improved format string parsing to gracefully handle whitespace characters after repeat counts, providing clearer error messaging when format specifiers are missing or malformed.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 16, 2025

Walkthrough

This 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

Cohort / File(s) Summary
Spell-check dictionary
.cspell.dict/cpython.txt
Added three new entries: DICTFLAG, keeped, and stginfo to the allowed words list.
Struct format parsing
crates/vm/src/buffer.rs
Modified format string parser to skip whitespace (space, tab, newline, carriage return) after repeat counts. Adjusted logic to read the format character after whitespace, with conditional error handling: returns error if no format character follows a non-default repeat count (repeat != 1), otherwise stops parsing gracefully.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

  • buffer.rs logic: Requires verification that whitespace-skipping doesn't inadvertently consume format specifiers and that error conditions (missing specifier vs. default repeat) are correctly handled.
  • Dictionary entries: Confirm these are intentional keyword additions without typos.

Poem

🐰 A hopping through format strings with glee,
Whitespace begone! We skip merrily,
Three words join the spell-check spree,
Parsing cleaner, errors run free! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding whitespace handling in format string parsing, which aligns with the buffer.rs modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@youknowone youknowone marked this pull request as ready for review December 16, 2025 14:10
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 272b36d and 61a032d.

📒 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 running cargo fmt to 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
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for "keeped" in the codebase
rg -i "keeped" -C 2

Repository: 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.

Copy link
Contributor

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.

Copy link
Contributor

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:

  1. Open .cspell.dict/cpython.txt
  2. Remove line 32 containing "keeped"
  3. Save and commit the change

Please let me know how you'd like to proceed!

@youknowone youknowone merged commit aef4de4 into RustPython:main Dec 16, 2025
13 checks passed
@youknowone youknowone deleted the buffer branch December 16, 2025 14:27
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