Remove assert from Python script processor

[iorsh]

Python methods normally exit gracefully in case of error. Killing FontForge due to incorrect scripting is generally undesirable.

Type of change

• Bug fix

[skef]

I would say that the description on this PR reflects a confused idea of the place of assert() calls. Assertions are only complied when debugging flags are turned on. No one is expected to be running python scripts with debug-compiled code under normal circumstances, and if they do the goal should be to capture a problem at the point in the code where it is detected, which an assert() does just fine.

So the question here is how you can reach this part of the code while failing PyTuple_Check(), and whether that circumstance is "valid". If it is valid, then the change is appropriate because we need to handle it in RELEASE contexts. If it is not valid it should be fixed upstream.

[iorsh]

Oh, my bad! In Release mode there is no crash, naturally. After the asset the workflow continues to address a non-tuple object as tuple and eventually leads to a misleading error message:

Traceback (most recent call last):
  File "/home/iorsh/.config/fontforge/python/main.py", line 15, in <module>
    fontforge.registerMenuItem(CreatePrecomposedGlyphs.CreatePrecomposedGlyphs,None,None,"Font",None,[1, 2],"Create Precomposed Glyphs");
ValueError: A `name` or `submenu` tuple must have either 2 or 3 elements

So maybe the patch is still worth something...

[skef]

Note: I haven't the code leading up to this code. Unlike with other contributors I'm shifting to asking you the questions I would normally ask and research myself, both to lower my "workload" and to reflect the role you're (voluntarily) taking on.

Anyway ...

It sounds like you're saying that there are valid cases in which "args" won't be a tuple, where "valid" doesn't mean "correct", but "undesirable to fix up-stream". That is, this is the appropriate place to handle a case in which "args" is not a tuple (or string, given the earlier check).

If that's right, then yes, this PR is appropriate, not because asserts are bad in this context in theory, but because this assertion is wrong: "args" can be something other than a string or tuple at this point of execution.

If you confirm this analysis I'll approve and merge.

[iorsh]

Well, the answer is probably no. I'm talking about a Python command with intentionally incorrect argument (a list where only tuple or string is permitted by spec) aimed at demonstrating the issue. The correct course of action for the user would be fixing the input.

Code in question:
fontforge.registerMenuItem(CreatePrecomposedGlyphs.CreatePrecomposedGlyphs,None,None,"Font",None,[1, 2],"Create Precomposed Glyphs");
The 6th argument [1,2] should not be list according to spec. Hope this clarifies the situation