Re: Libxfce4ui api review

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
_______________________________________________
Xfce4-dev mailing list
Xfce4-dev@...
http://foo-projects.org/mailman/listinfo/xfce4-dev