gh-95504: Fix sign placement in PyUnicode_FromFormat

[philg314]

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".

• Issue: Wrong sign placement in PyUnicode_FromFormat #95504

[ghost]

All commit authors signed the Contributor License Agreement.

[methane]

LGTM.
But this is bit sensitive to backport because Python 3.11 is right before RC.

[philg314]

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.

[philg314]

@encukou this is ready for review.

[serhiy-storchaka]

LGTM in general, but I would wait for testcapi refactoring.

[serhiy-storchaka]

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().

[philg314]

That wouldn't work with precision: %.5d with -123 would be 00-123.

[encukou]

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:

• the change PR shows what changed in the tests
• if something goes wrong, the change can be reverted without the refactoring
• the refactoring can be applied to older unfixed versions, if necessary (e.g. wep only need to backport a subset of changes)

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”.

[philg314]

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:

• the change PR shows what changed in the tests
• something goes wrong, the change can be reverted without the refactoring
• refactoring can be applied to older unfixed versions, if necessary (e.g. wep only need to backport a subset of changes)

No problem, makes sense.

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”.

Sure, and for future reference I'll never have a problem with anything like that. Please ping me when the refactor is merged.

[encukou]

@philg314,
Tests were merged in #95819.
The behavior change is in #95848, could you check everything is in order?

[encukou]

The changes were merged in #95819 and #95848.

Thanks for the contribution, @philg314!

[philg314]

Thanks for the reviews @methane and @serhiy-storchaka and thanks for merging @encukou!