Add create-engine signal in IBusFactory for non-C applications.

Add create-engine signal in IBusFactory for non-C applications.


TEST=Linux desktop

Separated from CL #4853041.

[Peng, 2011-08-06 18:24:41]

http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c
File src/ibusfactory.c (right):

http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c#newcode121
src/ibusfactory.c:121: factory_signals[CREATE_ENGINE] =
Please add inline gtk document here.

http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c#newcode207
src/ibusfactory.c:207: gchar *engine_name = NULL;
How about call emit create-engine signal directly, and move original create
engine logic to create-engine default signal callback?

[Peng, 2011-08-08 20:17:49]

http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c
File src/ibusfactory.c (right):

http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c#newcode207
src/ibusfactory.c:207: gchar *engine_name = NULL;
On 2011/08/06 18:24:41, Peng wrote:
> How about call emit create-engine signal directly, and move original create
> engine logic to create-engine default signal callback?

I mean this function may simply emit create-engine signal, if there is signal
handler returns non-NULL, you should stop call other handlers. Otherwise gobject
will call default signal handler factory->create_engine which contains origin
create engine logic (looking up the hashtable, and create an engine instance).
Does it make sense?

[fujiwara, 2011-08-09 01:13:50]

On 2011/08/08 20:17:49, Peng wrote:
> http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c
> File src/ibusfactory.c (right):
> 
> http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c#newcode207
> src/ibusfactory.c:207: gchar *engine_name = NULL;
> On 2011/08/06 18:24:41, Peng wrote:
> > How about call emit create-engine signal directly, and move original create
> > engine logic to create-engine default signal callback?
> 
> I mean this function may simply emit create-engine signal, if there is signal
> handler returns non-NULL, you should stop call other handlers. Otherwise
gobject
> will call default signal handler factory->create_engine which contains origin
> create engine logic (looking up the hashtable, and create an engine instance).
> Does it make sense?

But do connected handlers also need to update factory->priv->engine_list and
they return IBusEngine ?
I think your issue is to add an accumulator.

[Peng, 2011-08-09 02:30:58]

On 2011/08/09 01:13:50, fujiwara wrote:
> On 2011/08/08 20:17:49, Peng wrote:
> > http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c
> > File src/ibusfactory.c (right):
> > 
> > http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c#newcode207
> > src/ibusfactory.c:207: gchar *engine_name = NULL;
> > On 2011/08/06 18:24:41, Peng wrote:
> > > How about call emit create-engine signal directly, and move original
create
> > > engine logic to create-engine default signal callback?
> > 
> > I mean this function may simply emit create-engine signal, if there is
signal
> > handler returns non-NULL, you should stop call other handlers. Otherwise
> gobject
> > will call default signal handler factory->create_engine which contains
origin
> > create engine logic (looking up the hashtable, and create an engine
instance).
> > Does it make sense?
> 
> But do connected handlers also need to update factory->priv->engine_list and
> they return IBusEngine ?
> I think your issue is to add an accumulator.

That's right. This function should update the engine_list, and could use
accumulator to check the return value of signal handlers.

[Peng, 2011-08-09 02:30:59]

On 2011/08/09 01:13:50, fujiwara wrote:
> On 2011/08/08 20:17:49, Peng wrote:
> > http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c
> > File src/ibusfactory.c (right):
> > 
> > http://codereview.appspot.com/4801081/diff/1/src/ibusfactory.c#newcode207
> > src/ibusfactory.c:207: gchar *engine_name = NULL;
> > On 2011/08/06 18:24:41, Peng wrote:
> > > How about call emit create-engine signal directly, and move original
create
> > > engine logic to create-engine default signal callback?
> > 
> > I mean this function may simply emit create-engine signal, if there is
signal
> > handler returns non-NULL, you should stop call other handlers. Otherwise
> gobject
> > will call default signal handler factory->create_engine which contains
origin
> > create engine logic (looking up the hashtable, and create an engine
instance).
> > Does it make sense?
> 
> But do connected handlers also need to update factory->priv->engine_list and
> they return IBusEngine ?
> I think your issue is to add an accumulator.

That's right. This function should update the engine_list, and could use
accumulator to check the return value of signal handlers.

[fujiwara, 2011-08-09 02:43:30]

On 2011/08/09 02:30:59, Peng wrote:
> That's right. This function should update the engine_list, and could use
> accumulator to check the return value of signal handlers.

OK, I understood your 'This function' is ibus_factory_service_method_call. Will
implement it.

[fujiwara, 2011-08-12 10:19:35]

On 2011/08/09 02:43:30, fujiwara wrote:
> On 2011/08/09 02:30:59, Peng wrote:
> > That's right. This function should update the engine_list, and could use
> > accumulator to check the return value of signal handlers.
> 
> OK, I understood your 'This function' is ibus_factory_service_method_call.
Will
> implement it.

Updated the patch.

[Peng, 2011-08-12 13:11:34]

LGTM except some minor comments. Please fix them and use git cl push to commit
CL.

Thanks for the CL.

http://codereview.appspot.com/4801081/diff/8001/src/ibusfactory.c
File src/ibusfactory.c (right):

http://codereview.appspot.com/4801081/diff/8001/src/ibusfactory.c#newcode127
src/ibusfactory.c:127: _ibus_factory_accumulator_non_null_wins
(GSignalInvocationHint *ihint,
The name is little unclear. Could you rename it to
_ibus_factory_create_engine_accumulator?

http://codereview.appspot.com/4801081/diff/8001/src/ibusfactory.c#newcode274
src/ibusfactory.c:274: if (engine_type == G_TYPE_INVALID) {
Use engine != NULL is enough here. gobject will check the signal return type
base on the signal define.