| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: dcc84219-ea1d-4c5e-ba8f-aa817942d469 📥 CommitsReviewing files that changed from the base of the PR and between c344c34 and 8af4bd3. 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 Walkthrough WalkthroughBoolean formatting now spells values only for empty specifications. Non-empty specifications use integer formatting rules, including width, alignment, signs, grouping, presentation types, and related errors. Rust and Python tests cover these behaviors. ChangesBoolean formatting
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to 8af4b This change makes boolean formatting follow integer formatting rules for width, alignment, padding, signs, separators, and invalid precision specs, with targeted coverage reported as passing. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: andrej730 🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
format_bool answered "True" or "False" for every spec that carries no
presentation type, so width, fill, alignment and sign were all discarded:
>>> f"{True:>5}"
'True'
CPython has no bool.__format__ of its own. It uses int's, where an empty
spec on a subclass gives str(self) and everything else formats the integer.
The empty spec keeps the spelled out answer, the rest now goes to
format_int, which also brings back the errors an integer spec raises.
Assisted-by: Claude Code:claude-opus-5
| Back | FazBrowse Home | New Git URL |
Summary
A bool ignores every format spec that does not name a presentation type:
Width, fill, alignment, sign and the thousands separator are all dropped, and a spec that an integer would reject is accepted quietly. Lining a boolean column up in a table is the case that runs into this, since that spec has no type letter in it.
format(True, "d") and the rest of the presentation types were already right, which is why this only shows up on the specs that leave the letter out.
FormatSpec::format_bool has a None arm for "no presentation type" that returns the spelled out name whatever else the spec holds. CPython does not give bool a __format__ at all:
It inherits int.__format__, and the rule there is the one already written in PyInt::__format__ in this tree: an empty spec on a subclass gives str(self), anything else formats the integer. So the empty spec keeps the old answer and every other spec goes to format_int, which is also what restores Precision not allowed in integer format specifier and the z rejection.
is_empty is written as a destructure of Self rather than a chain of self.field, so that adding a field to FormatSpec later fails to compile here instead of silently making an empty spec look non-empty.
The literal "True" / "False" replaces a round trip through to_string() plus to_uppercase() on the first byte, in the arm that was being rewritten anyway.
Test Plan
Built in a Debian container on rustc 1.98.0.
The three clippy jobs and the WASM check are red for the reason in #8564, unrelated to this change.
Summary by CodeRabbit
New Features
Bug Fixes