[g-a-devel]Re: at-poke patch for interactive refresh and Java compat

[Michael Meeks <michael ximian com>]

Hi Bill,

On Tue, 2002-05-14 at 14:34, Bill Haneman wrote:
> * adds listeners to common at-spi events, so that events that change the
> state of the currently selected accessible object cause the at-poke
> object display to be updated;

	Sounds great.

> * makes sure that the callbacks on the object display don't collide with
> the update handler (above); at the moment the approach is to block the
> handlers for the at-poke UI unless the poke window is active, and
> conversely to block the event notification callbacks when the at-poke UI
> has focus.

	Interesting.

> * corrects the spelling of the work "caret" ;)

	:-)

> * somewhat distressingly, provides conditional compilation guard
> "JAVA_ENABLED" to removes some sanity checks which objects from the
> java-access-bridge fail.  This is intended as a temporary measure until
> we can figure out why certain sanity checks fail, and allows at-poke to
> successfully query accessible Java applications (if compiled with
> #define JAVA_ENABLED).

	Yes - that sucks, but whatever - it's great to have the bridge in a
testable state :-)

> I am a little unsure of how to include the new files in the diff, so I
> am attaching them separately.

	Fine.

> comments please ? (such as, "ok to commit" ;-)

	I have a few, entirely stylistic comments, to your nice patch.

> +	accessible_listener_set_target (
> +	accessible_listener_get(), accessible);

	Could use some more indentation and '_get ()'.

> +	if (event->in)
> +	{
...
> +	}
> +	else
> +	{
...
> +	}

	Can you use:

	if (a) {
	} else {
	}

	To save hspace.

>  	/* FIXME: need a toggle for this */
> -	poker->ctype = SPI_COORD_TYPE_WINDOW;
> +	poker->ctype = SPI_COORD_TYPE_SCREEN;

	It would be nice to have a toggle for this, as it says - but screen is
almost certainly more useful :-)

> 	AccessibleListener *al = user_data;
> 	if (event->source == al->target) {
> 		g_signal_emit (
> 			al, signals [UPDATE], 0);
> 	}

	Can you remove the redundant braces and the emit ( line break.

> 	for (i = 0; event_names [i] != NULL; ++i )
> 	{

	ditto for redundant braces, and redundant space after '++i', also
event_names [i] is quite sufficient without the != NULL.

	Apart from that it looks really great. Thanks for fixing that up Bill -
it's starting to look nice.

	I just need to totally re-write the tree model to try and synchronize
the tree model on both sides of the link in a reliable way and make that
jive with the tree model at some stage, and then we'll have a rock-solid
beast :-)

	So please do commit, with those fixes,

	Regards,

		Michael.

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot