Re: [Planner Dev] New patch for undo: Focus in - focus out undo

[Richard Hult <richard imendio com>]

Hi,

> Hmmm, so the changes won't be done in the variable until de focus get
> put of the field? I haven't followed this way because it was to change
> the way planner works in several places: don't change the variable until
> de focus gets out of the field. Yes, with this change, it will be easy
> for the undo system to work. I can make this changes in Planner, but to
> be consistent we need to change it in several places. What do you think?

We can leave that for 0.13 or later perhaps.

> > Also the g_object_set/get_data should be done by adding the
> > necessary fields to DialogData instead, IMO.
> > 
> 
> Ok, I have done it this way because it is a very short live variable and
> associated with a very specific field so ... but it is clean to put it
> in DialogData. My fear is DialogData getting complex.

Overusing object data leads to code that is hard to follow and maintain,
in my experience. We can leave this as well for 0.13+ though.

> > Another thing that makes the undo stuff a bit unnecessarily complex is
> > the use of gvalues everywhere... Do we really need that in the dialogs
> > that don't use gvalues otherwise? I used it in the task-tree edited_cb
> > to begin with just because there is one callback that handles all cells,
> > and gvalues fits in there nicely. 
> 
> I have used it because if we use specific methods for example, for all
> the different properties with different type, we will have lots of code
> that do very similar things but using different value type.

But in this latest patch, there were one function per property anyway,
so it only means more code with gvalue than without (since it's a lot
cleaner to just do str = g_strdup (foo) than to setup and handle a
gvalue with a string in it).

> > In those places where there's actually one callback/cmd per command,
> > then we should definitely use gchar *, gint, etc instead of gvalues. It
> > would make the code a lot clearer and simpler.
> > 
> 
> Sure, I have to check it, but I have the feeling that we will duplicate
> a lot of very similar code.

I disagree :) But I might be wrong, we can think a bit about it.

/Richard

-- 
Imendio HB, http://www.imendio.com/