Re: [Emacs-diffs] master 7b31de4: Add hook for all events

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 7b31de4: Add hook for all events

Daniel Colascione-5
On 1/22/19 1:46 PM, Phillip Lord wrote:
> branch: master
> commit 7b31de4d107302ed91ce7519cd778b340a9880ee
> Author: Phillip Lord <[hidden email]>
> Commit: Phillip Lord <[hidden email]>
>
>      Add hook for all events

Why?

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 7b31de4: Add hook for all events

Phillip Lord-3
Daniel Colascione <[hidden email]> writes:

> On 1/22/19 1:46 PM, Phillip Lord wrote:
>> branch: master
>> commit 7b31de4d107302ed91ce7519cd778b340a9880ee
>> Author: Phillip Lord <[hidden email]>
>> Commit: Phillip Lord <[hidden email]>
>>
>>      Add hook for all events
>
> Why?


Finally got around to doing this 4 years after I first hit the problem.

https://lists.gnu.org/archive/html/emacs-devel/2015-05/msg00742.html

Short answer: input methods break completion provided by both company
and my own pabbrev. For example, try copying the company-mode
documentation into *scratch*, M-x text-mode, M-x company-mode, M-x
set-input-method "italian-postfix". Now type "Comple". You should see
something like:


Compl
Complements
Complete
Completing
Completion
Complex
e_[]

where e_ is e underscore and [] is the cursor and the lines in the
middle are offered completions.

The problem is that company uses pre-command-hook to remove old offered
completions. But self-insert hasn't run yet, because the user hasn't
decided whether to type "e" or "e'". With this hook, I can pick up the
intermediate keypresses.

Does it cause any problems you can foresee (other than signally a lot).

Phil




Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 7b31de4: Add hook for all events

Stefan Monnier
> Short answer: input methods break completion provided by both company
> and my own pabbrev. For example, try copying the company-mode
> documentation into *scratch*, M-x text-mode, M-x company-mode, M-x
> set-input-method "italian-postfix". Now type "Comple". You should see
> something like:
>
> Compl
> Complements
> Complete
> Completing
> Completion
> Complex
> e_[]
>
> where e_ is e underscore and [] is the cursor and the lines in the
> middle are offered completions.
>
> The problem is that company uses pre-command-hook to remove old offered
> completions. But self-insert hasn't run yet, because the user hasn't
> decided whether to type "e" or "e'". With this hook, I can pick up the
> intermediate keypresses.

So, IIUC the issue is to detect when to pop down the completions offered
by the likes of company/pabbrev/...
And the idea is that those packages can stop using pre-command-hook and
use input-event-functions.

So, IIUC now company will pop down its menu as soon as you hit a prefix
key like C-x or C-c, right?  Is that a desirable or undesirable
side-effect?

I guess the previously available alternative was to use sit-for to wait
for the next event, right (and indeed, I think input-event-functions is
a better option than sit-for)?


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 7b31de4: Add hook for all events

Phillip Lord-3
Stefan Monnier <[hidden email]> writes:
>> The problem is that company uses pre-command-hook to remove old offered
>> completions. But self-insert hasn't run yet, because the user hasn't
>> decided whether to type "e" or "e'". With this hook, I can pick up the
>> intermediate keypresses.
>
> So, IIUC the issue is to detect when to pop down the completions offered
> by the likes of company/pabbrev/...
> And the idea is that those packages can stop using pre-command-hook and
> use input-event-functions.

I'm currently using both pre-command and input-event but yes, you are
probably correct I wouldn't need pre-command any more.

Currently, I am doing this:

(defun pabbrev-input-event-functions (event)
  "Remove offering expansion from buffer on events.

This function is normally run off `input-event-functions'. It
main purpose is to remove completions during multi-keystroke
keyboard events associated with many input methods. These do not
signal a pre-command-hook because the command only completes when
they are unambiguously complete."
  (cond
   ((mouse-movement-p event) nil)
   (t
    (pabbrev-pre-command-hook))))
   
> So, IIUC now company will pop down its menu as soon as you hit a prefix
> key like C-x or C-c, right?  Is that a desirable or undesirable
> side-effect?

Yes, I think it would do that. I don't know if this is desirable or
not. I will have to try it. I expect it is a difference most people
wouldn't see.

It also has a practical impact; it is not possible to complete when an
input method is in the middle of the multi key press. So, for example,
with italian-postfix you can no longer complete on a prefix ending in
an "e" or and "i"; pressing "e" removes completion suggestions, but does
not trigger adding them because "e" could be "e`".

> I guess the previously available alternative was to use sit-for to wait
> for the next event, right (and indeed, I think input-event-functions is
> a better option than sit-for)?

Well, you should find it better; this hook is your code (I stole it from
a patch you sent in 2015). Sit-for, yes, would be a possibility, that I
hadn't thought off; you'd still need to filter the event using
`last-input-event' I guess. That would be a way of fixing company and
pabbrev in current Emacs. I will investigate.

Phil

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 7b31de4: Add hook for all events

Stefan Monnier
> Well, you should find it better; this hook is your code (I stole it from
> a patch you sent in 2015). Sit-for, yes, would be a possibility, that I
> hadn't thought off; you'd still need to filter the event using
> `last-input-event' I guess. That would be a way of fixing company and
> pabbrev in current Emacs. I will investigate.

Another way might be to use a before/after-change-function (i.e. don't
detect an input event but rather detect that the input method inserted
something into the buffer).


        Stefan

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 7b31de4: Add hook for all events

Stefan Monnier
In reply to this post by Phillip Lord-3
> It also has a practical impact; it is not possible to complete when an
> input method is in the middle of the multi key press. So, for example,
> with italian-postfix you can no longer complete on a prefix ending in
> an "e" or and "i"; pressing "e" removes completion suggestions, but does
> not trigger adding them because "e" could be "e`".

Hmm... this is a bummer (in some input methods, many/most chars are
prefixes of something else).  IIUC this is not a new problem introduced
by the use of input-event-functions, tho, right?


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 7b31de4: Add hook for all events

Phillip Lord-3
In reply to this post by Stefan Monnier
Stefan Monnier <[hidden email]> writes:

>> Well, you should find it better; this hook is your code (I stole it from
>> a patch you sent in 2015). Sit-for, yes, would be a possibility, that I
>> hadn't thought off; you'd still need to filter the event using
>> `last-input-event' I guess. That would be a way of fixing company and
>> pabbrev in current Emacs. I will investigate.
>
> Another way might be to use a before/after-change-function (i.e. don't
> detect an input event but rather detect that the input method inserted
> something into the buffer).

I haven't checked, but I always assumed that input methods do not insert
anything in the buffer until a key sequence completes; rather than
everything was visual. Perhaps not; pabbrev used to insert suggestions
into the buffer, until you fixed it.

Phil

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 7b31de4: Add hook for all events

Phillip Lord-3
In reply to this post by Stefan Monnier
Stefan Monnier <[hidden email]> writes:

>> It also has a practical impact; it is not possible to complete when an
>> input method is in the middle of the multi key press. So, for example,
>> with italian-postfix you can no longer complete on a prefix ending in
>> an "e" or and "i"; pressing "e" removes completion suggestions, but does
>> not trigger adding them because "e" could be "e`".
>
> Hmm... this is a bummer (in some input methods, many/most chars are
> prefixes of something else).  IIUC this is not a new problem introduced
> by the use of input-event-functions, tho, right?


My experience is that completion doesn't work when an input method is
half way through, although I only use simple input methods.

Anyway, I checked (should have done this before I sent the last email,
sorry). The buffer is not changed by an input method half way through
(or at least "before-change-function" is not called, which I am assuming
is the same thing). So, when the buffer appears to say "pizza_[`]", if
it does offer a completion at all, it's going to offer one based on
"pizz" because they "a" is not in the buffer.

The ideal behaviour here would be to offer completions based on either
"pizza" or "pizza`". Completion would then do the work of the input
method also. I am guessing that this is would be non trivial to add. For
sure, pabbrev and I guess company would have to work on what the user
sees, not what is in the buffer; and this is likely to be wrong nearly
as often as it is right.

Phil

Reply | Threaded
Open this post in threaded view
|

Timing of input-method output (was: [Emacs-diffs] master 7b31de4: Add hook for all events)

Stefan Monnier
>> Hmm... this is a bummer (in some input methods, many/most chars are
>> prefixes of something else).  IIUC this is not a new problem introduced
>> by the use of input-event-functions, tho, right?
> My experience is that completion doesn't work when an input method is
> half way through, although I only use simple input methods.

Right: so this is not a new problem.

> Anyway, I checked (should have done this before I sent the last email,
> sorry). The buffer is not changed by an input method half way through
> (or at least "before-change-function" is not called, which I am assuming
> is the same thing). So, when the buffer appears to say "pizza_[`]", if
> it does offer a completion at all, it's going to offer one based on
> "pizz" because they "a" is not in the buffer.

Actually, quail.el does insert the "a", but it does so only temporarily,
within a with-silent-modification, and it removes it as soon as we know
what will actually be the next result of the input method (to the user,
that's basically when the underscore is (re)moved).

IOW, the underlined text behaves more or less like it's not in the
buffer (technically it is, but it's transient and the *-change-functions
aren't notified).

For latin-based input methods (especially postfix-style ones) where it's
extremely frequent for the first char passed to an input method to end up
passed through unmodified, it would make sense to try and change the
behavior of quail so it really inserts the first char right away, even
if it may later modify it because of some subsequent user input.

E.g. currently, in latin-postfix methods, when I type

    Time0   Time1
    -----   -----
    e       n

the input looks (to the rest of Emacs) as if I had typed

    Time0   Time1
    -----   -----
            e n

this is so that when I type

    Time0   Time1
    -----   -----
    e       '

Emacs only sees

    Time0   Time1
    -----   -----
            é

but we it would make sense to try and make it behave such that an input like:

    Time0   Time1   Time2   Time3
    -----   -----   -----   -----
    e       n       e       '

results in the following (decoded) input events:

    Time0   Time1   Time2   Time3
    -----   -----   -----   -----
    e       n       e       DEL é

[ More or less: the DEL there can't be right since the user can bind it
  to anything else, so we'd have to delete the previous char "manually"
  rather than by emitting a DEL event.  ]

This would be a pretty significant change in quail.el (which is likely
to require experimentation before we can find something that works
well), and is likely to only make sense in some input methods.

I think it'd be nice for someone to play with such a quail variant
(it's probably easiest to do it be adding a config var to quail.el to
control that new behavior) to see if it can fly.


        Stefan

Reply | Threaded
Open this post in threaded view
|

Re: Timing of input-method output

Phillip Lord-3
Stefan Monnier <[hidden email]> writes:

>>> Hmm... this is a bummer (in some input methods, many/most chars are
>>> prefixes of something else).  IIUC this is not a new problem introduced
>>> by the use of input-event-functions, tho, right?
>> My experience is that completion doesn't work when an input method is
>> half way through, although I only use simple input methods.
>
> Right: so this is not a new problem.

Based on the limited evidence of me poking the one input method I use
with a stick, no.


>> Anyway, I checked (should have done this before I sent the last email,
>> sorry). The buffer is not changed by an input method half way through
>> (or at least "before-change-function" is not called, which I am assuming
>> is the same thing). So, when the buffer appears to say "pizza_[`]", if
>> it does offer a completion at all, it's going to offer one based on
>> "pizz" because they "a" is not in the buffer.
>
> Actually, quail.el does insert the "a", but it does so only temporarily,
> within a with-silent-modification, and it removes it as soon as we know
> what will actually be the next result of the input method (to the user,
> that's basically when the underscore is (re)moved).
>
> IOW, the underlined text behaves more or less like it's not in the
> buffer (technically it is, but it's transient and the *-change-functions
> aren't notified).
>
> For latin-based input methods (especially postfix-style ones) where it's
> extremely frequent for the first char passed to an input method to end up
> passed through unmodified, it would make sense to try and change the
> behavior of quail so it really inserts the first char right away, even
> if it may later modify it because of some subsequent user input.
>
> E.g. currently, in latin-postfix methods, when I type
>
>     Time0   Time1
>     -----   -----
>     e       n
>
> the input looks (to the rest of Emacs) as if I had typed
>
>     Time0   Time1
>     -----   -----
>             e n
>
> this is so that when I type
>
>     Time0   Time1
>     -----   -----
>     e       '
>
> Emacs only sees
>
>     Time0   Time1
>     -----   -----
>             é
>
> but we it would make sense to try and make it behave such that an input like:
>
>     Time0   Time1   Time2   Time3
>     -----   -----   -----   -----
>     e       n       e       '
>
> results in the following (decoded) input events:
>
>     Time0   Time1   Time2   Time3
>     -----   -----   -----   -----
>     e       n       e       DEL é
>
> [ More or less: the DEL there can't be right since the user can bind it
>   to anything else, so we'd have to delete the previous char "manually"
>   rather than by emitting a DEL event.  ]
>
> This would be a pretty significant change in quail.el (which is likely
> to require experimentation before we can find something that works
> well), and is likely to only make sense in some input methods.
>
> I think it'd be nice for someone to play with such a quail variant
> (it's probably easiest to do it be adding a config var to quail.el to
> control that new behavior) to see if it can fly.

I think that this would work more cleanly. In this case, at Time 2 we
would be offering a completion for prefix "ene" rather than "ene'" which
should then be functional with a [tab] (or whatever) at Time 3. And,
yes, that DEL would also need to be special, also not to appear in the
undo list, since we would want the events to appear to be "ene'" and not
have a DEL in the middle.

One question I would have wrt to completion is whether and how input
methods affect the visualisation of the buffer. For example, the one I
use puts an underline underneath the "e". This clearly needs to happen
at the right time so it doesn't break the visualisation that both
company and pabbrev drop into the buffer (it's the visual artifact that
made me investigate this all in the first place).

Having said all this, I am currently struggling to get the
input-event-functions to function properly for my use case -- I have got
it stopping the strange visualisation, but alas, at the cost of breaking
completion which is a baby-and-bathwater thing. The reason for this is
that I use `this-command' to work out whether to remove the completion
or not and `input-event-functions' runs before this is set.

Is there any way of knowing whether quail is currently offering a choice
of input?
`quail-is-waiting-for-another-keystroke-to-work-out-what-to-do-p'
perhaps?

Phil





Reply | Threaded
Open this post in threaded view
|

Re: Timing of input-method output

Stefan Monnier
> yes, that DEL would also need to be special, also not to appear in the
> undo list, since we would want the events to appear to be "ene'" and not
> have a DEL in the middle.

Hmm... yes, I think we'll need to play with this until we find something
good enough.

I can foresee potential interactions also with keyboard macros, so I'm
sure there'll be yet more interactions to deal with.
It's a tricky area.

But that doesn't mean it has to be hard: by making it a new package or
an option that's disabled by default we can play with it and live with
some quirks until it's polished enough to (maybe) become the new default.

> One question I would have wrt to completion is whether and how input
> methods affect the visualisation of the buffer. For example, the one I
> use puts an underline underneath the "e". This clearly needs to happen
> at the right time so it doesn't break the visualisation that both
> company and pabbrev drop into the buffer (it's the visual artifact that
> made me investigate this all in the first place).

AFAIK quail just puts an overlay over the text it has transiently
inserted and that overlay by default has the underline face.  I don't
foresee any particular troublesome interaction here.  Any reason why you
think this will be problematic?

> Is there any way of knowing whether quail is currently offering a choice
> of input?
> `quail-is-waiting-for-another-keystroke-to-work-out-what-to-do-p'
> perhaps?

You could try something like

    (defvar within-the-input-method nil)
    (add-function :around (local 'input-method-function)
                  (lambda (orig-fun &rest args)
                    (let ((within-the-input-method t))
                      (apply orig-fun args))))
                     
which in recent enough Emacsen can even be shortened to

    (defvar within-the-input-method nil)
    (add-function :around (local 'input-method-function)
                  (lambda (&rest args)
                    (let ((within-the-input-method t))
                      (apply args))))

Tho a quick'n'dirty hack might be to simply check
`inhibit-modification-hooks` which should be t when you're within Quail
(because of its use of `with-silent-modification`) and should be nil in
the normal case where the event is read by `read-key-sequence`.


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: Timing of input-method output

Phillip Lord-3
Stefan Monnier <[hidden email]> writes:

>> yes, that DEL would also need to be special, also not to appear in the
>> undo list, since we would want the events to appear to be "ene'" and not
>> have a DEL in the middle.
>
> Hmm... yes, I think we'll need to play with this until we find something
> good enough.
>
> I can foresee potential interactions also with keyboard macros, so I'm
> sure there'll be yet more interactions to deal with.
> It's a tricky area.
>
> But that doesn't mean it has to be hard: by making it a new package or
> an option that's disabled by default we can play with it and live with
> some quirks until it's polished enough to (maybe) become the new default.

Yes, I think that this all makes sense. Working exactly what to do is
likely to be far harder than how to actually do it.


>> One question I would have wrt to completion is whether and how input
>> methods affect the visualisation of the buffer. For example, the one I
>> use puts an underline underneath the "e". This clearly needs to happen
>> at the right time so it doesn't break the visualisation that both
>> company and pabbrev drop into the buffer (it's the visual artifact that
>> made me investigate this all in the first place).
>
> AFAIK quail just puts an overlay over the text it has transiently
> inserted and that overlay by default has the underline face.  I don't
> foresee any particular troublesome interaction here.  Any reason why you
> think this will be problematic?

No, it's just one of those things that breaks at the moment. If they
modification hooks get called before the overlay is placed there is a
potential problem.


>> Is there any way of knowing whether quail is currently offering a choice
>> of input?
>> `quail-is-waiting-for-another-keystroke-to-work-out-what-to-do-p'
>> perhaps?
>
> You could try something like
>
>     (defvar within-the-input-method nil)
>     (add-function :around (local 'input-method-function)
>                   (lambda (orig-fun &rest args)
>                     (let ((within-the-input-method t))
>                       (apply orig-fun args))))
>                      
> which in recent enough Emacsen can even be shortened to
>
>     (defvar within-the-input-method nil)
>     (add-function :around (local 'input-method-function)
>                   (lambda (&rest args)
>                     (let ((within-the-input-method t))
>                       (apply args))))

Surely that just tells me when the input-method-function is being run,
which is on every keypress I think?


> Tho a quick'n'dirty hack might be to simply check
> `inhibit-modification-hooks` which should be t when you're within Quail
> (because of its use of `with-silent-modification`) and should be nil in
> the normal case where the event is read by `read-key-sequence`.

Seemed like a good idea, but I think the timing is wrong. I tried adding
this to `input-event-functions'

(defun emacs-clean-input-event (event)
  (cond
   ((mouse-movement-p event) nil)
   (t
    (princ (format "event:%s i-h-k: %s i-m-f: %s\n" event inhibit-modification-hooks
                   input-method-function
                   )
           #'external-debugging-output))))

Then typed a->j with italian-postfix. The output is below (with the key
press added in brackets). I added quail-input-method because this
input-method-function because this gets set to zero when its doing it's
thing.

(a) event:97 i-h-k: nil i-m-f: quail-input-method
(b) event:98 i-h-k: t i-m-f: nil
(c) event:99 i-h-k: nil i-m-f: quail-input-method
(d) event:100 i-h-k: nil i-m-f: quail-input-method
(e) event:101 i-h-k: nil i-m-f: quail-input-method
(f) event:102 i-h-k: t i-m-f: nil
(g) event:103 i-h-k: nil i-m-f: quail-input-method
(h) event:104 i-h-k: nil i-m-f: quail-input-method
(i) event:105 i-h-k: nil i-m-f: quail-input-method
(j) event:106 i-h-k: t i-m-f: nil

As you can see, it's the keypress after which sees the
with-silent-modification.

Also tried detecting the overlay

(and
   (boundp 'quail-overlay)
   (overlayp quail-overlay)
   (overlay-start quail-overlay))

Same problem.

I'm guessing this is because of the location of the input-event-function
call. It's happening too soon, before quail has done anything to respond
to the keypress that will result in complex input happening.

Complicated business this, don't you think?

Phil

Reply | Threaded
Open this post in threaded view
|

Re: Timing of input-method output

Stefan Monnier
> I'm guessing this is because of the location of the input-event-function
> call. It's happening too soon, before quail has done anything to respond
> to the keypress that will result in complex input happening.

Oh, here's another random idea:

   in input-event-functions, start a timer to run "right away" (i.e. at
   the next occasion, which will be when we finish processing the
   current key and start waiting for the next).


-- Stefan

Reply | Threaded
Open this post in threaded view
|

Re: Timing of input-method output

Phillip Lord-3
Stefan Monnier <[hidden email]> writes:

>> I'm guessing this is because of the location of the input-event-function
>> call. It's happening too soon, before quail has done anything to respond
>> to the keypress that will result in complex input happening.
>
> Oh, here's another random idea:
>
>    in input-event-functions, start a timer to run "right away" (i.e. at
>    the next occasion, which will be when we finish processing the
>    current key and start waiting for the next).


Oh dear, that really does seem nasty to me.

I wonder if I am going about this in the wrong way. Given that this only
happens when quail is active, is it not quail that needs hooking?
"pre-quail-command-hook" or something. Quail knows that it's about to do
something weird that is not going to signal pre-command-hook because we
are still awaiting more information before the command happens.

If it works, it would mean that input-event-functions is not really fit
for the purpose I intended and should, perhaps, be removed.

Thoughts?

Phil

Reply | Threaded
Open this post in threaded view
|

Re: Timing of input-method output

Stefan Monnier
> Oh dear, that really does seem nasty to me.

;-)

> I wonder if I am going about this in the wrong way. Given that this only
> happens when quail is active, is it not quail that needs hooking?
> "pre-quail-command-hook" or something.  Quail knows that it's about to do
> something weird that is not going to signal pre-command-hook because we
> are still awaiting more information before the command happens.

Yeah, it's probably better to try and hack Quail to better cooperate.

> If it works, it would mean that input-event-functions is not really fit
> for the purpose I intended and should, perhaps, be removed.

When you added it, I thought you had already confirmed that it satisfies
your needs.  But if it doesn't, then maybe it's not a good hook.


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: Timing of input-method output

Phillip Lord-3
Stefan Monnier <[hidden email]> writes:

>> Oh dear, that really does seem nasty to me.
>
> ;-)
>
>> I wonder if I am going about this in the wrong way. Given that this only
>> happens when quail is active, is it not quail that needs hooking?
>> "pre-quail-command-hook" or something.  Quail knows that it's about to do
>> something weird that is not going to signal pre-command-hook because we
>> are still awaiting more information before the command happens.
>
> Yeah, it's probably better to try and hack Quail to better cooperate.
>
>> If it works, it would mean that input-event-functions is not really fit
>> for the purpose I intended and should, perhaps, be removed.
>
> When you added it, I thought you had already confirmed that it satisfies
> your needs.  But if it doesn't, then maybe it's not a good hook.


Well, I had checked that it did do what I wanted, in terms of stopping
the buggy completion display. I hadn't checked that it didn't break
the completion in the first place. Oh dear.

I'll remove this and see if I can fix up quail instead with a hook
there. As you say, perhaps the ultimate fix is to change quail to enter
information in a different way, but one step at a time.

Phil