Signal editor (was Re: [Glade-devel] small patch)

[Joaquin Cuenca Abela]

Hi!

I've been hacking a bit on the signal editor.

What do you think of an interface like that one:

http://e98cuenc.free.fr/signals.png

You add a new signal handler just typing the name of the handler, and you
remove it just erasing the name of the handler.

I want to also add the name of the class that each signal belongs (as with
the current "add a signal handler dialog").

I've just done the interface of the screenshot, but I've not yet made it to
work (that will be for tomorrow).

It looks to me much simpler to use than the current one (for those that
don't know, the current one is a clone of the glade2 one).

Cheers,
 
Paolo wrote:
> 
> On Tue, 2004-01-13 at 14:10, Sridhar R wrote:
> > Hi,
> > 
> 
> Hi Sridhar,
> 	I've not tried the patch (yet), but if I understand it 
> correctly the result is that, e.g. for a GtkButton, the 
> GtkButton signals list node in the tree is expanded.
> Did I get it right? If yes, I like it :) [if someone else 
> doesn't feel free to voice your opinion]
> 
> Some comments (nitpicks mostly) on the patch follow:
> 
> 1) it seems to me that your mailer ate the patch a bit (it 
> breaks some lines). If you use Evolution you can send the 
> patch as an attachment and it will be sent in plain text, 
> don't know if other mail apps do the same...
> 
> > I'm all new to collabarative developments.  Here is my
> > (initial) small patch to CVS glade-signal-editor.c, regarding 
> > usability.  That is, it will now expand the first row of 
> signal-list 
> > dialog, which is what any user would do more often.
> > 
> > Here goes the diff (diff --context=2 . .)
> 
> please use diff -pu (the p option tells in which function the 
> changes are made, making the patch more readable. The default 
> context (3 iirc is just fine).
> Beside also provide a ChangeLog entry.
> 
> > *** ../../glade3.orig/src/glade-signal-editor.c Mon Jan 12 21:38:09 
> > 2004
> > --- glade-signal-editor.c       Mon Jan 12 22:04:06
> > 2004
> > ***************
> > *** 158,161 ****
> > --- 158,162 ----
> >         GtkTreeIter *parent = NULL;
> >         GList *list = NULL;
> > +       GtkTreePath *path_first = NULL;
> >         GladeWidgetClassSignal *signal;
> >     
> 
> No need to initialize local vars to NULL as far as I can see, 
> while you are at it remove it also from the other vars; 
> beside, not related to your code, but usually a TreeIter is 
> allocated on the stack: i.e.
> GtkTreeIter iter;
> gtk_tree_functio (..., &iter, ...);
> 
> > ***************
> > *** 174,177 ****
> > --- 175,182 ----
> >                
> > glade_signal_editor_dialog_append_signal (lst_model,
> > signal->nam
> > e, parent);
> >         }
> > +       /* Sridhar R: Expand the first row */
> > +       path_first = gtk_tree_path_new_first();
> > +       gtk_tree_view_expand_row(GTK_TREE_VIEW (view),
> > path_first, FALSE);
> > +       gtk_tree_path_free(path_first);
> >   }
> >   
> 
> We usually don't add comments like "paolo: did this" for each 
> patch because they would fastly clutter the code. In this 
> particular case the whole comment seems superflous, since 
> it's just stating what the 3 line under it do.
> 
> > ***************
> > *** 183,186 ****
> > --- 188,193 ----
> >         gint response;
> >   
> > +       g_assert(editor);
> > +       g_assert(editor->class);
> >         g_return_if_fail (editor->class->signals != NULL);
> >   
> 
> Use g_return_if_fail instead of assert to check function args.
> 
> > ***************
> > *** 663,665 ****
> >         }
> >   }
> > -
> > --- 670,671 ----
> > I have also attached the diff file.
> > 
> > Well, what are all the stuffs that I can start working on?
> > 
> 
> If you want to continue with ui stuff on the signal editor 
> there is a detailed bug in bugzilla.gnome.org, suggesting 
> some changes that can be made. I don't have the bug number at 
> hand, but since at the moment the open bugs against glade3 
> are just 2, you should find it pretty easily :)
> 
> Thanks for your work, I'm sure that once you get started 
> things will go more smoothly!
> 
> ciao
> 	paolo