Don't require individual tuple encapsulation in fontforge.font.bitmapSizes setter#5138
Merged
skef merged 1 commit intofontforge:masterfrom Jan 10, 2024
nabijaczleweli:master
Merged
Don't require individual tuple encapsulation in fontforge.font.bitmapSizes setter#5138skef merged 1 commit intofontforge:masterfrom nabijaczleweli:master
skef merged 1 commit intofontforge:masterfrom
nabijaczleweli:master
Conversation
iorsh
reviewed
Jan 5, 2024
| sizes = malloc((cnt+1)*sizeof(int)); | ||
| for ( i=0; i<cnt; ++i ) { | ||
| if ( !PyArg_ParseTuple(PyTuple_GetItem(value,i),"i", &sizes[i])) { | ||
| sizes[i] = PyLong_AsLong(PyTuple_GetItem(value,i)); |
Contributor
There was a problem hiding this comment.
This fixes the issue, but will break the existing scripts for those who managed to figure out this peculiarity.
On one hand, preserving existing state has merits, but on the other hand we do break things from time to time, like when migrating to Python3 or updating AGL for new fonts.
@skef, what do you think?
Contributor
There was a problem hiding this comment.
Yeah, I'm fine with standardizing on flat ints but the code should still accept the singleton tuples in case scripts have relied on that behavior. It's not that hard to code that way.
…Sizes setter
Documentation specs that this should take the same format as
font.bitmapSizes returns: it doesn't;
>>> f.bitmapSizes = ((1,), (2,))
yields
>>> f.bitmapSizes
(1, 2)
Which is fun, but what actually happens is the user gets
>>> f.bitmapSizes = (1, 2)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
SystemError: new style getargs format but argument is not a tuple
and gives up.
Closes: #5137
Contributor
Author
|
Updated to allow both |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Documentation specs that this should take the same format as font.bitmapSizes returns: it doesn't;
yields
Which is fun, but what actually happens is the user gets
and gives up, as in #5137. This is also, obviously, not reversible, unlike specced.
The new behaviour is congruent with the documentation (error cases included at the end):
idk what the ERANGEs are but they're unrelated
Type of change