[sugar] Recursive Signal Loop.

[Eben Eliason]

Hello, my name is Eben Eliason, and I have a problem...

Symptom: The keep buttons in the Journal (the stars) take 1 second or
so between being clicked and visually reflecting that change.  This
is, admittedly, a rather minor problem, but as you'll see, it spells
out a larger underlying ugliness.  I expect the feedback to be instant
both because it's better that way, and also because the identical
(well, nearly so, as shown below) favorite icons used in the list view
of the new Home already function perfectly, and quickly.

Diagnosis: Upon inspection, the KeepIcon class used in the Journal
differs from the FavoriteIcon class used in Home in one crucial way.
The former does not connect to a button-release-event at all, and
instead depends on the code which uses it to do so for it, take the
necessary actions based on the toggle, and then tell the KeepIcon
itself that it has been clicked on in order to update its internal
state and reflect that with visual changes.  This is obviously the
incorrect approach to the problem, as the KeepIcon should abide by all
the wonderful properties that makes object oriented programming
useful, manage its internal state so that, if nothing else, it remains
consistent when clicked (it's basically a checkbox....a boolean
toggle), and notify anything that may happen to care about its value
when it changes.

Treatment: Take the object oriented approach above, which works nicely
in Home, and should also work nicely in the Journal.  This cleans up
the code, and allows the KeepIcon to update its internal state
immediately before emitting the signal, thus redrawing before anything
which wishes to be notified of its state has a chance to waste lots of
time, slowing feedback.

Side-effects: This is where things get ugly.  The above changes were
trivial, but they create an ugly signal loop which makes the treatment
worthless in its purest state as described above.  Here's what goes
down:

When a KeepIcon is clicked, its button-release-event handler updates
its internal state, including its keep property, which thus emits a
notify::keep signal.  The CollapsedEntry object which contains the
KeepIcon connects to this signal in order to update the value of the
keep key in the metadata of the associated JObject.  So far so good.
The DS then emits an updated signal, which includes a reference to the
JObject which has been updated.  Off in Never-Never-Land, query.py is
listening for the update signal from the DS, and the handler for this
signal replaces the JObject in its internal dict, and then emits its
own modified signal.  The listview.py connects to the modified signal,
and its handler then calls self._do_scroll, which in turn calls
self._refresh_view.  The latter of these functions takes it upon
itself to loop over all of the CollapsedEntry objects visible on
screen, updating a match for the changed JObject by setting its
corresponding jobject property.  The method responsible for handling
the setting of this jobject property within the CollapsedEntry then
reads all of the info out of the newly passed JObject, and updates the
entry as required.  This includes setting the keep property of the
KeepIcon within the entry, which cannot assume that it has previously
been set, since this is also the function that gets called to
initialize the CollapsedEntry in the first place.  This, of course,
triggers the do_set_property of the KeepIcon, which in turn emits a
notify::keep signal. Hooray.

Treatment of side-effects: Anyone know the best way to handle this
issue?  I'm fairly convinced that the fundamental changes to the
KeepIcon class suggested above are the correct approach logically and
semantically.  I'm unsure about the correct way to sever the recursive
loop, however.  Any thoughts are very much appreciated, since I've now
spent considerable time messing with an issue that I thought would
have a 10 minute fix.

- Eben

PS.  Thanks for listening!