Implement embed_preedit_text.

Implement embed_preedit_text.


BUG=http://code.google.com/p/ibus/issues/detail?id=1606

[fujiwara, 2013-03-28 20:02:19]

Separated CL. Part I.
panel.vala will use set_dbus_property.

[Peng, 2013-03-28 22:08:12]

https://codereview.appspot.com/8112044/diff/1/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/8112044/diff/1/bus/ibusimpl.c#newcode182
bus/ibusimpl.c:182: "    <method name='EmbedPreeditText'>\n"
For the first CL. I think it is better to use <property>. So we can experimental
<property>. and find out how migrate others to <property>

https://codereview.appspot.com/8112044/diff/1/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/8112044/diff/1/src/ibusbus.h#newcode1047
src/ibusbus.h:1047: * ibus_bus_set_dbus_property:
Could we use ibus_bus_set_ibus_property()?

[fujiwara, 2013-03-29 01:54:43]

https://codereview.appspot.com/8112044/diff/1/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/8112044/diff/1/bus/ibusimpl.c#newcode182
bus/ibusimpl.c:182: "    <method name='EmbedPreeditText'>\n"
On 2013/03/28 22:08:12, Peng wrote:
> For the first CL. I think it is better to use <property>. So we can
experimental
> <property>. and find out how migrate others to <property>

Added <property>.

https://codereview.appspot.com/8112044/diff/1/src/ibusbus.h
File src/ibusbus.h (right):

https://codereview.appspot.com/8112044/diff/1/src/ibusbus.h#newcode1047
src/ibusbus.h:1047: * ibus_bus_set_dbus_property:
On 2013/03/28 22:08:12, Peng wrote:
> Could we use ibus_bus_set_ibus_property()?

Done.

[Peng, 2013-03-29 03:24:08]

https://codereview.appspot.com/8112044/diff/5001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/8112044/diff/5001/bus/ibusimpl.c#newcode1350
bus/ibusimpl.c:1350: ibus->embed_preedit_text = g_variant_get_boolean (value);
Please emit PropertiesChanged signal. check
org.freedesktop.DBus.Properties.PropertiesChanged

https://codereview.appspot.com/8112044/diff/5001/bus/ibusimpl.c#newcode1431
bus/ibusimpl.c:1431: gint i;
I think we need check interface_name.

like:

if (g_strcmp0(interface_name, "org.freedesktop.IBus") == 0) {
    ....
} else {
    return super_class->service_get_property(...);
}

https://codereview.appspot.com/8112044/diff/5001/ui/gtk3/panel.vala
File ui/gtk3/panel.vala (right):

https://codereview.appspot.com/8112044/diff/5001/ui/gtk3/panel.vala#newcode286
ui/gtk3/panel.vala:286: Variant var_embed_preedit = variant;
How about reuse variant?

[fujiwara, 2013-03-29 13:50:53]

https://codereview.appspot.com/8112044/diff/5001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/8112044/diff/5001/bus/ibusimpl.c#newcode1350
bus/ibusimpl.c:1350: ibus->embed_preedit_text = g_variant_get_boolean (value);
On 2013/03/29 03:24:08, Peng wrote:
> Please emit PropertiesChanged signal. check
> org.freedesktop.DBus.Properties.PropertiesChanged 

Added _emit_properties_changed()

https://codereview.appspot.com/8112044/diff/5001/bus/ibusimpl.c#newcode1431
bus/ibusimpl.c:1431: gint i;
On 2013/03/29 03:24:08, Peng wrote:
> I think we need check interface_name.
> 
> like:
> 
> if (g_strcmp0(interface_name, "org.freedesktop.IBus") == 0) {
>     ....
> } else {
>     return super_class->service_get_property(...);
> }

I think the check is done in line #1442.

https://codereview.appspot.com/8112044/diff/5001/ui/gtk3/panel.vala
File ui/gtk3/panel.vala (right):

https://codereview.appspot.com/8112044/diff/5001/ui/gtk3/panel.vala#newcode286
ui/gtk3/panel.vala:286: Variant var_embed_preedit = variant;
On 2013/03/29 03:24:08, Peng wrote:
> How about reuse variant?

If I use variant directly, the following build error is output:

panel.vala:302.13-302.73: error: Invalid assignment from owned expression to
unowned variable
            variant = m_config.get_value("general", "embed_preedit_text");
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Peng, 2013-04-27 01:31:28]

LGTM with several comments

https://codereview.appspot.com/8112044/diff/14001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/8112044/diff/14001/bus/ibusimpl.c#newcode1464
bus/ibusimpl.c:1464: g_return_val_if_reached (NULL);
Maybe print some more useful information here.
like property_name

https://codereview.appspot.com/8112044/diff/14001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/8112044/diff/14001/src/ibusbus.c#newcode2116
src/ibusbus.c:2116: DBUS_INTERFACE_PROPERTIES,
Maybe using "org.freedesktop.DBus.Properties" directly is better?

https://codereview.appspot.com/8112044/diff/14001/ui/gtk3/panel.vala
File ui/gtk3/panel.vala (right):

https://codereview.appspot.com/8112044/diff/14001/ui/gtk3/panel.vala#newcode286
ui/gtk3/panel.vala:286: Variant var_embed_preedit = variant;
why not use variant directly?

[fujiwara, 2013-04-30 03:08:31]

https://codereview.appspot.com/8112044/diff/14001/bus/ibusimpl.c
File bus/ibusimpl.c (right):

https://codereview.appspot.com/8112044/diff/14001/bus/ibusimpl.c#newcode1464
bus/ibusimpl.c:1464: g_return_val_if_reached (NULL);
On 2013/04/27 01:31:28, Peng wrote:
> Maybe print some more useful information here.
> like property_name 

OK, I used g_warning.

https://codereview.appspot.com/8112044/diff/14001/src/ibusbus.c
File src/ibusbus.c (right):

https://codereview.appspot.com/8112044/diff/14001/src/ibusbus.c#newcode2116
src/ibusbus.c:2116: DBUS_INTERFACE_PROPERTIES,
On 2013/04/27 01:31:28, Peng wrote:
> Maybe using "org.freedesktop.DBus.Properties" directly is better?

Done.

https://codereview.appspot.com/8112044/diff/14001/ui/gtk3/panel.vala
File ui/gtk3/panel.vala (right):

https://codereview.appspot.com/8112044/diff/14001/ui/gtk3/panel.vala#newcode286
ui/gtk3/panel.vala:286: Variant var_embed_preedit = variant;
On 2013/04/27 01:31:28, Peng wrote:
> why not use variant directly?

I replied in the previous comment:
If I use variant directly, the following build error is output:

panel.vala:302.13-302.73: error: Invalid assignment from owned expression to
unowned variable
            variant = m_config.get_value("general", "embed_preedit_text");
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

So that line assigns unowned value to owned value.