Libxfce4ui api review

Brian J. Tarricone brian at tarricone.org
Mon Jul 6 21:58:11 CEST 2009


Random stuff in no particular order:

1.  xfce_message_dialog_vnew() -- please use the 'standard' naming 
convention as used in glib/gtk: xfce_message_dialog_new_valist(). 
"_vnew" isn't used at all; if you were to use "_newv", that would be for 
a function that takes an array length and an array of parameters.

2.  xfce_message_dialog_run() -- I'm a little skeptical of this, since 
it doesn't really line up with (e.g.) gtk_dialog_run().  If you're just 
changing it for the sake of changing it, please don't: why waste time 
doing s/xfce_message_dialog/xfce_message_dialog_run/g for a cosmetic 
difference when it doesn't really make the function use clearer?

3.  xfce_message_dialog_show_{info,warning,error}() -- I'm happy to be 
rid of xfce_{info,warn,err}(), but we should make these HIG-compliant by 
taking separate primary and secondary text.  Aside from changing the 
ugly naming, this doesn't really have much of a benefit over the old 
ones.  Ditto for _confirm().

4.  xfce-dialogs.h in general -- why are the 'parent' arguments 
gpointers?  They should be GtkWindows.  Yes, I know, more typing for the 
cast, but... well, if we didn't care about type-safety, then we might as 
well just have everything take a gpointer argument and be done with it. 
  (See #8 below; I realised why you did this, but it's still bad.)

5.  xfce-execute.h -- if we're going to rename them, we might as well 
use the glib/gdk convention of using "spawn" in the name.  Nice idea on 
providing a startup timestamp.  Any reason for dropping the 
'in_terminal' stuff?  That's still useful for anything that executes a 
.desktop file.

6.  xfce_gdk_pixbuf_new_from_inline_at_size() -- I forget why we need 
this... is using gdk directly much more difficult?

7.  We could also use a xfce_gtk_window_center_on_screen() that takes a 
GdkScreen* and monitor number.  I know I use the analogous libxfcegui4 
function in places.

8.  I don't really understand what xfce_gtk_dialog_parse_parent() does 
based on the API description.  Can you explain it better?  Also I very 
much dislike how 'parent' can be a GtkWidget or GdkScreen -- ohhhh, I 
see why the xfce_message_dialog*() functions take a gpointer now.  Every 
time I think about doing something like that for *private* internal API, 
I cringe and try to find a better way.  Absolutely we shouldn't do 
something like this for *public* API.

Yeah, I'm worried about the session client stuff too.  GNOME seems to be 
attempting to move away from XSMP; I'm not yet convinced that doing the 
same makes sense for us.  We won't be able to kill libxfcegui4 in Xfce 
core entirely without a working session client, though :-( .

	-brian



More information about the Xfce4-dev mailing list