masm11 / emacs

Mirror of GNU Emacs
http://www.gnu.org/software/emacs/
GNU General Public License v3.0
198 stars 14 forks source link

Input method behaviour #43

Closed A6GibKm closed 4 years ago

A6GibKm commented 4 years ago

One of the goals of Wayland is to be transparent to the user, user facing features should not be different from their X counterpart.

On vanilla emacs -Q 26.3, pressing ´ a results in á on a spanish (Latin America) keyboard, the interface does not tell the user it is waiting after the prefix key ´ has been pressed, this last part is not consistence with the global experience in gtk, where you see, for example image (note the underline) on firefox or other gtk apps, when waiting for a key after the prefix ´.

On the pgtk branch, pressing ´ a on emacs -Q wrongly inserts a. Adding

(when (eq window-system 'pgtk)
  (pgtk-use-im-context t))

Does fix the issue, ´ a results in á , and the behaviour is the expected one for a gtk app (the screenshot above is from pgtk Emacs).

Suggestion

Since the default emacs behaviour is to recognize á, and the expected gtk behaviour is to recognize these bindings and to show this "waiiting" as in the screenshot, (pgtk-use-im-context t) should be set as the default.

My suggestion is to add a variable, named similarly to pgtk-use-im-context, to enable (pgtk-use-im-context t) at startup, this variable is set to t by default. From my experience, this kind of behaviours are generally handled via variables instead of functions, but as long as the defaults are what they should be expected it does not matter how this is implemented.

P.S. This is the one remaining difference (regression) I can notice from vanilla Emacs, thanks for your great work.

masm11 commented 4 years ago

WIth pgtk-use-im-context, he in #30 seems not to be able to use reverse-im.el. Should he disable pgtk-use-im-context explicitly?

EDIT: Most people in the world don't need GtkIMContext. Some people think GtkIMContext is in the way. So I think GtkIMContext should be disabled by default.

A6GibKm commented 4 years ago

Fair enough. How does vanilla emacs provide this basic support for prefix keys? I just hope pgtk to not introduce regressions.

masm11 commented 4 years ago

Here, if the frame has XIC, then XmbLookupString is used. Otherwise, XLookupString is used.

For you, XLookupString is suitable. For he, XmbLookupString is suitable, maybe.

XLookupString takes compose_status as the last argument, which must be for you.

And, how to decide whether the frame should have XIC.

It is set here in create_frame_xic, which is called from here only when use_xim is true.

use_xim is set around here and here. It is controlled by configure option and X resources.

Maybe your use_xim and his are different. pgtk doesn't have use_xim. Instead it has pgtk-use-im-context.

ghost commented 4 years ago

@masm11

WIth pgtk-use-im-context, he in #30 seems not to be able to use reverse-im.el.

No-no it works as it should after you patched it. And actually I don't see why it should be disabled by default, but maybe I miss something.

masm11 commented 4 years ago

@Svadkos Thanks for the comment.

@A6GibKm I'll change to enable by default.

A6GibKm commented 4 years ago

Tested #44 and it is indeed enabled by default :+1: . I have a unrelated question, I was looking at this emacs-devel thread, are there plans to allow building against gtk4? the most interesting feature is the gpu acceleration, which as far as I know can be only be achieved in emacs using this PR at remacs.

masm11 commented 4 years ago

Thanks for the testing, but it needs more code.

Po Lu is developing Gtk4 support.

If I apply his patch and push to here, the code becomes mine. I don't hope that. His code should be his.

If he makes a PR for my fork, I'll want to accept it. When he emailed me recently, I asked him that, but he didn't. He may want to work on fsf's repo after we (I and fejfighter) push to fsf's repo.

masm11 commented 4 years ago

44 is done. could you test again?

A6GibKm commented 4 years ago

Done, works. The env variable also correctly disables IM when set to nil. Yo might want to update the README.md.

masm11 commented 4 years ago

Thanks for the testing. I'm sorry that I forgot to notice about the new variable.

I also updated README.md.