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