Apply the format spec to a bool instead of dropping it - #8566
Conversation
|
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: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughBoolean 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 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: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
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
c344c34 to
8af4bd3
Compare
Summary
A
boolignores 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_boolhas aNonearm for "no presentation type" that returns the spelled out name whatever else the spec holds. CPython does not giveboola__format__at all:It inherits
int.__format__, and the rule there is the one already written inPyInt::__format__in this tree: an empty spec on a subclass givesstr(self), anything else formats the integer. So the empty spec keeps the old answer and every other spec goes toformat_int, which is also what restoresPrecision not allowed in integer format specifierand thezrejection.is_emptyis written as a destructure ofSelfrather than a chain ofself.field, so that adding a field toFormatSpeclater fails to compile here instead of silently making an empty spec look non-empty.The literal
"True"/"False"replaces a round trip throughto_string()plusto_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.
crates/common/src/format.rsgainsformat_bool_without_a_presentation_type, next to the existingformat_bool_basic. Without the change it fails withleft: Ok("True")againstright: Ok(" 1").extra_tests/snippets/builtin_format.pygains the same ground at the Python level, including the four specs that have to raise. Without the change it stops atassert format(True, "5") == " 1".pytest test_snippets.py -k builtin_format, both legs green, so the file also holds under CPython 3.14.7.-m test test_format,-m test test_booland-m test test_typeson the release build: 18, 31 and 129 tests, all SUCCESS.cargo clippywith the flags CI uses, clean, andcargo fmt --checkclean.ruff format --checkandruff check --select Iclean on the snippet.cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi: no failures.The three clippy jobs and the WASM check are red for the reason in #8564, unrelated to this change.
Summary by CodeRabbit
New Features
True/Falseoutput for empty formatting specifications.Bug Fixes
ValueErrormessage.