Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
Added early in this cycle for testing. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#issuecomment-320167344
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
Closed #1514 via 08e2714c1cf6e341da9a9c253ab938ae5bea12c5. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#event-1192613891
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
@sinpowei well, somebody (other than you :) needs to test it, but not many Geany contributors use IME languages so most (all?) can't test it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#issuecomment-317369842
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
@b4n I've already changed the code many days ago, how can this be merged into new release? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#issuecomment-317367501
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
@b4n @elextr , I've changed the `ime_interaction` to int type, and the set default value to 0, as same as scintilla's default. Because scintilla use an enum for this option, and always initialize it to 0 (windowed) , so I'm not use an additional 'if' for 'none set' case. @elextr, I've also moved the setup code from `editor_apply_update_prefs` to `create_new_sci`, I was expecting it applies immediately, but it doesn't seem to work very well, so change back. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#issuecomment-307682141
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
@sinpowei pushed 1 commit. 5144e59 Add an editor option to set IME's candidate window behaviour. -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1514/files/ced43efe4e6e60fe3d878e66798034ebea0606d5..5144e59418281e189804ba2f524c04b3b5bc28c3
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
@b4n, well, the scintilla [docs](http://www.scintilla.org/ScintillaDoc.html#SCI_SETIMEINTERACTION) reads to me that Scintilla doesn't have control on some platforms, but it does on others, and there is nothing about which is default on the platforms where it does have control. So I read that as, on platforms where it has a choice, it leaves the default up to the platform unless the user sets one option or the other, otherwise why have two options, just have say windowed as default and set inline instead? I havn't checked Scintilla's code, I'm not really interested in digging into several platform layers that I know nothing about. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#issuecomment-307601078
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
> The way I read the Scintilla documentation, how the IME defaults is platform > dependent, so setting neither may be different to setting one or the other, > so the current behaviour of not setting any ime interaction should be > preserved as the default. Is it? It sounds fairly odd for a Scintilla setting to work that way. But indeed if it is the case we should be able to preserve the default. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#issuecomment-307600849
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
@sinpowei thanks for making this, and extra marks for adding the documentation in the manual :smile:. The way I read the Scintilla documentation, how the IME defaults is platform dependent, so setting neither may be different to setting one or the other, so the current behaviour of not setting any ime interaction should be preserved as the default. So if you follow @b4n's suggestion you need three options "none", "inline" or "windowed" or you can follow my suggestion of setting only inline in an if and retain the boolean setting. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#issuecomment-307595488
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
elextr commented on this pull request. > @@ -2527,6 +2527,8 @@ show_editor_scrollbarsWhether to display > scrollbars. If set to t indent_hard_tab_width The size of a tab character. Don't change 8 immediately it unless you really need to; use the indentation settings instead. +editor_ime_inline Whether to display input method editor's trueto new + candidate window inline documents The setting is in `editor_apply_update_prefs` which comments that it is non-document prefs, so does this really apply to new documents only? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#pullrequestreview-43307734
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
elextr commented on this pull request. > @@ -5172,6 +5172,9 @@ void editor_apply_update_prefs(GeanyEditor *editor) /* virtual space */ SSM(sci, SCI_SETVIRTUALSPACEOPTIONS, editor_prefs.show_virtual_space, 0); + /* input method editor's candidate window behaviour */ + SSM(sci, SCI_SETIMEINTERACTION, editor_prefs.ime_interaction, 0); + The current situation is that no IME interaction is set, unless you can be sure that setting an interaction has no effect anywhere else, then not setting it at all should be the default. So you can use your boolean and put this in an if. That would also actually match your documentation above. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#pullrequestreview-43307710
Re: [Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
b4n requested changes on this pull request. Not tested yet, but looks almost good, see comments. > @@ -137,6 +137,7 @@ typedef struct GeanyEditorPrefs gbooleanlong_line_enabled; gintautocompletion_update_freq; gintscroll_lines_around_cursor; + gbooleanime_interaction; /* input method editor's candidate window behaviour */ This should be an int and use on of the Scintilla constants, `SC_IME_WINDOWED` or `SC_IME_INLINE`, so it can be easily extended in the future, and have a semantic meaning. > @@ -255,6 +255,8 @@ static void init_pref_groups(void) "extract_filetype_regex", GEANY_DEFAULT_FILETYPE_REGEX); stash_group_add_boolean(group, &search_prefs.replace_and_find_by_default, "replace_and_find_by_default", TRUE); + stash_group_add_boolean(group, &editor_prefs.ime_interaction, + "editor_ime_inline", TRUE); Same here, should be ```C stash_group_add_integer(group, &editor_prefs.ime_interaction, "editor_ime_interaction", SC_IME_WINDOWED); ``` And better keep the current behavior as default unless we have a compelling set of data that the current default is a problem for everyone. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514#pullrequestreview-43305226
[Github-comments] [geany/geany] Add an editor option to enable IME's candidate window display inline (#1514)
This will fix the issue that candidate window can't follow cursor. See https://github.com/geany/geany/issues/1513 and https://github.com/geany/geany/issues/920 You can view, comment on, or merge this pull request online at: https://github.com/geany/geany/pull/1514 -- Commit Summary -- * Add an editor option to enable IME's candidate window display inline, -- File Changes -- M doc/geany.txt (2) M src/editor.c (3) M src/editor.h (1) M src/keyfile.c (2) -- Patch Links -- https://github.com/geany/geany/pull/1514.patch https://github.com/geany/geany/pull/1514.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1514
