Simplify surrounding-text initialization.

Simplify surrounding-text initialization.

Currently the immodule tries to retrieve surrounding-text unconditionally
on focus_in and enabled.  These calls could be eliminated if engine were
able to proclaim that it will need surrounding-text.

This patch extends ibus_engine_get_surrounding_text() to allow this.
Engines that need surrounding-text are expected to have:

    /* Indicate we will use surrounding-text. */
    ibus_engine_get_surrounding_text (engine, NULL, NULL);

in their enable() method.  This would work because enable() is called before
SetCapabilities DBus call.

BUG=none
TEST=manually with ibus-m17n, with the above change.

[Daiki Ueno, 2011-06-14 07:25:16]

In Fedora, we have applied a patch to enable/disable surrounding-text using
component XML:

http://pkgs.fedoraproject.org/gitweb/?p=ibus.git;a=blob;h=2bde53cc;hb=HEAD

which could be superseded with this patch.

Tested with modified ibus-m17n:
http://codereview.appspot.com/4589052

[Peng, 2011-06-15 14:16:01]

Did you update this CL successfully? It looks like some file are not updated.
Please upload this CL again.

On 2011/06/14 07:25:16, Daiki Ueno wrote:
> In Fedora, we have applied a patch to enable/disable surrounding-text using
> component XML:
> 
> http://pkgs.fedoraproject.org/gitweb/?p=ibus.git;a=blob;h=2bde53cc;hb=HEAD
> 
> which could be superseded with this patch.
> 
> Tested with modified ibus-m17n:
> http://codereview.appspot.com/4589052

[Daiki Ueno, 2011-06-16 06:43:25]

On 2011/06/15 14:16:01, Peng wrote:
> Did you update this CL successfully? It looks like some file are not updated.
> Please upload this CL again.

Hmm, I remember that git cl from the latest depot_tools didn't complain
anything.  Now I just uploaded a new CL (Patch Set 2) rebased against the
current git master, but there seems no change since Patch Set 1.  If this does
not cleanly apply, I'll open a new review request.

[Peng, 2011-06-16 16:10:36]

I don't know why I can not publish an inline comment in configure.ac for this
CL. I have to copy it to here.

For configure.ac
I think the surrounding text is not well supported right now from clients. So I
suggest keep it disable by default right now.

https://codereview.appspot.com/4613043/diff/3001/client/gtk2/ibusimcontext.c
File client/gtk2/ibusimcontext.c (right):

https://codereview.appspot.com/4613043/diff/3001/client/gtk2/ibusimcontext.c#...
client/gtk2/ibusimcontext.c:271: ibus_input_context_needs_surrounding_text
(context->ibuscontext)) {
Does ibus_input_context_needs_surrounding_text() return FALSE, if user switch to
an engine which does not support surrounding text?

[Daiki Ueno, 2011-06-17 03:11:03]

https://codereview.appspot.com/4613043/diff/3001/client/gtk2/ibusimcontext.c
File client/gtk2/ibusimcontext.c (right):

https://codereview.appspot.com/4613043/diff/3001/client/gtk2/ibusimcontext.c#...
client/gtk2/ibusimcontext.c:271: ibus_input_context_needs_surrounding_text
(context->ibuscontext)) {
On 2011/06/16 16:10:36, Peng wrote:
> Does ibus_input_context_needs_surrounding_text() return FALSE, if user switch
to
> an engine which does not support surrounding text?

Yes, it is.  needs_surrounding_text flag is cleared on "Enabled" signal on input
context (ibusinputcontext.c:656).  The signal is always emitted when engine is
switched, through bus_input_context_set_engine_by_desc.

[Peng, 2011-06-17 03:43:18]

You did not reply the comment for configure.ac.?

On 2011/06/17 03:11:03, Daiki Ueno wrote:
> https://codereview.appspot.com/4613043/diff/3001/client/gtk2/ibusimcontext.c
> File client/gtk2/ibusimcontext.c (right):
> 
>
https://codereview.appspot.com/4613043/diff/3001/client/gtk2/ibusimcontext.c#...
> client/gtk2/ibusimcontext.c:271: ibus_input_context_needs_surrounding_text
> (context->ibuscontext)) {
> On 2011/06/16 16:10:36, Peng wrote:
> > Does ibus_input_context_needs_surrounding_text() return FALSE, if user
switch
> to
> > an engine which does not support surrounding text?
> 
> Yes, it is.  needs_surrounding_text flag is cleared on "Enabled" signal on
input
> context (ibusinputcontext.c:656).  The signal is always emitted when engine is
> switched, through bus_input_context_set_engine_by_desc.

[Daiki Ueno, 2011-06-17 03:56:56]

On 2011/06/17 03:43:18, Peng wrote:
> You did not reply the comment for configure.ac.?

Sorry.  I don't have strong opinion on it - I agree to keep it disabled by
default if you like.

Anyway, with this change, ibus won't emit "retrieve-surrounding" signal anymore,
until engine requests it.  So I think it should be safe even if it is enabled by
default.

[Peng, 2011-06-18 03:52:39]

On 2011/06/17 03:56:56, Daiki Ueno wrote:
> On 2011/06/17 03:43:18, Peng wrote:
> > You did not reply the comment for configure.ac.?
> 
> Sorry.  I don't have strong opinion on it - I agree to keep it disabled by
> default if you like.
> 
> Anyway, with this change, ibus won't emit "retrieve-surrounding" signal
anymore,
> until engine requests it.  So I think it should be safe even if it is enabled
by
> default.

LGTM. I merged it with a little change in configure.ac. Thx.

[fujiwara, 2011-06-20 04:37:32]

On 2011/06/14 07:25:16, Daiki Ueno wrote:
> In Fedora, we have applied a patch to enable/disable surrounding-text using
> component XML:

It seems this does not include the xml patch so _request_surrounding_text()
always emits "retrieve-surrounding" in focus_in and enabled class methods.
My understanding is this is the different behavior from Fedora. I'd expected to
emit "retrieve-surrounding" in focus_in for the specific engines only.

[fujiwara, 2011-06-20 04:47:31]

On 2011/06/20 04:37:32, fujiwara wrote:
> On 2011/06/14 07:25:16, Daiki Ueno wrote:
> > In Fedora, we have applied a patch to enable/disable surrounding-text using
> > component XML:
> 
> It seems this does not include the xml patch so _request_surrounding_text()
> always emits "retrieve-surrounding" in focus_in and enabled class methods.
> My understanding is this is the different behavior from Fedora. I'd expected
to
> emit "retrieve-surrounding" in focus_in for the specific engines only.

Ah, OK, I mistook '+' and '-' in _request_surrounding_text() about the 'force'
argument and it looks same with Fedora.
Thanks.