gh-95504: Fix sign placement in PyUnicode_FromFormat #95505
Conversation
|
All commit authors signed the Contributor License Agreement. |
LGTM.
But this is bit sensitive to backport because Python 3.11 is right before RC.
|
I emailed core-mentorship about this on Friday night but didn't wait to hear back before submitting this PR. @encukou is in the process of splitting _testcapimodule.c up into more manageable pieces (#93649) and has asked me to move Unicode related tests into a separate file. I'll convert this PR to a draft while working on that and convert it back once that's done. |
|
@encukou this is ready for review. |
| if (negative && !zeropad) { | ||
| if (_PyUnicodeWriter_WriteChar(writer, '-') == -1) | ||
| return NULL; | ||
| } |
There was a problem hiding this comment.
It can be simpler. Instead of writing minus explicitly, you can write it as a part of the buffer.
You will get the same result if remove this if and pass &buffer[negative && zeropad] instead of &buffer[negative] to _PyUnicodeWriter_WriteASCIIString().
There was a problem hiding this comment.
That wouldn't work with precision: %.5d with -123 would be 00-123.
|
Sorry for making you do more work, but it works best if the test refactoring is done first, in a separate pull request, so that:
Do you want to split this up? I can also do it, and it wouldn't be too much extra work in addition to a review, but I don't want to “steal your contribution”. |
No problem, makes sense.
Sure, and for future reference I'll never have a problem with anything like that. Please ping me when the refactor is merged. |
gh-95504: Fix sign placement in PyUnicode_FromFormat
The sign doesn't stick to the number anymore:
PyUnicode_FromFormat("%05d", -123); results in "-0123" instead of "0-123",
PyUnicode_FromFormat("%.5d", -123); results in "-00123" instead of "0-123".