Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pangocffi to not run verify #5

Merged
merged 3 commits into from Aug 1, 2014
Merged

Conversation

flacjacket
Copy link

In light of [1], try to get pangocffi to use ffi.dlopen() rather than ffi.verify(). There is still some problems with the tests, I probably need to edit the cdef to not have ellipses (???). It'd be nice if we could run verify, but it'd take an upstream change.

[1] Kozea/cairocffi#37 (comment)

@tych0
Copy link
Owner

tych0 commented Jul 2, 2014

Yeah, I think if we want pangocffi to not call verify(), we'll have to not use the '...'s. Actually I'm not sure that CFFI provides a great way to deal with this: if we don't use '...' then we have to keep our library up to date with every change in pango, but if we do, then users have to have a C compiler. It is much simpler to require people to have a C compiler, but then we run into problems on machines with old versions of cairo (or really, any version of cairo which is different than the one cairocffi's mkconstants.py was run on).

Anyway, because of the way cairocffi is done I think it is probably best to hard code the constants ourselves and just advise people to use certain versions of the libraries, vs. allowing them to use whatever as long as it compiles. So, in short, I think you're right in that getting rid of the '...'s is the way to go.

@flacjacket flacjacket changed the title [WIP] Fix pangocffi to not run verify Fix pangocffi to not run verify Jul 31, 2014
@flacjacket
Copy link
Author

Barring the problem I reported on the email list, I think this is getting close...

A couple things:

I changed

ret = C.pango_parse_markup(value, -1, 0, attr_list, text, ffi.NULL, error):
if ret:

to:

ret = pango.pango_parse_markup(value, -1, accel_marker, attr_list, text, ffi.NULL, error)
if ret == 0:

Since the docs on pango_parse_markup [1] say it returns FALSE = 0 [2] on error.

I also change the return:

return attr_list, text

to:

return attr_list[0], ffi.string(text[0]), unichr(accel_marker)

to be more like the PyGTK implementation [3], especially since the only place we use this it is parsed as a 3-tuple [4]. I'm not sure why this hasn't thrown errors for us... Do any of the tests cover this?

[1] https://developer.gnome.org/pango/stable/pango-Text-Attributes.html#pango-parse-markup
[2] https://developer.gnome.org/glib/unstable/glib-Standard-Macros.html#FALSE:CAPS
[3] http://www.pygtk.org/pygtk2reference/class-pangoattrlist.html#function-pango--parse-markup
[4] https://github.com/tych0/qtile/blob/cffi/libqtile/drawer.py#L35

@tych0 tych0 merged commit a46a846 into tych0:cffi Aug 1, 2014
@tych0
Copy link
Owner

tych0 commented Aug 1, 2014

Awesome, thanks for doing this work! We can puzzle through the last bug on the ML, I'll just merge this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants