pre-command-hook with input methods

pre-command-hook with input methods

[Phillip Lord]

I'm trying to understand the behaviour of pre-command-hook with an input
method set. Using the code below to probe this, I find that with
italian-postfix the pre-command-hook does not get run when I expect.

For instance consider this scenario

Key   Cumulative OnScreen   p-c-h-fired?

g     g                     Yes
a     ga[_`]                No
b     gab                   Yes, twice


So, the pre-command-hook runs only *after* the multi-key press behaviour
has been completed (either by introducing a diacritical letter or by not
doing so). Hence on pressing "b" it gets run twice -- once to say "a has
been entered" and once to say "b has been entered".

All fine, but it's breaking my completion framework which removes
previously offered completions on the pre-command-hook. I need it to run
as soon as the "a" key has been pressed.

Is there a better hook?


(add-hook 'pre-command-hook
          'temp-pre-command-hook)

(defvar-local temp-enable nil)
(defvar temp-count 1)

(defun temp-pre-command-hook ()
  (when temp-enable
    (message "pch:%s"
             (incf temp-count))))

Re: pre-command-hook with input methods

[Stefan Monnier]

> So, the pre-command-hook runs only *after* the multi-key press behaviour
> has been completed (either by introducing a diacritical letter or by not
> doing so). Hence on pressing "b" it gets run twice -- once to say "a has
> been entered" and once to say "b has been entered".

This is correct, since when you hit `a' the self-insert-command is not
run yet (instead the `a' char is inserted in the buffer by the input
method, which then waits for the next key to see which character was
really meant); it's only run after you hit `b'.

> All fine, but it's breaking my completion framework which removes
> previously offered completions on the pre-command-hook.

Indeed, I wouldn't be surprised if several other uses of
pre-command-hook could suffer from unexpected behaviors in
such situations.

> I need it to run as soon as the "a" key has been pressed.

Of course, you can do that using one of the input-method hooks (maybe
input-method-after-insert-chunk-hook), or you could rely on
before-change-functions.

> Is there a better hook?

I don't think so, sadly.

Maybe we could introduce a new hook like `after-idle-hook'.  Part of the
question is what should happen if a timer or a process filter runs
(these can run while Emacs is "idle")?  How 'bout when that
timer/process-filter inserts text near/at point?


        Stefan

Re: pre-command-hook with input methods

[Phillip Lord]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> All fine, but it's breaking my completion framework which removes
>> previously offered completions on the pre-command-hook.
>
> Indeed, I wouldn't be surprised if several other uses of
> pre-command-hook could suffer from unexpected behaviors in
> such situations.

At a quick test, the auto-complete package also behaves badly wrt input
methods. The on-screen output is bizarre, although what you end up with
in the buffer looks correct, so I guess it is the same problem.

>
>> I need it to run as soon as the "a" key has been pressed.
>
> Of course, you can do that using one of the input-method hooks (maybe
> input-method-after-insert-chunk-hook), or you could rely on
> before-change-functions.

Gave that a quick go. As far as I can see, after-insert-chunk-hook
runs after the input is complete. Likewise, b-c-f which only runs when
the buffer has changed.


>> Is there a better hook?
>
> I don't think so, sadly.
>
> Maybe we could introduce a new hook like `after-idle-hook'.  Part of the
> question is what should happen if a timer or a process filter runs
> (these can run while Emacs is "idle")?  How 'bout when that
> timer/process-filter inserts text near/at point?

I do buffer analytics in the idle cycle -- I would guess that the same
thing is true for most completion packages. So I'd need to be able to
distinguish between this and the user doing something.

What about after-user-interaction-hook? Or
after-something-has-happened-that-changes-the-display-on-screen-of-a-buffer.
Although the latter might be affected by things that change the mode
line in the idle cycle (like a word count mode).

Phil

Re: pre-command-hook with input methods

[Stefan Monnier]

> Gave that a quick go.  As far as I can see, after-insert-chunk-hook
> runs after the input is complete.

Hmm...

> Likewise, b-c-f which only runs when the buffer has changed.

No, it should run *before* the buffer is changed, and AFAIK the buffer
*is* changed (by inserting an "a") when you hit the "a".  this "a" will
get removed later (after you hit the "b") before running the
self-insert-command which will (re)do the insertion.

Oh, wait, I see that quail-input-method starts by let-binding
inhibit-modification-hooks, so indeed, before-change-functions won't be run.
Hmm... of course, you can still try

   (add-function :before input-method-function ...)

but since its default value is nil (i.e. incompatible with
add-function), it'll be a bit more tricky (you'll need an :around
advice, and you'll need to check the orig function (in case it's nil)
before calling it).

There's clearly room for improvement here.


        Stefan

Re: pre-command-hook with input methods

[Phillip Lord]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Gave that a quick go.  As far as I can see, after-insert-chunk-hook
>> runs after the input is complete.
>
> Hmm...
>
>> Likewise, b-c-f which only runs when the buffer has changed.
>
> No, it should run *before* the buffer is changed, and AFAIK the buffer
> *is* changed (by inserting an "a") when you hit the "a".  this "a" will
> get removed later (after you hit the "b") before running the
> self-insert-command which will (re)do the insertion.
>
> Oh, wait, I see that quail-input-method starts by let-binding
> inhibit-modification-hooks, so indeed, before-change-functions won't be run.
> Hmm... of course, you can still try
>
>    (add-function :before input-method-function ...)
>
> but since its default value is nil (i.e. incompatible with
> add-function), it'll be a bit more tricky


That's because by default there's no input method active. So, I guess,
the trick would be to use input-method-activate-hook to advice the
input-method-function.


> (you'll need an :around advice, and you'll need to check the orig
> function (in case it's nil) before calling it).
>
> There's clearly room for improvement here.


A bit messy, yes. I blame all foreigners and their diacritics.

Phil

Re: pre-command-hook with input methods

[Stefan Monnier]

> That's because by default there's no input method active.  So, I guess,
> the trick would be to use input-method-activate-hook to advice the
> input-method-function.

The Right Way to fix this is to change the default value to be non-nil.
In this case, I think using #'list might work.

>> (you'll need an :around advice, and you'll need to check the orig
>> function (in case it's nil) before calling it).
>> There's clearly room for improvement here.
> A bit messy, yes. I blame all foreigners and their diacritics.

Obviously if we could ditch those damn foreigners things would be
a lot easier.


        Stefan

Re: pre-command-hook with input methods

[Phillip Lord]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> That's because by default there's no input method active.  So, I guess,
>> the trick would be to use input-method-activate-hook to advice the
>> input-method-function.
>
> The Right Way to fix this is to change the default value to be non-nil.
> In this case, I think using #'list might work.

I am not sure how that would work. Surely the point of having this as a
variable is that it can be changed?

I could just advice quail-input-method and robin-input-method which
a quick grep suggests are the only two functions of relevance.

Phil

Re: pre-command-hook with input methods

[Stefan Monnier]

>> The Right Way to fix this is to change the default value to be non-nil.
>> In this case, I think using #'list might work.
> I am not sure how that would work.  Surely the point of having this as a
> variable is that it can be changed?

I'm only suggesting to change the default value from nil to `list',
which I just did in "master".

This then allows you to use `add-function' on that variable.

But I still think there's room for improvement with something like
a `pre-interaction-hook' or something like that.  Brings me to the next
question.  We know you'd like such a pre-interaction-hook to be run just
before processing the first char in a multi-char input-method element,
but what about the simpler/common case of a key prefix:

When the user hits C-x C-f, would you like to run this
pre-interaction-hook right when the user hits C-x, or only after the
user hit C-x C-f?

Maybe what you're after is just an input-event-hook?


        Stefan

Re: pre-command-hook with input methods

[Phillip Lord]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> The Right Way to fix this is to change the default value to be non-nil.
>>> In this case, I think using #'list might work.
>> I am not sure how that would work.  Surely the point of having this as a
>> variable is that it can be changed?
>
> I'm only suggesting to change the default value from nil to `list',
> which I just did in "master".
>
> This then allows you to use `add-function' on that variable.

Ah, sorry. The advice is actually on the variable, and no on the
symbol stored in the variable? So it runs even if the value changes?



> But I still think there's room for improvement with something like
> a `pre-interaction-hook' or something like that.  Brings me to the next
> question.  We know you'd like such a pre-interaction-hook to be run just
> before processing the first char in a multi-char input-method element,
> but what about the simpler/common case of a key prefix:
>
> When the user hits C-x C-f, would you like to run this
> pre-interaction-hook right when the user hits C-x, or only after the
> user hit C-x C-f?
>
> Maybe what you're after is just an input-event-hook?

So, currently, the offered completion remains in buffer until the key
sequence has been completed. Once you have typed the first part of the
keysequence through (C-u or C-x) then the abbreviation is going to
disappear one way or the other. Either you complete the sequence (which
runs a command) or you quit (which runs a command).

I think for consistency, the hook should run immediately before or after
Emacs responds to C-u (or C-x) depending on whether it is pre- or post-.
Effectively, the C-x of a C-xC-f is, I think equivalent to the first "a"
of a "a`" post-fix input key sequence. The only reason that the former works
fine for me, and the latter is problematic is the latter changes the
display of the buffer while the former changes just the mini-buffer.

So, my initial feeling is that it doesn't make any difference for this
particular use case.

The other possibility is not to have an interaction hook but to have a
"the buffer has just changed in some way that is liable to cause a
redisplay"-hook. I don't know that this would be better. I throw it out
as a possibility, depending on which is easier.

Phil

Re: pre-command-hook with input methods

[Stefan Monnier]

> Ah, sorry.  The advice is actually on the variable, and no on the
> symbol stored in the variable? So it runs even if the value changes?

Ah, you're right that other packages won't use add-function, so they'll
stomp on our advice!

>> Maybe what you're after is just an input-event-hook?
> I think for consistency, the hook should run immediately before or after
> Emacs responds to C-u (or C-x) depending on whether it is pre- or post-.
> Effectively, the C-x of a C-xC-f is, I think equivalent to the first "a"
> of a "a`" post-fix input key sequence.  The only reason that the former
> works fine for me, and the latter is problematic is the latter changes the
> display of the buffer while the former changes just the mini-buffer.

So it sounds like you'd indeed be happy with an input-event-hook.

> The other possibility is not to have an interaction hook but to have a
> "the buffer has just changed in some way that is liable to cause a
> redisplay"-hook. I don't know that this would be better.  I throw it out
> as a possibility, depending on which is easier.

Then maybe the (new in 24.4) pre-redisplay-function could be used
for that.  Something like

    (add-function :before pre-redisplay-function
                  (lambda (wins)
                    (if (or (eq t wins)
                            (memq <mybuffer> (mapcar #'window-buffer wins)))
                        <take-foo-down>)))

Of course, this will require some care, since a redisplay happens
probably right after you've set things up (i.e. before you start
waiting for user input), and you don't want *that* redisplay to trigger
the <take-foo-down>.


        Stefan

Re: pre-command-hook with input methods

[Phillip Lord]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> Maybe what you're after is just an input-event-hook?
>> I think for consistency, the hook should run immediately before or after
>> Emacs responds to C-u (or C-x) depending on whether it is pre- or post-.
>> Effectively, the C-x of a C-xC-f is, I think equivalent to the first "a"
>> of a "a`" post-fix input key sequence.  The only reason that the former
>> works fine for me, and the latter is problematic is the latter changes the
>> display of the buffer while the former changes just the mini-buffer.
>
> So it sounds like you'd indeed be happy with an input-event-hook.

I think so yes -- so long as input-event means "user input-event" or I
have some way of telling. Something coming in from a process, or
file-watch notifications need to be ignored.


>> The other possibility is not to have an interaction hook but to have a
>> "the buffer has just changed in some way that is liable to cause a
>> redisplay"-hook. I don't know that this would be better.  I throw it out
>> as a possibility, depending on which is easier.
>
> Then maybe the (new in 24.4) pre-redisplay-function could be used
> for that.  Something like
>
>     (add-function :before pre-redisplay-function
>                   (lambda (wins)
>                     (if (or (eq t wins)
>                             (memq <mybuffer> (mapcar #'window-buffer wins)))
>                         <take-foo-down>)))
>
> Of course, this will require some care, since a redisplay happens
> probably right after you've set things up (i.e. before you start
> waiting for user input), and you don't want *that* redisplay to trigger
> the <take-foo-down>.


Yes, that would be tricky. I'd probably have to ignore the first
redisplay after a completion has been offered. Also, my package does
idle cycle operations which include "sit-for" as a way of returning
control to the user. These trigger redisplay also which would be
unfortunate.

Probably they shouldn't do that. I *think* I added that so I could have
the idle-cycle operations add visible overlays for debugging purposes,
so the sit-for is necessary.

Still, it's there now, so I will give it a go. But having the
input-event-hook might be better.

Phil

Re: pre-command-hook with input methods

[Phillip Lord]

Phillip Lord <phillip.lord@newcastle.ac.uk> writes:
>>> The other possibility is not to have an interaction hook but to have a
>>> "the buffer has just changed in some way that is liable to cause a
>>> redisplay"-hook. I don't know that this would be better.  I throw it out
>>> as a possibility, depending on which is easier.
>>
>> Then maybe the (new in 24.4) pre-redisplay-function could be used
>> for that.  Something like
>>
>>     (add-function :before pre-redisplay-function
>>                   (lambda (wins)
>>                     (if (or (eq t wins)
>>                             (memq <mybuffer> (mapcar #'window-buffer wins)))
>>                         <take-foo-down>)))
>>
>> Of course, this will require some care, since a redisplay happens
>> probably right after you've set things up (i.e. before you start
>> waiting for user input), and you don't want *that* redisplay to trigger
>> the <take-foo-down>.
> [snip]
>
> Still, it's there now, so I will give it a go. But having the
> input-event-hook might be better.


Given this a quick go. I was a bit confused because it's
"pre-redisplay-function" and not "pre-redisplay-functions", so I spent a
while using add-hook (didn't have your email to hand at the time).

Once I fixed that, I found that pre-redisplay-function gets called an
awful lot. It's very hard for me to work out why, because all my
debugging techniques for emacs involve some output to screen (i.e.
redisplay!).

For completion purposes though, it does look like pre-redisplay-function
is going to trigger far too often.

I'll try advising the input-method next!

Phil

Re: pre-command-hook with input methods

[Stefan Monnier]

> Once I fixed that, I found that pre-redisplay-function gets called an
> awful lot.

Yes.  Emacs needs to redisplay very often.  E.g. blinking the cursor
requires "redisplay".


        Stefan

Re: pre-command-hook with input methods

[Phillip Lord]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Once I fixed that, I found that pre-redisplay-function gets called an
>> awful lot.
>
> Yes.  Emacs needs to redisplay very often.  E.g. blinking the cursor
> requires "redisplay".


Ah, of course. Which will (potentially) happen in any buffer with a
completion.

Phil

Re: pre-command-hook with input methods

[Stefan Monnier]

> I think so yes -- so long as input-event means "user input-event" or I
> have some way of telling. Something coming in from a process, or
> file-watch notifications need to be ignored.

Could you try the patch below to see if it provides the feature
you need?


        Stefan


diff --git a/etc/NEWS b/etc/NEWS
index 1cccc31..f1b7119 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -811,6 +811,8 @@ behavior, set `diff-switches' to `-c'.
 \f
 * Lisp Changes in Emacs 25.1
 
+** New hook `input-event-functions' run whenever a user-input event is read.
+
 ** The default value of `load-read-function' is now `read'.
 
 ** New hook `pre-redisplay-functions', a bit easier to use than pre-redisplay-function.
diff --git a/lisp/subr.el b/lisp/subr.el
index b9a847d..df0ed42 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1110,6 +1110,12 @@ See `event-start' for a description of the value returned."
   "Return the multi-click count of EVENT, a click or drag event.
 The return value is a positive integer."
   (if (and (consp event) (integerp (nth 2 event))) (nth 2 event) 1))
+
+(defvar input-event-functions nil
+  ;; BEWARE: If it looks like this is not run anywhere, it's normal:
+  ;; this is run in keyboard.c.
+  "Special hook run each time a user-input event is read.
+Each function is called with one argument: the event.")
 \f
 ;;;; Extracting fields of the positions in an event.
 
diff --git a/src/keyboard.c b/src/keyboard.c
index eb66c44..5c64199 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2454,7 +2454,7 @@ read_char (int commandflag, Lisp_Object map,
 	  /* Also check was_disabled so last-nonmenu-event won't return
 	     a bad value when submenus are involved.  (Bug#447)  */
 	  && (EQ (c, Qtool_bar) || EQ (c, Qmenu_bar) || was_disabled))
-	*used_mouse_menu = 1;
+	*used_mouse_menu = true;
 
       goto reread_for_input_method;
     }
@@ -2995,6 +2995,8 @@ read_char (int commandflag, Lisp_Object map,
   if (! NILP (also_record))
     record_char (also_record);
 
+  Frun_hook_with_args (2, ((Lisp_Object []) {Qinput_event_functions, c}));
+
   /* Wipe the echo area.
      But first, if we are about to use an input method,
      save the echo area contents for it to refer to.  */
@@ -3986,7 +3988,7 @@ kbd_buffer_get_event (KBOARD **kbp,
             obj = list1 (intern ("ns-unput-working-text"));
 	  kbd_fetch_ptr = event + 1;
           if (used_mouse_menu)
-            *used_mouse_menu = 1;
+            *used_mouse_menu = true;
         }
 #endif
 
@@ -4181,13 +4183,13 @@ kbd_buffer_get_event (KBOARD **kbp,
 		  && !EQ (event->frame_or_window, event->arg)
 		  && (event->kind == MENU_BAR_EVENT
 		      || event->kind == TOOL_BAR_EVENT))
-		*used_mouse_menu = 1;
+		*used_mouse_menu = true;
 #endif
 #ifdef HAVE_NS
 	      /* Certain system events are non-key events.  */
 	      if (used_mouse_menu
                   && event->kind == NS_NONKEY_EVENT)
-		*used_mouse_menu = 1;
+		*used_mouse_menu = true;
 #endif
 
 	      /* Wipe out this event, to catch bugs.  */
@@ -8445,7 +8447,7 @@ read_char_x_menu_prompt (Lisp_Object map,
 			 Lisp_Object prev_event, bool *used_mouse_menu)
 {
   if (used_mouse_menu)
-    *used_mouse_menu = 0;
+    *used_mouse_menu = false;
 
   /* Use local over global Menu maps.  */
 
@@ -8494,7 +8496,7 @@ read_char_x_menu_prompt (Lisp_Object map,
       else if (NILP (value))
 	value = Qt;
       if (used_mouse_menu)
-	*used_mouse_menu = 1;
+	*used_mouse_menu = true;
       return value;
     }
   return Qnil ;
@@ -9067,7 +9069,7 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
 	 : (/* indec.start < t || fkey.start < t || */ keytran.start < t))
     {
       Lisp_Object key;
-      bool used_mouse_menu = 0;
+      bool used_mouse_menu = false;
 
       /* Where the last real key started.  If we need to throw away a
          key that has expanded into more than one element of keybuf
@@ -11078,6 +11080,7 @@ syms_of_keyboard (void)
   DEFSYM (Qself_insert_command, "self-insert-command");
   DEFSYM (Qforward_char, "forward-char");
   DEFSYM (Qbackward_char, "backward-char");
+  DEFSYM (Qinput_event_functions, "input-event-functions");
 
   /* Non-nil disable property on a command means do not execute it;
      call disabled-command-function's value instead.  */

Re: pre-command-hook with input methods

[Phillip Lord]

Had a bit of a nightmare building from trunk. I still do not understand
why this happens so inconsistently for me; my "main" clone failed to
build even after make bookstrap, but a clone of the clone worked fine. I
really must be doing something wrong thing.

Still I have tried adding this to pabbrev. The -off variable is just so I
test what I am seeing.

(defvar pabbrev-input-event-functions-off t)

(defun pabbrev-input-event-functions (_)
  (unless pabbrev-input-event-functions-off
    (condition-case err
        (when pabbrev-marker
          (pabbrev-delete-last-suggestion)))))

This appears to work. Using italian-postfix, with -off set to t in
scratch I get, after typing "bu" I get

b[u]u_

(where u_ is u underlined).

With it -off set to nil, I get

bu_

(i.e. a u waiting for the dead-key). At the moment, this also means that
it's never possible to complete after a "u" (or a "u`") or any character
with a dead key. That would require quite a lot more thought; I guess
that the "u" you can see on screen is not really in the buffer until the
command (and key sequence) has completed anyway. Still if someone really
cared about this, they could switch to a pre-fix method. I tried
french-prefix and that works nicely, although, even using Emacs, I still
can't actually speak French.

So, the practical upshot is, yes, it seems to work nicely!

Phil



Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I think so yes -- so long as input-event means "user input-event" or I
>> have some way of telling. Something coming in from a process, or
>> file-watch notifications need to be ignored.
>
> Could you try the patch below to see if it provides the feature
> you need?
>
>
>         Stefan
>
>
> diff --git a/etc/NEWS b/etc/NEWS
> index 1cccc31..f1b7119 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -811,6 +811,8 @@ behavior, set `diff-switches' to `-c'.
>  \f
>  * Lisp Changes in Emacs 25.1
>  
> +** New hook `input-event-functions' run whenever a user-input event is read.
> +
>  ** The default value of `load-read-function' is now `read'.
>  
>  ** New hook `pre-redisplay-functions', a bit easier to use than pre-redisplay-function.
> diff --git a/lisp/subr.el b/lisp/subr.el
> index b9a847d..df0ed42 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -1110,6 +1110,12 @@ See `event-start' for a description of the value returned."
>    "Return the multi-click count of EVENT, a click or drag event.
>  The return value is a positive integer."
>    (if (and (consp event) (integerp (nth 2 event))) (nth 2 event) 1))
> +
> +(defvar input-event-functions nil
> +  ;; BEWARE: If it looks like this is not run anywhere, it's normal:
> +  ;; this is run in keyboard.c.
> +  "Special hook run each time a user-input event is read.
> +Each function is called with one argument: the event.")
>  \f
>  ;;;; Extracting fields of the positions in an event.
>  
> diff --git a/src/keyboard.c b/src/keyboard.c
> index eb66c44..5c64199 100644
> --- a/src/keyboard.c
> +++ b/src/keyboard.c
> @@ -2454,7 +2454,7 @@ read_char (int commandflag, Lisp_Object map,
>  	  /* Also check was_disabled so last-nonmenu-event won't return
>  	     a bad value when submenus are involved.  (Bug#447)  */
>  	  && (EQ (c, Qtool_bar) || EQ (c, Qmenu_bar) || was_disabled))
> -	*used_mouse_menu = 1;
> +	*used_mouse_menu = true;
>  
>        goto reread_for_input_method;
>      }
> @@ -2995,6 +2995,8 @@ read_char (int commandflag, Lisp_Object map,
>    if (! NILP (also_record))
>      record_char (also_record);
>  
> +  Frun_hook_with_args (2, ((Lisp_Object []) {Qinput_event_functions, c}));
> +
>    /* Wipe the echo area.
>       But first, if we are about to use an input method,
>       save the echo area contents for it to refer to.  */
> @@ -3986,7 +3988,7 @@ kbd_buffer_get_event (KBOARD **kbp,
>              obj = list1 (intern ("ns-unput-working-text"));
>  	  kbd_fetch_ptr = event + 1;
>            if (used_mouse_menu)
> -            *used_mouse_menu = 1;
> +            *used_mouse_menu = true;
>          }
>  #endif
>  
> @@ -4181,13 +4183,13 @@ kbd_buffer_get_event (KBOARD **kbp,
>  		  && !EQ (event->frame_or_window, event->arg)
>  		  && (event->kind == MENU_BAR_EVENT
>  		      || event->kind == TOOL_BAR_EVENT))
> -		*used_mouse_menu = 1;
> +		*used_mouse_menu = true;
>  #endif
>  #ifdef HAVE_NS
>  	      /* Certain system events are non-key events.  */
>  	      if (used_mouse_menu
>                    && event->kind == NS_NONKEY_EVENT)
> -		*used_mouse_menu = 1;
> +		*used_mouse_menu = true;
>  #endif
>  
>  	      /* Wipe out this event, to catch bugs.  */
> @@ -8445,7 +8447,7 @@ read_char_x_menu_prompt (Lisp_Object map,
>  			 Lisp_Object prev_event, bool *used_mouse_menu)
>  {
>    if (used_mouse_menu)
> -    *used_mouse_menu = 0;
> +    *used_mouse_menu = false;
>  
>    /* Use local over global Menu maps.  */
>  
> @@ -8494,7 +8496,7 @@ read_char_x_menu_prompt (Lisp_Object map,
>        else if (NILP (value))
>  	value = Qt;
>        if (used_mouse_menu)
> -	*used_mouse_menu = 1;
> +	*used_mouse_menu = true;
>        return value;
>      }
>    return Qnil ;
> @@ -9067,7 +9069,7 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
>  	 : (/* indec.start < t || fkey.start < t || */ keytran.start < t))
>      {
>        Lisp_Object key;
> -      bool used_mouse_menu = 0;
> +      bool used_mouse_menu = false;
>  
>        /* Where the last real key started.  If we need to throw away a
>           key that has expanded into more than one element of keybuf
> @@ -11078,6 +11080,7 @@ syms_of_keyboard (void)
>    DEFSYM (Qself_insert_command, "self-insert-command");
>    DEFSYM (Qforward_char, "forward-char");
>    DEFSYM (Qbackward_char, "backward-char");
> +  DEFSYM (Qinput_event_functions, "input-event-functions");
>  
>    /* Non-nil disable property on a command means do not execute it;
>       call disabled-command-function's value instead.  */

Re: pre-command-hook with input methods

[Stefan Monnier]

> Had a bit of a nightmare building from trunk. I still do not understand
> why this happens so inconsistently for me; my "main" clone failed to
> build even after make bookstrap, but a clone of the clone worked fine.

The "original clone" was probably not completely clean, and the recent
change that removed lisp.mk required rebuilding the Makefile in a way
that's not handled by "make bootstrap".  Our makefiles have a lot of
circular dependencies (e.g. they're built from autoconf/automake files, so
the makefiles themselves need to be re-made).  I suggest you file a bug
report about it: we really should split the "bootstrap" target into two:
one that handles the Elisp-bootstrapping (where we first need to build
a "bootstrap-emacs" with ldefs-boot.el and uncompiled files and then build
the real loaddefs.el and compile the .el files to then build the real
"emacs"), and another that handles the "build machinery's own bootstrap"
(which would remove a few more files than the current "bootstrap" but
wouldn't need to remove the .elc and loaddefs.el files).

> I really must be doing something wrong thing.

No, it's not just you.  Many people have bumped into this.  I bumped
into it as well, it's just that my experience allowed me to handle it
without too much trouble.

> This appears to work.

Thanks for confirming.

> Using italian-postfix, with -off set to t in
> scratch I get, after typing "bu" I get

> b[u]u_

IIUC that's because you had "b[u]" before you typed the "u" which
then inserted "u_" without running pre-command-hook (since we don't know
yet exactly what command we're going to run).

> With it -off set to nil, I get

> bu_

> (i.e. a u waiting for the dead-key).

And here the "[u]" is gone because we ran pabbrev-input-event-functions
right after reading the "u".

> At the moment, this also means that it's never possible to complete
> after a "u" (or a "u`") or any character with a dead key.

Not sure what you mean here.  It prevents pabbrev from *displaying* the
completion, but it doesn't prevent the user from causing the completion
(assuming she guessed what the completion would be, despite not seeing
it displayed), right?

> That would require quite a lot more thought; I guess that the "u" you
> can see on screen is not really in the buffer until the command (and
> key sequence) has completed anyway.

IIRC the way quail works, the "u_" is very much inserted in the buffer
(it's not just a display-only artifact such as an overlay with an
after-string, tho it could also use such a thing), but not via a command
(e.g. self-insert-command) but via quail's internal code and that "u_"
will be erased before we (later) run pre-command-hook and then
self-insert-command.

> Still if someone really cared about this, they could switch to
> a pre-fix method.

That should suffer from the same problem, tho affecting the "accent"
chars rather then u/e/...

To solve this problem in general, I guess we could/should generalize
input-event-functions into a before-input-event-functions (aka
after-read-event-functions) and an after-input-event-functions (aka
before-read-event-functions).

So you could use after-input-event-functions instead of post-command-hook
to insert pabbrev's completion "box".

> I tried french-prefix and that works nicely,
> although, even using Emacs, I still can't actually speak French.

Hmm... sounds like a bug in the "french-prefix" method.  Oh wait, no
indeed it only lets you write French, but not speak it.  You could still
open a feature request for it, tho.

> So, the practical upshot is, yes, it seems to work nicely!

> Phil



> Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> I think so yes -- so long as input-event means "user input-event" or I
>>> have some way of telling. Something coming in from a process, or
>>> file-watch notifications need to be ignored.
>> 
>> Could you try the patch below to see if it provides the feature
>> you need?
>> 
>> 
>> Stefan
>> 
>> 
>> diff --git a/etc/NEWS b/etc/NEWS
>> index 1cccc31..f1b7119 100644
>> --- a/etc/NEWS
>> +++ b/etc/NEWS
>> @@ -811,6 +811,8 @@ behavior, set `diff-switches' to `-c'.
>> \f
>> * Lisp Changes in Emacs 25.1
>> 
>> +** New hook `input-event-functions' run whenever a user-input event is read.
>> +
>> ** The default value of `load-read-function' is now `read'.
>> 
>> ** New hook `pre-redisplay-functions', a bit easier to use than pre-redisplay-function.
>> diff --git a/lisp/subr.el b/lisp/subr.el
>> index b9a847d..df0ed42 100644
>> --- a/lisp/subr.el
>> +++ b/lisp/subr.el
>> @@ -1110,6 +1110,12 @@ See `event-start' for a description of the value returned."
>> "Return the multi-click count of EVENT, a click or drag event.
>> The return value is a positive integer."
>> (if (and (consp event) (integerp (nth 2 event))) (nth 2 event) 1))
>> +
>> +(defvar input-event-functions nil
>> +  ;; BEWARE: If it looks like this is not run anywhere, it's normal:
>> +  ;; this is run in keyboard.c.
>> +  "Special hook run each time a user-input event is read.
>> +Each function is called with one argument: the event.")
>> \f
>> ;;;; Extracting fields of the positions in an event.
>> 
>> diff --git a/src/keyboard.c b/src/keyboard.c
>> index eb66c44..5c64199 100644
>> --- a/src/keyboard.c
>> +++ b/src/keyboard.c
>> @@ -2454,7 +2454,7 @@ read_char (int commandflag, Lisp_Object map,
>> /* Also check was_disabled so last-nonmenu-event won't return
>> a bad value when submenus are involved.  (Bug#447)  */
>> && (EQ (c, Qtool_bar) || EQ (c, Qmenu_bar) || was_disabled))
>> -	*used_mouse_menu = 1;
>> +	*used_mouse_menu = true;
>> 
>> goto reread_for_input_method;
>> }
>> @@ -2995,6 +2995,8 @@ read_char (int commandflag, Lisp_Object map,
>> if (! NILP (also_record))
>> record_char (also_record);
>> 
>> +  Frun_hook_with_args (2, ((Lisp_Object []) {Qinput_event_functions, c}));
>> +
>> /* Wipe the echo area.
>> But first, if we are about to use an input method,
>> save the echo area contents for it to refer to.  */
>> @@ -3986,7 +3988,7 @@ kbd_buffer_get_event (KBOARD **kbp,
>> obj = list1 (intern ("ns-unput-working-text"));
>> kbd_fetch_ptr = event + 1;
>> if (used_mouse_menu)
>> -            *used_mouse_menu = 1;
>> +            *used_mouse_menu = true;
>> }
>> #endif
>> 
>> @@ -4181,13 +4183,13 @@ kbd_buffer_get_event (KBOARD **kbp,
>> && !EQ (event->frame_or_window, event->arg)
>> && (event->kind == MENU_BAR_EVENT
>> || event->kind == TOOL_BAR_EVENT))
>> -		*used_mouse_menu = 1;
>> +		*used_mouse_menu = true;
>> #endif
>> #ifdef HAVE_NS
>> /* Certain system events are non-key events.  */
>> if (used_mouse_menu
>> && event->kind == NS_NONKEY_EVENT)
>> -		*used_mouse_menu = 1;
>> +		*used_mouse_menu = true;
>> #endif
>> 
>> /* Wipe out this event, to catch bugs.  */
>> @@ -8445,7 +8447,7 @@ read_char_x_menu_prompt (Lisp_Object map,
>> Lisp_Object prev_event, bool *used_mouse_menu)
>> {
>> if (used_mouse_menu)
>> -    *used_mouse_menu = 0;
>> +    *used_mouse_menu = false;
>> 
>> /* Use local over global Menu maps.  */
>> 
>> @@ -8494,7 +8496,7 @@ read_char_x_menu_prompt (Lisp_Object map,
>> else if (NILP (value))
>> value = Qt;
>> if (used_mouse_menu)
>> -	*used_mouse_menu = 1;
>> +	*used_mouse_menu = true;
>> return value;
>> }
>> return Qnil ;
>> @@ -9067,7 +9069,7 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
>> : (/* indec.start < t || fkey.start < t || */ keytran.start < t))
>> {
>> Lisp_Object key;
>> -      bool used_mouse_menu = 0;
>> +      bool used_mouse_menu = false;
>> 
>> /* Where the last real key started.  If we need to throw away a
>> key that has expanded into more than one element of keybuf
>> @@ -11078,6 +11080,7 @@ syms_of_keyboard (void)
>> DEFSYM (Qself_insert_command, "self-insert-command");
>> DEFSYM (Qforward_char, "forward-char");
>> DEFSYM (Qbackward_char, "backward-char");
>> +  DEFSYM (Qinput_event_functions, "input-event-functions");
>> 
>> /* Non-nil disable property on a command means do not execute it;
>> call disabled-command-function's value instead.  */

Re: pre-command-hook with input methods

[Phillip Lord]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Had a bit of a nightmare building from trunk. I still do not understand
>> why this happens so inconsistently for me; my "main" clone failed to
>> build even after make bookstrap, but a clone of the clone worked fine.
>
> The "original clone" was probably not completely clean, and the recent
> change that removed lisp.mk required rebuilding the Makefile in a way
> that's not handled by "make bootstrap".  Our makefiles have a lot of
> circular dependencies (e.g. they're built from autoconf/automake files, so
> the makefiles themselves need to be re-made).


I know, I have been reading the mailing list. But I managed to get
several different results from slightly different copies of the source
-- one that build, on that failed to compile the lisp, and one core
dumped.

> I suggest you file a bug report about it: we really should split the
> "bootstrap" target into two: one that handles the Elisp-bootstrapping
> (where we first need to build a "bootstrap-emacs" with ldefs-boot.el
> and uncompiled files and then build the real loaddefs.el and compile
> the .el files to then build the real "emacs"), and another that
> handles the "build machinery's own bootstrap" (which would remove a
> few more files than the current "bootstrap" but wouldn't need to
> remove the .elc and loaddefs.el files).

I'd like some "build absolutely from start" clean. Emacs doesn't take
that long to build these days, and I'd value being able to start from a
clean state after switching branch.

More or less by definition, I don't have a clean bug to report. Although
I have a different one, wrt to the build, so I'll send that one in!



>> I really must be doing something wrong thing.
>
> No, it's not just you.  Many people have bumped into this.  I bumped
> into it as well, it's just that my experience allowed me to handle it
> without too much trouble.
>
>> This appears to work.
>
> Thanks for confirming.
>
>> Using italian-postfix, with -off set to t in
>> scratch I get, after typing "bu" I get
>
>> b[u]u_
>
> IIUC that's because you had "b[u]" before you typed the "u" which
> then inserted "u_" without running pre-command-hook (since we don't know
> yet exactly what command we're going to run).

Yes, exactly. And trying to use pabbrev in these circumstances is next
to impossible, since you have two "completions" on show at once. It even
affects normal typing since the cursor effectively jumps.

>> With it -off set to nil, I get
>
>> bu_
>
>> (i.e. a u waiting for the dead-key).
>
> And here the "[u]" is gone because we ran pabbrev-input-event-functions
> right after reading the "u".

Yes, which is what I wanted.


>> At the moment, this also means that it's never possible to complete
>> after a "u" (or a "u`") or any character with a dead key.
>
> Not sure what you mean here.  It prevents pabbrev from *displaying* the
> completion, but it doesn't prevent the user from causing the completion
> (assuming she guessed what the completion would be, despite not seeing
> it displayed), right?

I think with the way that I have coded it, actually, the wrong
completion would happen. So, after typing

b

we get offered

b[uffer] (ie pabbrev is offering "uffer" as a completion).

Now we type "u", so quail offers

bu_

but the "uffer" completion is still active (since we've had no
post-c-h), so that's clearly a bug.

The key point is that completion is offering in the gap between
post-command-hook and pre-command-hook which is normally idle. But when
a quail completion is offered typing "f" causes pre-command (on the u),
post-command (on the u), pre-command (on the f) and post-command (on the
f again) without any gaps.



>> That would require quite a lot more thought; I guess that the "u" you
>> can see on screen is not really in the buffer until the command (and
>> key sequence) has completed anyway.
>
> IIRC the way quail works, the "u_" is very much inserted in the buffer
> (it's not just a display-only artifact such as an overlay with an
> after-string, tho it could also use such a thing), but not via a command
> (e.g. self-insert-command) but via quail's internal code and that "u_"
> will be erased before we (later) run pre-command-hook and then
> self-insert-command.

Okay, so it might just be possible to offer the right completion -- so
after typing

"bu" we offer completions on "bu", after "bu`" we offer "bu`".

I am not sure I would go this route with pabbrev, though, simply because
pabbrev would be offering an in-buffer completion, while quail is
offering a different one in the minibuffer.


>> Still if someone really cared about this, they could switch to
>> a pre-fix method.
>
> That should suffer from the same problem, tho affecting the "accent"
> chars rather then u/e/...

Yes. Although (like most completion frameworks) pabbrev is word-centric,
so after "b`" (i.e. b followed by ` because that's what I want, not
because I am next going to type u and get an accent) there are no
completions anyway, since there is no word before point.

I am sure that there is some mode in existance where ` is word syntax,
but life is too short to worry.


> To solve this problem in general, I guess we could/should generalize
> input-event-functions into a before-input-event-functions (aka
> after-read-event-functions) and an after-input-event-functions (aka
> before-read-event-functions).
>
> So you could use after-input-event-functions instead of post-command-hook
> to insert pabbrev's completion "box".

This makes sense, but I'd really need to think about it carefully. It
sounds like a straight swap (i.e. pre/post command for
befor/after-input), but I wonder whether accepting the completion would
work as currently. If it works, though, it would potentially be cleaner,
since I've got duplicated logic on pre-command and input-event-functions
at the moment.

Is it easy to try? I guess you just add another run_hooks_with_args call
somewhere in read_char, but being honest, I am slightly intimidated by a
function like read_char, I would not want to guess where it should be
added.



>> I tried french-prefix and that works nicely,
>> although, even using Emacs, I still can't actually speak French.
>
> Hmm... sounds like a bug in the "french-prefix" method.  Oh wait, no
> indeed it only lets you write French, but not speak it.  You could still
> open a feature request for it, tho.

Nah, don't know anyone who speaks French anyway.

A` biento^t!

Phil