[Glade-devel] small patch

[sridharinfinity yahoo com (Sridhar R)]

--- Paolo Borelli <pborelli katamail com> 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 :)

  Yes.

> 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...

   Anyhow, I also (and will) attached the files
inlined.  Since I am using Yahoo web client, i doubt
whether this is possible.

> 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.

   Thanks for the info.

> > *** ../../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, ...);

   I don't understand.

> > ***************
> > *** 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.

   Fine.

> > ***************
> > *** 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.

  No.  If you have seen my bug report on glade3, the
program actually segfaults.  But with these assert
statement, it shows the correct message as 'assert
failure', saving minutes of fiddling up with gdb.
  Any thoughts?

> > ***************
> > *** 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


=====
Sridhar R 
Email: r_sridhar users sf net
WWW:   http://sridhar.has.it

__________________________________
Do you Yahoo!?
Yahoo! Hotjobs: Enter the "Signing Bonus" Sweepstakes
http://hotjobs.sweepstakes.yahoo.com/signingbonus