[Xcb] EWMH API for XCB
jamey at minilop.net
Tue Dec 15 19:34:35 PST 2009
On Wed, Dec 16, 2009 at 01:14:47AM +0100, Arnaud Fontaine wrote:
> >>>>> Jamey Sharp <jamey at minilop.net> writes:
> >> I'm a bit confused by this macro and does not understand anything
> >> besides of the basic principle. I was thinking about simply
> >> adding an assertion otherwise.
> > Absolutely, that would be better than nothing.
> Well, I think it would be better to not make it static as it could be
> useful to any program implementing extensions to EWMH.
That makes sense.
> Therefore, I have just added an assert(). I think it's ok this way,
> isn't it?
Josh's explanation of the difficulties with asserts aside, it's fine
with me. :-)
> > I'm not talking about different connections. I'm talking about
> > different screens on the same connection. All the atoms are
> > guaranteed to be the same in that case, except for _NET_WM_CM_Sn.
> Oh ok, I didn't think at all about that as I have never used such
A nice way to test a multi-screen setup is with Xnest or Xephyr, which
let you configure several screens that all show up as windows on your
> Therefore, I removed root member of xcb_ewmh_connection_t
> in favor of screens and nb_screens. I added a parameter, namely the
> screen number, to all the functions using the root window. Also, I
> modified _NET_WM_CM_Sn which is now per-screen (xcb_atom_t *). I have
> committed it there.
Cool, I like this much better! I hope passing the screen number to all
those calls doesn't make using this code too painful?
Minor further suggestions, in xcb_ewmh_init_atoms:
You don't need to count the number of screens. You have a bunch of
options, but since you're getting an iterator over them, I'd just set
ewmh->nb_screens = screen_iter.rem before using the iterator in any
loops. Or if you want your code to be more self-explanatory, set
ewmh->nb_screens = xcb_setup_roots_length(setup).
Each entry in wm_cm_sn is used only once, so I'd declare it inside the
loop as a single buffer that you reuse each time, instead of using C99's
variable-length arrays (VLA). It's nice that C99 added VLA support, but
you don't need it here, and last I checked GDB did a bad job debugging
functions with VLAs in them.
> Hrm ok, oops... I have silently and discretly edited my previous commit
> to hide that :).
Nicely done. ;-) Looks right to me now, and the history clearly shows
that it was right all along. ;-)
> >> > Do you really want it in the xcb-util bundle though?
> I think it does make sense and I will put it in a separate repository
> then. However, I think it would be good to share at least configure.ac,
> coding style (I have not found anything like that except by looking at
> existing code but it's pretty unconsistent ATM AFAIK), and Doxygen
> configuration in order to have something consistent across libraries,
> even though I'm not sure how it could be achieve. Maybe a document on
> the wiki giving some templates such as Doxygen and configure.ac and
> another one for the coding style as it's more general?
I'm not as concerned as you are about consistency across different
projects, but I think there's a lot of value in documenting "best
practices" for setting up these projects. I'd completely approve of
additions to the wiki about these topics, but of course it's a wiki, so
you don't need my approval anyway. :-)
> Thank you very much for all your comments. The code looks much better
Thanks for your hard work. :-) This is cool!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 198 bytes
Desc: Digital signature
Url : http://lists.freedesktop.org/archives/xcb/attachments/20091215/962c6529/attachment.pgp
More information about the Xcb