[Glade-devel] Signal editor rework

[tristan.van.berkom at gmail.com (Tristan Van Berkom)]

On Wed, Aug 11, 2010 at 6:18 PM, Johannes Schmid <jhs at jsschmid.de> wrote:
> Hi!
>
> Code: http://git.gnome.org/browse/glade3/log/?h=signal-tree-model
>
> Interesting parts:
> *
> http://git.gnome.org/browse/glade3/tree/gladeui/glade-signal-editor.c?h=signal-tree-model
> *
> http://git.gnome.org/browse/glade3/tree/gladeui/glade-signal-model.c?h=signal-tree-model

Hi Johannes !
   I am sorry I did not give your branch more attention thus far, I'm
giving it a run down
right now and try to get the ball rolling again...

So here are some points I can spot so far...

  - Firstly I can see you went and implemented the GtkTreeIter->stamp a bit more
    robustly than in GladeProject, additionally the code needs to assert for
    any input iter: (iter->stamp == model->priv->stamp). I've already
done this for
    GladeProject and will commit that soon (it helped alot to debug...
unfortunately
    I could still not completely solve bug 627078 and have not
committed that yet,
    I'll commit the checking code now regardless).

  - Can you explain why the model->priv->stamp must be incremented
when the overall
    model data changes ? (it could be that GladeProject needs this
treatment too... or
    that the model's stamp should not change... I'm not sure).

  - The iter->stamp should be initialized to something random,
otherwise every model
    starts with stamp == 0 (possibly making the iter from the last
destroyed signal model
    appear "valid")

  - I would prefer we stick with firing Glade's generic object
selection dialog to select
    the project object for a signal user data - I dont like the idea
of navigating recursive menus
    or dropping down a combo box with a possible > 1000 objects to select.

I have to compile and run the code to say more, but my initial
feelings are that we
really need to stamp those iters and check the iters on every model input.

Cheers,
       -Tristan

> Why?
>
> My starting idea was to be able to drag signal handlers from glade into
> anjuta and automatically creating the prototypes. Some may have seen my
> GUADEC lightning talk on that issue.
>
> So I looked at the SignalEditor and that was a big monolitic block with
> many custom cell rendering methods with a backend based on a GtkTreeStore.
> The GtkTreeStore meant that updating things was rather painful and that
> the data structure of the GladeWidget was duplicated and had to be kept up
> to date with the widget.
>
> So my idea was to use a custom GtkTreeModel instead that would use the
> signal structure of the GladeWidget directly. I though about implementing
> directly in glade-widget.c but realized pretty soon that this would bloat
> that file two much and that it wouldn't be obvious that a GladeWidget
> implements a GtkTreeModel.
>
> As such, I started on GladeSignalModel which is basically initialised
> using a GladeWidget and displays it's data. In the model/view-concept this
> holds the signal data which is taken from the GladeWidget directly without
> duplicating it.
>
> This model implements the drag and drop stuff based on GtkTreeDragSource.
> The most complicated thing in the tree model is that we need to take care
> of dummy nodes that a used to enable the user to add new signals.
>
> The implementation of the GladeSignalEditor on top of that was trivial then.
>
> Status
>
> Everything is feature complete compared to the old GladeSignalEditor,
> minus the deprecation warnings and the devhelp button. These are both not
> difficult to add I just didn't come to it, yet.
>
> What needs to be improved?
>
> I don't like the way the GtkCellRendererCombo works for the object
> selection with a real tree (not a list). I think that isn't convenient yet
> and needs a better solution.
>
> Some horizontal space could be saved by using the "Name" column for both,
> object names and signal-names instead of a additional "Handler" column.
> This is quite easy to do.
>
> Conclusion:
> While I didn't save as much code as I wanted to (mainly because of the
> GObject template code necessary) I think the implementation is much
> cleaner than the code before.
>
> Regards,
> Johannes
>
> _______________________________________________
> Glade-devel maillist ?- ?Glade-devel at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/glade-devel