[sugar] [PATCH] Fix Keep Icon in Journal (Was: Recursive Signal Loop.)
Eben Eliason
eben.eliason at gmail.com
Fri Apr 25 12:39:09 EDT 2008
Well, here's what I've got at present. It seems to be a worthwhile
cleanup patch even though it doesn't fix the problem I set out to fix.
It's true that the new Journal designs will eliminate the problem
once we get around to implementing them. I suppose we'll have to live
with the slow feedback on the favorite buttons in the interim.
- Eben
On Fri, Apr 25, 2008 at 10:55 AM, Tomeu Vizoso <tomeu at tomeuvizoso.net> wrote:
>
> On Thu, Apr 24, 2008 at 11:37 PM, Eben Eliason <eben.eliason at gmail.com> wrote:
> > On Thu, Apr 24, 2008 at 5:25 PM, Bert Freudenberg <bert at freudenbergs.de> wrote:
> > >
> > >
> > > On 24.04.2008, at 22:56, Eben Eliason wrote:
> > >
> > >
> > > > On Thu, Apr 24, 2008 at 4:47 PM, Jameson Chema Quinn
> > > > <jquinn at cs.oberlin.edu> wrote:
> > > >
> > > > > you check the keep property before you set it, and do not touch it if
> > > you
> > > > > are not going to change it.
> > > > >
> > > >
> > > > That does in fact sound like a reasonable way to handle it. It
> > > > doesn't require an extra time around "the loop"....it simply stops it
> > > > directly at the signal handler for the KeepIcon change event by not
> > > > overwriting with the same value. Don't I feel stupid now....
> > > >
> > >
> > >
> > > I'd even consider it a bug if overwriting a property with the same value
> > > would emit a change event - but maybe this is how the framework works?
> >
> > Good point. That seems not to be the case.
> >
> > In other news, despite the much cleaner underlying code, my initial
> > treatment fails to alleviate the symptom! It appears that the redraw
> > doesn't happen until after all of the signal handling business has run
> > its course. This includes, as mentioned in my first message,
> > refreshing the entire view. I looked at this again, and there is no
> > check for the particular CollapsedEntry which changed at all.
> > Instead, it loops over each one, refreshing each by setting the
> > jobject property of each, which of course resets the keep icon, resets
> > the object icon, creates a brand new palette from scratch and attaches
> > it to the object icon, looks up and formats the date, reads the list
> > of buddies and creates their associated objects and palettes, and a
> > number of other smaller tasks. That's for /each/ entry shown, all
> > because one toggle button was clicked! This seems like serious
> > overkill, but I'm not sure if there's an obvious way to isolate the
> > changes. It does seem that, if anything, either the query.py or
> > listview.py code should be more intelligent about determining what
> > changes are worth doing such comprehensive updates for. This is not
> > one of them.
> >
> > Tomeu, you worked on the query and list code. Do you have any insight
> > into this?
>
> The lazy programmer that coded this (me) thought that the simple
> solution implemented was efficient enough and could be written in a
> simple way with many mainteinance advantages.
>
> Eben, your new designs will render this code totally obsolete, so
> what's the point in changing this right now?
>
> About the signal loop, it's my opinion that most property setting code
> should only do its thing if the new value is different to the current
> one. This can affect performance considerably, as well.
>
> Thanks,
>
> Tomeu
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix_keep_icon_in_journal.diff
Type: text/x-patch
Size: 4831 bytes
Desc: not available
Url : http://lists.laptop.org/pipermail/sugar/attachments/20080425/d00f65b7/attachment.bin
More information about the Sugar
mailing list