Add validation for Pango Markup - Fixes #8 #9
Conversation
|
As soon as this is merged, I file another PR for the change of |
manimpango/cmanimpango.pyx
Outdated
| def validate(text: str) -> bool: | ||
| cdef GError** res = NULL | ||
| text_bytes = text.encode("utf-8") | ||
| return pango_parse_markup(text_bytes, len(text_bytes), 0, NULL, NULL, NULL, res) |
There was a problem hiding this comment.
I think res is NULLABLE, so if we aren't using it then it can be NULL. Also, len(text_bytes) isn't required. Instead it can be -1 and Pango calculates the length.
| return pango_parse_markup(text_bytes, len(text_bytes), 0, NULL, NULL, NULL, res) | |
| return pango_parse_markup(text_bytes, -1, 0, NULL, NULL, NULL, NULL) |
There was a problem hiding this comment.
You are right, it actually works even if res is NULL. The docs said the function would return false if error was set, so I'd better give a real pointer. For the length, I suggest keeping this. It does not cost us anything and automatic determination of length will fail if \0 is in the string (no idea why anyone would want that in their string, but still). My experience tells me it is always better to indicate the length, if this can be done "for free".
There was a problem hiding this comment.
Looks like this is causing some compiler warnings on Windows.
See https://github.com/ManimCommunity/ManimPango/runs/1696088350?check_suite_focus=true#step:5:96
doing like this fixes it
return pango_parse_markup(text_bytes, <int>len(text_bytes), 0, NULL, NULL, NULL, res)I have done this in #12
|
Also, it would be good to have a small test for that :) |
I will produce one in the next few days. |
|
How can black fail and tell me the line no. 5 is too long? It's 44 chars now.... |
|
The lint check is driving me mad. First it complained, because the line was longer than 79 chars. And now it seemingly puts all the stuff on one line again?
What length am I supposed to use? @naveen521kk |
Fixes #8