D11061: Migration request from IBusConfig to GSettings

2018-03-07 Thread Xuetian Weng
xuetianweng added a comment.


  Not so sure why phabricator didn't set you as the commit author... sorry 
about that.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-07 Thread Xuetian Weng
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:e2a29f8ea072: Migration request from IBusConfig to 
GSettings (authored by xuetianweng).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11061?vs=28920&id=28964

REVISION DETAIL
  https://phabricator.kde.org/D11061

AFFECTED FILES
  applets/kimpanel/backend/ibus/ibus15/panel.cpp

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-07 Thread Takao Fujiwara
fujiwara updated this revision to Diff 28920.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11061?vs=28881&id=28920

REVISION DETAIL
  https://phabricator.kde.org/D11061

AFFECTED FILES
  applets/kimpanel/backend/ibus/ibus15/panel.cpp

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-07 Thread Xuetian Weng
xuetianweng added inline comments.

INLINE COMMENTS

> panel.cpp:922
> +impanel_update_engines(impanel, var_engines);
> +if (var_engines)
> +g_variant_unref(var_engines);

https://community.kde.org/Policies/Kdelibs_Coding_Style#Braces

Always use braces

> panel.cpp:933
> +impanel_update_triggers(impanel, var_triggers);
> +if (var_triggers)
> +g_variant_unref(var_triggers);

Same as above

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread Takao Fujiwara
fujiwara added a comment.


  In D11061#219841 , @davidedmundson 
wrote:
  
  > FWIW, there's a lovely Qt-GConf wrapper in plasma-pa/gconfitem
  
  
  I may drop gconf and dconf support in IBus 1.6. It would be nice if Qt API 
could read GSettings.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread Takao Fujiwara
fujiwara marked an inline comment as done.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread Takao Fujiwara
fujiwara updated this revision to Diff 28881.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11061?vs=28793&id=28881

REVISION DETAIL
  https://phabricator.kde.org/D11061

AFFECTED FILES
  applets/kimpanel/backend/ibus/ibus15/panel.cpp

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread Xuetian Weng
xuetianweng requested changes to this revision.
xuetianweng added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> panel.cpp:1049
> +g_signal_handlers_disconnect_by_func (impanel->settings_hotkey, 
> (gpointer)impanel_settings_changed_callback, impanel);
> +g_clear_pointer (&impanel->settings_general, g_object_unref);
> +g_clear_pointer (&impanel->settings_hotkey, g_object_unref);

I think you can use g_clear_object here.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread Xuetian Weng
xuetianweng added a comment.


  In D11061#219841 , @davidedmundson 
wrote:
  
  > FWIW, there's a lovely Qt-GConf wrapper in plasma-pa/gconfitem
  
  
  imho, this is pretty irrelevant here.. gsettings is not directly gconf and 
gconf itself is deprecated since gtk3 iirc.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread David Edmundson
davidedmundson added a comment.


  FWIW, there's a lovely Qt-GConf wrapper in plasma-pa/gconfitem

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: davidedmundson, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread Takao Fujiwara
fujiwara requested review of this revision.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread Takao Fujiwara
fujiwara updated this revision to Diff 28793.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11061?vs=28722&id=28793

REVISION DETAIL
  https://phabricator.kde.org/D11061

AFFECTED FILES
  applets/kimpanel/backend/ibus/ibus15/panel.cpp

To: fujiwara, #plasma, hein, xuetianweng
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-06 Thread Takao Fujiwara
fujiwara planned changes to this revision.
fujiwara marked 2 inline comments as done.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-05 Thread Xuetian Weng
xuetianweng added inline comments.

INLINE COMMENTS

> panel.cpp:1031
>  impanel->xkbLayoutManager = new XkbLayoutManager;
> +impanel->settings_general = g_settings_new (IBUS_SCHEMA_GENERAL);
> +impanel->settings_hotkey = g_settings_new (IBUS_SCHEMA_HOTKEY);

IMHO you need to g_object_unref this two new settings in 
ibus_panel_impanel_destroy.

> panel.cpp:1033
> +impanel->settings_hotkey = g_settings_new (IBUS_SCHEMA_HOTKEY);
> +g_signal_connect(impanel->settings_general, "changed", G_CALLBACK 
> (impanel_settings_changed_callback), impanel);
> +g_signal_connect(impanel->settings_hotkey, "changed", G_CALLBACK 
> (impanel_settings_changed_callback), impanel);

and disconnect the signal too.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-05 Thread Xuetian Weng
xuetianweng requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-05 Thread Takao Fujiwara
fujiwara added a comment.


  In D11061#219603 , @xuetianweng 
wrote:
  
  > May I ask when this gsettings is introduced? Is it required after certain 
ibus version? Is there any case that this will not work?
  >
  > If so I'd like to see a version check on ibus at compile time.
  
  
  GSettings has been supported in IBus 1.5.0 at least and you don't have to 
mind IBus.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-05 Thread Xuetian Weng
xuetianweng added a comment.


  May I ask when this gsettings is introduced? Is it required after certain 
ibus version? Is there any case that this will not work?
  
  If so I'd like to see a version check on ibus at compile time.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-05 Thread Christoph Feck
cfeck added a reviewer: xuetianweng.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein, xuetianweng
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-05 Thread Luca Beltrame
lbeltrame added reviewers: Plasma, hein.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

To: fujiwara, #plasma, hein
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D11061: Migration request from IBusConfig to GSettings

2018-03-05 Thread Takao Fujiwara
fujiwara created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
fujiwara requested review of this revision.

REVISION SUMMARY
  kimpanel-ibus-panel uses IBusConfig and I'd ask to migrate it to GSettings 
and delete IBusConfig.
  
  https://groups.google.com/forum/#!topic/ibus-devel/Mu1IoFX-bKE

TEST PLAN
  Run ibus-setup and changes "Next input method" or IMEs and KIM panel works 
fine.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11061

AFFECTED FILES
  applets/kimpanel/backend/ibus/ibus15/panel.cpp

To: fujiwara
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart