input-pending-p after make-frame-visible

input-pending-p after make-frame-visible

[Aaron Jensen]

Hi all,

Unfortunately, I don't have a repro for this, but I'm putting it out
there in case anyone has any insight into it.

There is a package that makes it so that your minibuffer is rendered
into a child-frame called mini-frame:
https://github.com/muffinmad/emacs-mini-frame/

I've noticed that at some point, Emacs gets into a state where after
make-frame-visible is called with the child frame, input-pending-p
starts returning t, rather than nil:

    (message "pending: %S" (input-pending-p))
    (make-frame-visible mini-frame-frame)
    (message "pending: %S" (input-pending-p))

Prints:

nil
t

Source: https://github.com/muffinmad/emacs-mini-frame/blob/57a049b9e1ea4a9b65e82fc10c8c7e70a042f636/mini-frame.el#L313

This ordinarily wouldn't matter, but I use it with a package that uses
while-no-input and this unexpected input causes issues with that
package (https://github.com/minad/vertico/issues/115).

What could possibly cause an input to become pending when the frame is
made visible?

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

 > There is a package that makes it so that your minibuffer is rendered
 > into a child-frame called mini-frame:
 > https://github.com/muffinmad/emacs-mini-frame/
 >
 > I've noticed that at some point, Emacs gets into a state where after
 > make-frame-visible is called with the child frame, input-pending-p
 > starts returning t, rather than nil:
 >
 >      (message "pending: %S" (input-pending-p))
 >      (make-frame-visible mini-frame-frame)
 >      (message "pending: %S" (input-pending-p))
 >
 > Prints:
 >
 > nil
 > t
 >
 > Source: https://github.com/muffinmad/emacs-mini-frame/blob/57a049b9e1ea4a9b65e82fc10c8c7e70a042f636/mini-frame.el#L313
 >
 > This ordinarily wouldn't matter, but I use it with a package that uses
 > while-no-input and this unexpected input causes issues with that
 > package (https://github.com/minad/vertico/issues/115).
 >
 > What could possibly cause an input to become pending when the frame is
 > made visible?

Can you try the attached patch?  And, if possible, read the thread of
Bug#49997 to tell us whether it is related and what would be needed to
fix both issues - the one described there and yours.

Thanks, martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: while-no-input-ignore-events.diff --]
[-- Type: text/x-patch; name="while-no-input-ignore-events.diff", Size: 2907 bytes --]

diff --git a/lisp/subr.el b/lisp/subr.el
index 0a31ef2b29..f100259e9e 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4350,11 +4350,6 @@ with-local-quit
 	   ;; that intends to handle the quit signal next time.
 	   (eval '(ignore nil)))))

-;; Don't throw `throw-on-input' on those events by default.
-(setq while-no-input-ignore-events
-      '(focus-in focus-out help-echo iconify-frame
-        make-frame-visible selection-request))
-
 (defmacro while-no-input (&rest body)
   "Execute BODY only as long as there's no pending input.
 If input arrives, that ends the execution of BODY,
diff --git a/src/keyboard.c b/src/keyboard.c
index 2e4c4e6aab..48b9904d85 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2927,20 +2927,8 @@ read_char (int commandflag, Lisp_Object map,
       last_input_event = c;
       call4 (Qcommand_execute, tem, Qnil, Fvector (1, &last_input_event), Qt);

-      if (CONSP (c)
-          && (EQ (XCAR (c), Qselect_window)
-              || EQ (XCAR (c), Qfocus_out)
-#ifdef HAVE_DBUS
-	      || EQ (XCAR (c), Qdbus_event)
-#endif
-#ifdef USE_FILE_NOTIFY
-	      || EQ (XCAR (c), Qfile_notify)
-#endif
-#ifdef THREADS_ENABLED
-	      || EQ (XCAR (c), Qthread_event)
-#endif
-	      || EQ (XCAR (c), Qconfig_changed_event))
-          && !end_time)
+      if (CONSP (c) && !NILP (Fmemq (XCAR (c), Vwhile_no_input_ignore_events))
+	  && !end_time)
 	/* We stopped being idle for this event; undo that.  This
 	   prevents automatic window selection (under
 	   mouse-autoselect-window) from acting as a real input event, for
@@ -11550,6 +11538,27 @@ init_keyboard (void)
   {SYMBOL_INDEX (Qselect_window),       SYMBOL_INDEX (Qswitch_frame)}
 };

+static Lisp_Object
+init_while_no_input_ignore_events (void)
+{
+  Lisp_Object events = listn (9, Qselect_window, Qhelp_echo, Qmove_frame,
+			      Qiconify_frame, Qmake_frame_visible,
+			      Qfocus_in, Qfocus_out, Qconfig_changed_event,
+			      Qselection_request);
+
+#ifdef HAVE_DBUS
+  events = Fcons (Qdbus_event, events);
+#endif
+#ifdef USE_FILE_NOTIFY
+  events = Fcons (Qfile_notify, events);
+#endif
+#ifdef THREADS_ENABLED
+  events = Fcons (Qthread_event, events);
+#endif
+
+  return events;
+}
+
 static void syms_of_keyboard_for_pdumper (void);

 void
@@ -12444,7 +12453,12 @@ syms_of_keyboard (void)

   DEFVAR_LISP ("while-no-input-ignore-events",
                Vwhile_no_input_ignore_events,
-               doc: /* Ignored events from while-no-input.  */);
+               doc: /* Ignored events from while-no-input.
+Events in this list do not count as pending input while running
+`while-no-input' and do not cause any idle timers to get reset when they
+occur.  */);
+
+  Vwhile_no_input_ignore_events = init_while_no_input_ignore_events ();

   pdumper_do_now_and_after_load (syms_of_keyboard_for_pdumper);
 }

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Sun, Sep 26, 2021 at 5:11 AM martin rudalics <rudalics@gmx.at> wrote:
>
> Can you try the attached patch?  And, if possible, read the thread of
> Bug#49997 to tell us whether it is related and what would be needed to
> fix both issues - the one described there and yours.

I'll give the patch a shot. Does it filter events out for
input-pending-p as well? From the thread, it seems like that is the
idea, but I wanted to confirm that this particular patch intends to do
that because vertico uses both while-no-input and input-pending-p.

I don't know what else I have to offer on that thread over what is
discussed. Is there a way for me to tell what is triggering
input-pending-p to be t while I am in Emacs?

Thanks,

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > I'll give the patch a shot. Does it filter events out for
 > input-pending-p as well? From the thread, it seems like that is the
 > idea, but I wanted to confirm that this particular patch intends to do
 > that because vertico uses both while-no-input and input-pending-p.
 >
 > I don't know what else I have to offer on that thread over what is
 > discussed. Is there a way for me to tell what is triggering
 > input-pending-p to be t while I am in Emacs?

Maybe

(progn
   (while (not (input-pending-p))
     (sit-for 1))
   (message "%s" last-input-event))

can reveal something.  With emacs -Q do C-x 5 2 and put that form in
*scratch*.  In the first frame show *Messages* so you don't miss any
output.  Have a window from some other process ready that you can give
focus to since otherwise you might get a `switch-frame-event' that
intervenes - the "first" frame should _never_ get focus by the window
manager.  Now eval the form in the second frame via C-x C-e and try to
lower the second frame via the window manager and raise it again.  There
should be no message.

I cannot do that here because for some reason I don't see your problem.
But I can reproduce it when dragging the second frame with the mouse.

martin

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Sun, Sep 26, 2021 at 1:51 PM martin rudalics <rudalics@gmx.at> wrote:
>
>  > I'll give the patch a shot. Does it filter events out for
>  > input-pending-p as well? From the thread, it seems like that is the
>  > idea, but I wanted to confirm that this particular patch intends to do
>  > that because vertico uses both while-no-input and input-pending-p.
>  >
>  > I don't know what else I have to offer on that thread over what is
>  > discussed. Is there a way for me to tell what is triggering
>  > input-pending-p to be t while I am in Emacs?
>
> Maybe
>
> (progn
>    (while (not (input-pending-p))
>      (sit-for 1))
>    (message "%s" last-input-event))
>
> can reveal something.  With emacs -Q do C-x 5 2 and put that form in
> *scratch*.  In the first frame show *Messages* so you don't miss any
> output.  Have a window from some other process ready that you can give
> focus to since otherwise you might get a `switch-frame-event' that
> intervenes - the "first" frame should _never_ get focus by the window
> manager.  Now eval the form in the second frame via C-x C-e and try to
> lower the second frame via the window manager and raise it again.  There
> should be no message.
>
> I cannot do that here because for some reason I don't see your problem.
> But I can reproduce it when dragging the second frame with the mouse.

I only see my problem sometimes. It doesn't reproduce if I restart
Emacs until it does (some random amount of time later)

When I do the above, if I focus away from the 2nd frame and back I see
nothing. If I minimize it (I'm on macOS, so not sure if that is the
closest thing to lowering) then I either get a focus-out-event or
switch-frame-event because it ends up giving the focus to the 1st
frame, that's just how macOS works. Maybe I can find a place to put
last-input-event if it reproduces again.

I'm using your patch now, so I can let you know if it happens again.

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > I only see my problem sometimes. It doesn't reproduce if I restart
 > Emacs until it does (some random amount of time later)

So you have to trigger it, somehow.

 > When I do the above, if I focus away from the 2nd frame and back I see
 > nothing.

With and/or without my patch?  Please compare the behaviors.

 > If I minimize it (I'm on macOS, so not sure if that is the
 > closest thing to lowering) then I either get a focus-out-event or
 > switch-frame-event because it ends up giving the focus to the 1st
 > frame, that's just how macOS works.

That's why I told you to provide for a third window (using another Emacs
process or another application) that gets focus when you minimize the
Emacs frame.  But getting a focus-out event with the patch applied _is_
bad - these should be caught by the patch.

 > Maybe I can find a place to put
 > last-input-event if it reproduces again.
 >
 > I'm using your patch now, so I can let you know if it happens again.

You can also set 'while-no-input-ignore-events' to include or rule out
specific events.

martin

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Mon, Sep 27, 2021 at 4:51 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  > I only see my problem sometimes. It doesn't reproduce if I restart
>  > Emacs until it does (some random amount of time later)
>
> So you have to trigger it, somehow.

Right, and I have no idea how at the moment.

>  > When I do the above, if I focus away from the 2nd frame and back I see
>  > nothing.
>
> With and/or without my patch?  Please compare the behaviors.

Same w/o the patch, I don't know if application focus events are not
handled on macos, only window (frame) focus changes

>  > If I minimize it (I'm on macOS, so not sure if that is the
>  > closest thing to lowering) then I either get a focus-out-event or
>  > switch-frame-event because it ends up giving the focus to the 1st
>  > frame, that's just how macOS works.
>
> That's why I told you to provide for a third window (using another Emacs
> process or another application) that gets focus when you minimize the
> Emacs frame.  But getting a focus-out event with the patch applied _is_
> bad - these should be caught by the patch.

Right, there is a 3rd window, but that's not how macOS works. I think
I found how to do it though. If I minimize the messages frame, then
eval the form, then minimize frame that and raise it, I do get a
focus-out on that frame, with or without the patch.

>  > Maybe I can find a place to put
>  > last-input-event if it reproduces again.
>  >
>  > I'm using your patch now, so I can let you know if it happens again.
>
> You can also set 'while-no-input-ignore-events' to include or rule out
> specific events.

Ok, I'll see if I can do that once I get it reproducing again.

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > Right, there is a 3rd window, but that's not how macOS works. I think
 > I found how to do it though. If I minimize the messages frame, then
 > eval the form, then minimize frame that and raise it, I do get a
 > focus-out on that frame, with or without the patch.

Which seems to indicate that the patch doesn't handle 'input-pending-p'.
I'm probably not of much help here - maybe Eli has an idea.

martin

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 27 Sep 2021 19:14:35 +0200
> Cc: emacs-devel@gnu.org
> 
>  > Right, there is a 3rd window, but that's not how macOS works. I think
>  > I found how to do it though. If I minimize the messages frame, then
>  > eval the form, then minimize frame that and raise it, I do get a
>  > focus-out on that frame, with or without the patch.
> 
> Which seems to indicate that the patch doesn't handle 'input-pending-p'.
> I'm probably not of much help here - maybe Eli has an idea.

I don't think I understand well enough what is the problem.  It sounds
like we have no idea what kind of event is received that causes
input-pending-p return non-nil, is that right?  Then the first thing
we should do is understand which event is that, and try figuring out
why we receive that event.

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > I don't think I understand well enough what is the problem.  It sounds
 > like we have no idea what kind of event is received that causes
 > input-pending-p return non-nil, is that right?

The problem is rather whether and how we can/should make the events in
a list like

(focus-in focus-out help-echo iconify-frame make-frame-visible selection-request)

not cause 'input-pending-p' return non-nil.  I suppose we can't but
maybe there is a way.

martin

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> Cc: aaronjensen@gmail.com, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 28 Sep 2021 09:41:18 +0200
> 
>  > I don't think I understand well enough what is the problem.  It sounds
>  > like we have no idea what kind of event is received that causes
>  > input-pending-p return non-nil, is that right?
> 
> The problem is rather whether and how we can/should make the events in
> a list like
> 
> (focus-in focus-out help-echo iconify-frame make-frame-visible selection-request)
> 
> not cause 'input-pending-p' return non-nil.  I suppose we can't but
> maybe there is a way.

We could extend process_special_events, I suppose.  But are we sure
the culprit is one of these events?

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > We could extend process_special_events, I suppose.  But are we sure
 > the culprit is one of these events?

No.  Investigating 'input-pending-p' from Elisp is not trivial.  I
wonder if an event we ignore via 'while-no-input-ignore-events' may
trigger another event we do not ignore.  To put it simply: I have no
idea how processing input events is supposed to work.

martin

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> Cc: aaronjensen@gmail.com, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Wed, 29 Sep 2021 11:28:26 +0200
> 
>  > We could extend process_special_events, I suppose.  But are we sure
>  > the culprit is one of these events?
> 
> No.  Investigating 'input-pending-p' from Elisp is not trivial.

Can this be done from GDB instead?

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Tue, Sep 28, 2021 at 1:50 AM Eli Zaretskii <eliz@gnu.org> wrote:
> But where does that M-x come from in the recipe in question?

Unfortunately my recipe at the moment involves a couple third party
packages. M-x is what I hit to trigger M-x, with mini-frame enabled,
which is what produces the problem. Specifically it's something about
how mini-frame raises the child frame that causes input-pending-p to
become t

On Wed, Sep 29, 2021 at 5:28 AM martin rudalics <rudalics@gmx.at> wrote:
> Could you please try to tell in a few sentences what you did and what
> happened so maybe someone else can understand the issue and chime in.
> I'm meanwhile completely lost.

Also, unfortunately, my recipe involves using Emacs normally for a day
or so with my configuration until the issue starts to manifest itself.
I'm not the only person with this same problem, but no one else has a
repro either.

This is the code that evaluates, with make-frame-visible being the
thing that changes the value of input-pending-p:

(defun mini-frame--display (fn args)
  "Show mini-frame and call FN with ARGS."
  (let* ((selected-frame (selected-frame))
         (selected-window (selected-window))
         (selected-is-mini-frame (memq selected-frame
                                       (list mini-frame-frame
                                             mini-frame-completions-frame)))
         (dd default-directory)
         (parent-frame-parameters `((parent-frame . ,(unless
mini-frame-standalone
                                                       selected-frame))))
         (show-parameters (if (functionp mini-frame-show-parameters)
                              (funcall mini-frame-show-parameters)
                            mini-frame-show-parameters))
         (show-parameters (append (unless (alist-get 'background-color
show-parameters)
                                    `((background-color . ,(funcall
mini-frame-background-color-function))))
                                  show-parameters)))
    (if (frame-live-p mini-frame-frame)
        (unless selected-is-mini-frame
          (setq mini-frame-selected-frame selected-frame)
          (setq mini-frame-selected-window selected-window)
          (modify-frame-parameters mini-frame-frame parent-frame-parameters))
      (setq mini-frame-selected-frame selected-frame)
      (setq mini-frame-selected-window selected-window)
      (setq mini-frame-frame
            (mini-frame--make-frame (append '((minibuffer . only))
                                            parent-frame-parameters
                                            show-parameters))))
    (mini-frame--move-frame-to-frame mini-frame-frame mini-frame-selected-frame)
    (modify-frame-parameters mini-frame-frame show-parameters)
    (when (and (frame-live-p mini-frame-completions-frame)
               (frame-visible-p mini-frame-completions-frame))
      (make-frame-invisible mini-frame-completions-frame))
    (message "before: %S" (input-pending-p))
    (make-frame-visible mini-frame-frame)
    (message "after: %S" (input-pending-p))
    (redirect-frame-focus mini-frame-selected-frame mini-frame-frame)
    (select-frame-set-input-focus mini-frame-frame)
    (setq default-directory dd)
    (apply fn args)))

On Wed, Sep 29, 2021 at 8:06 AM Eli Zaretskii <eliz@gnu.org> wrote:
> Can this be done from GDB instead?

For me, it'd be lldb. Would putting a breakpoint in input-pending-p
tell us anything?

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Wed, 29 Sep 2021 08:16:32 -0400
> Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> 
> > Can this be done from GDB instead?
> 
> For me, it'd be lldb. Would putting a breakpoint in input-pending-p
> tell us anything?

You should be able to look at the last event via kbd_fetch_ptr, yes.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Wed, Sep 29, 2021 at 9:13 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Wed, 29 Sep 2021 08:16:32 -0400
> > Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> >
> > > Can this be done from GDB instead?
> >
> > For me, it'd be lldb. Would putting a breakpoint in input-pending-p
> > tell us anything?
>
> You should be able to look at the last event via kbd_fetch_ptr, yes.

Would that include non-keyboard events like focus out and the like?

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Wed, 29 Sep 2021 10:16:22 -0400
> Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> 
> On Wed, Sep 29, 2021 at 9:13 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > From: Aaron Jensen <aaronjensen@gmail.com>
> > > Date: Wed, 29 Sep 2021 08:16:32 -0400
> > > Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> > >
> > > > Can this be done from GDB instead?
> > >
> > > For me, it'd be lldb. Would putting a breakpoint in input-pending-p
> > > tell us anything?
> >
> > You should be able to look at the last event via kbd_fetch_ptr, yes.
> 
> Would that include non-keyboard events like focus out and the like?

Yes.  Emacs has only one event queue, and the "kbd_" part of its name
is misleading (it's a remnant from the original pre-GUI design).

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Wed, Sep 29, 2021 at 11:42 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> Yes.  Emacs has only one event queue, and the "kbd_" part of its name
> is misleading (it's a remnant from the original pre-GUI design).

Ok, it appears to be 19, which I believe is HELP_EVENT. Does that make sense?

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Fri, 1 Oct 2021 13:31:55 -0400
> Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> 
> On Wed, Sep 29, 2021 at 11:42 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > Yes.  Emacs has only one event queue, and the "kbd_" part of its name
> > is misleading (it's a remnant from the original pre-GUI design).
> 
> Ok, it appears to be 19, which I believe is HELP_EVENT. Does that make sense?

help-echo?  That presumes that the mouse pointer was above
mouse-sensitive text.  Was it?

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Fri, 1 Oct 2021 13:31:55 -0400
> Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> 
> On Wed, Sep 29, 2021 at 11:42 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > Yes.  Emacs has only one event queue, and the "kbd_" part of its name
> > is misleading (it's a remnant from the original pre-GUI design).
> 
> Ok, it appears to be 19, which I believe is HELP_EVENT. Does that make sense?

help-echo?  That presumes that the mouse pointer was above
mouse-sensitive text.  Was it?

But according to my reading of the code, it's TOOL_BAR_EVENT, because
2 events are specific to MS-Windows.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Fri, Oct 1, 2021 at 1:57 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Fri, 1 Oct 2021 13:31:55 -0400
> > Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> >
> > On Wed, Sep 29, 2021 at 11:42 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > >
> > > Yes.  Emacs has only one event queue, and the "kbd_" part of its name
> > > is misleading (it's a remnant from the original pre-GUI design).
> >
> > Ok, it appears to be 19, which I believe is HELP_EVENT. Does that make sense?
>
> help-echo?  That presumes that the mouse pointer was above
> mouse-sensitive text.  Was it?

That was a helpful question. I can reproduce it now. If I click on the
echo area, it opens the messages buffer. Once that has been done once,
the issue happens every time.

> But according to my reading of the code, it's TOOL_BAR_EVENT, because
> 2 events are specific to MS-Windows.

That's what I got when I counted too, but it's HELP_EVENT. I verified
by printing it.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Fri, Oct 1, 2021 at 2:25 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Fri, Oct 1, 2021 at 1:57 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > From: Aaron Jensen <aaronjensen@gmail.com>
> > > Date: Fri, 1 Oct 2021 13:31:55 -0400
> > > Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> > >
> > > On Wed, Sep 29, 2021 at 11:42 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > > >
> > > > Yes.  Emacs has only one event queue, and the "kbd_" part of its name
> > > > is misleading (it's a remnant from the original pre-GUI design).
> > >
> > > Ok, it appears to be 19, which I believe is HELP_EVENT. Does that make sense?
> >
> > help-echo?  That presumes that the mouse pointer was above
> > mouse-sensitive text.  Was it?
>
> That was a helpful question. I can reproduce it now. If I click on the
> echo area, it opens the messages buffer. Once that has been done once,
> the issue happens every time.
>
> > But according to my reading of the code, it's TOOL_BAR_EVENT, because
> > 2 events are specific to MS-Windows.
>
> That's what I got when I counted too, but it's HELP_EVENT. I verified
> by printing it.

Does anyone know why showing a mini-buffer only child frame would
trigger a help event? It doesn't when I first start emacs, but if I
click on the echo area once, it will do it from then on out. I'll see
if I can narrow in a repro.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Sun, Oct 3, 2021 at 3:33 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> Does anyone know why showing a mini-buffer only child frame would
> trigger a help event? It doesn't when I first start emacs, but if I
> click on the echo area once, it will do it from then on out. I'll see
> if I can narrow in a repro.

Ok, here is a repro:

emacs -Q

Eval:

(defvar mini-frame-frame nil)

(defun repro ()
  (interactive)
  (setq mini-frame-frame
        (or mini-frame-frame
            (make-frame `((parent-frame . ,(selected-frame))))))
  (message "input-pending-p before: %S" (input-pending-p))
  (make-frame-visible mini-frame-frame)
  (message "input-pending-p after:  %S" (input-pending-p))
  (make-frame-invisible mini-frame-frame))

(global-set-key (kbd "C-x C-x") 'repro)

Then:

Switch to messages buffer
C-x C-x
Focus the original frame
C-x C-x

Note that input-pending-p remains nil

Click the echo area

C-x C-x

Note that input-pending-p is t after showing. You have to focus the
original frame each time, but it is consistent after clicking

Re: input-pending-p after make-frame-visible

[Gregory Heytings]

>
> Ok, here is a repro:
>

I'm currently unable to reproduce the issue, but your recipe is not clear 
enough:

>
> Focus the original frame
> C-x C-x
>
> [...]
>
> Note that input-pending-p is t after showing. You have to focus the 
> original frame each time, but it is consistent after clicking
>

What is "the original frame"?  There is apparently a single (visible) 
frame in the recipe.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Sun, Oct 3, 2021 at 9:00 PM Gregory Heytings <gregory@heytings.org> wrote:
> >
> > Ok, here is a repro:
> >
>
> I'm currently unable to reproduce the issue, but your recipe is not clear
> enough:

Are you on macOS? I don't know if this is a mac specific issue or not.

Also, I just tried this with an older build that doesn't use Martin's
patch and I see pending input after C-x C-x using these steps even
without clicking in the echo area.

> >
> > Focus the original frame
> > C-x C-x
> >
> > [...]
> >
> > Note that input-pending-p is t after showing. You have to focus the
> > original frame each time, but it is consistent after clicking
> >
>
> What is "the original frame"?  There is apparently a single (visible)
> frame in the recipe.

Yes, but at least on macOS, that frame loses focus after the first C-x
C-x. It's possible that the child frame has focus instead, even though
it is invisible.

Here is a video of the repro using martin's patch: https://cln.sh/Ts1MAC

Aaron

Re: input-pending-p after make-frame-visible

[martin rudalics]

 >> What is "the original frame"?  There is apparently a single (visible)
 >> frame in the recipe.
 >
 > Yes, but at least on macOS, that frame loses focus after the first C-x
 > C-x. It's possible that the child frame has focus instead, even though
 > it is invisible.

Awkward.  Can you try calling 'frame-focus-state' on both frames to find
out whether Emacs itself has an idea of which frame has focus?

martin

Re: input-pending-p after make-frame-visible

[Gregory Heytings]

>>> What is "the original frame"?  There is apparently a single (visible) 
>>> frame in the recipe.
>>
>> Yes, but at least on macOS, that frame loses focus after the first C-x 
>> C-x. It's possible that the child frame has focus instead, even though 
>> it is invisible.
>
> Awkward.  Can you try calling 'frame-focus-state' on both frames to find 
> out whether Emacs itself has an idea of which frame has focus?
>

FTR, I can reproduce this on macOS, with Emacs 27.2.  A more detailed 
recipe is:

1. emacs -Q
2. eval-buffer the recipe
3. C-x C-x
4. click on the original frame
5. click on the echo area of the original frame

After step 3 the invisible frame has the focus and the visible frame does 
not have the focus.  For example, C-x b makes the invisible 
"mini-frame-frame" frame visible again.

Steps 3 and 4 can be repeated, they will always return input-pending-p 
{before,after} = nil.

After step 5, which creates a window with the *Messages* buffer, steps 3 
and 4 can be repeated again, they will always return input-pending-p 
before = nil, input-pending-p after = t.  This is not 100% reproductibe 
however, it's what happens most of the times.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Mon, Oct 4, 2021 at 8:20 AM Gregory Heytings <gregory@heytings.org> wrote:
> FTR, I can reproduce this on macOS, with Emacs 27.2.  A more detailed
> recipe is:

Thank you for checking and the additional details.

I added Alan to the thread in case this is a macOS specific issue.
Alan, any sense of what might cause the help-echo event to get added
back to the queue any time a child frame is shown? (I don't know if I
properly described what is happening, but hopefully that's close
enough along with the repro)

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Alan Third]

On Mon, Oct 04, 2021 at 11:00:57AM -0400, Aaron Jensen wrote:
> On Mon, Oct 4, 2021 at 8:20 AM Gregory Heytings <gregory@heytings.org> wrote:
> > FTR, I can reproduce this on macOS, with Emacs 27.2.  A more detailed
> > recipe is:
> 
> Thank you for checking and the additional details.
> 
> I added Alan to the thread in case this is a macOS specific issue.
> Alan, any sense of what might cause the help-echo event to get added
> back to the queue any time a child frame is shown? (I don't know if I
> properly described what is happening, but hopefully that's close
> enough along with the repro)

Does this do anything:

modified   src/nsterm.m
@@ -7048,6 +7048,7 @@ - (void)windowDidResignKey: (NSNotification *)notification
       XSETFRAME (frame, emacsframe);
       help_echo_string = Qnil;
       gen_help_event (Qnil, frame, Qnil, Qnil, 0);
+      any_help_event_p = NO;
     }
 
   if (emacs_event && is_focus_frame)

?
-- 
Alan Third

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Mon, Oct 4, 2021 at 4:38 PM Alan Third <alan@idiocy.org> wrote:
>
> Does this do anything:
>
> modified   src/nsterm.m
> @@ -7048,6 +7048,7 @@ - (void)windowDidResignKey: (NSNotification *)notification
>        XSETFRAME (frame, emacsframe);
>        help_echo_string = Qnil;
>        gen_help_event (Qnil, frame, Qnil, Qnil, 0);
> +      any_help_event_p = NO;
>      }
>
>    if (emacs_event && is_focus_frame)
>
> ?

Not as far as I can tell. Can you reproduce it with Martin's patch?

Aaron

Re: input-pending-p after make-frame-visible

[Alan Third]

On Mon, Oct 04, 2021 at 06:12:17PM -0400, Aaron Jensen wrote:
> On Mon, Oct 4, 2021 at 4:38 PM Alan Third <alan@idiocy.org> wrote:
> >
> > Does this do anything:
> >
> > modified   src/nsterm.m
> > @@ -7048,6 +7048,7 @@ - (void)windowDidResignKey: (NSNotification *)notification
> >        XSETFRAME (frame, emacsframe);
> >        help_echo_string = Qnil;
> >        gen_help_event (Qnil, frame, Qnil, Qnil, 0);
> > +      any_help_event_p = NO;
> >      }
> >
> >    if (emacs_event && is_focus_frame)
> >
> > ?
> 
> Not as far as I can tell. Can you reproduce it with Martin's patch?

I don't have access to a Mac just now, so no. ;)

-- 
Alan Third

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Tue, Oct 5, 2021 at 11:47 AM Alan Third <alan@idiocy.org> wrote:
> > Not as far as I can tell. Can you reproduce it with Martin's patch?
>
> I don't have access to a Mac just now, so no. ;)

Ah, that's not permanent, is it?

Any other ideas on this?

Martin, do you plan to install your patch (assuming it hasn't been
installed already)?

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Thu, Oct 14, 2021 at 7:15 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Tue, Oct 5, 2021 at 11:47 AM Alan Third <alan@idiocy.org> wrote:
> > > Not as far as I can tell. Can you reproduce it with Martin's patch?
> >
> > I don't have access to a Mac just now, so no. ;)
>
> Ah, that's not permanent, is it?
>
> Any other ideas on this?

Alan, I think I did something wrong when attempting to repro w/ your
suggested change. I just did it again and found that it partially
fixes the issue in that only reproduces on the *next* child frame open
after clicking in the echo area and not any subsequent ones. So,
something is still wrong with the logic, but you seem to have
identified the area of the problem.

Aaron

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > Martin, do you plan to install your patch (assuming it hasn't been
 > installed already)?

It's installed on master only.

martin

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Thu, Oct 14, 2021 at 8:42 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  > Martin, do you plan to install your patch (assuming it hasn't been
>  > installed already)?
>
> It's installed on master only.

Ah. Is/should help event be excluded as well by default?

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > Ah. Is/should help event be excluded as well by default?

Does it fix anything on your side when you append it to
'while-no-input-ignore-events'?  If it does, we can add it to the
default and look whether it breaks things for someone else.

Please note that all I've done in the past was to add 'move-frame' to a
predecessor of that list.  Otherwise, I have no idea about the
implications adding/removing something to/from this list could have.

martin

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Fri, Oct 15, 2021 at 3:05 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  > Ah. Is/should help event be excluded as well by default?
>
> Does it fix anything on your side when you append it to
> 'while-no-input-ignore-events'?  If it does, we can add it to the
> default and look whether it breaks things for someone else.

It's already there, but it doesn't appear to be effective with either
while-no-input or input-pending-p. Here's an updated repro:

(defvar mini-frame-frame nil)

(defun repro ()
  (interactive)
  (setq mini-frame-frame
        (or mini-frame-frame
            (make-frame `((parent-frame . ,(selected-frame))))))
  (message "input-pending-p before: %S" (input-pending-p))
  (while-no-input
    (message "NO INPUT BEFORE"))

  (make-frame-visible mini-frame-frame)

  (message "input-pending-p after: %S" (input-pending-p))
  (while-no-input
    (message "NO INPUT AFTER"))
  (make-frame-invisible mini-frame-frame))

(global-set-key (kbd "C-x C-x") 'repro)

What does the help-echo event do exactly?

Aaron

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > It's already there, but it doesn't appear to be effective with either
 > while-no-input or input-pending-p. Here's an updated repro:

You could try to report the values of 'unread-command-events',
'unread-post-input-method-events' and 'unread-input-method-events' in

(message "input-pending-p after: %S" (input-pending-p))

martin

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Sat, Oct 16, 2021 at 3:54 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  > It's already there, but it doesn't appear to be effective with either
>  > while-no-input or input-pending-p. Here's an updated repro:
>
> You could try to report the values of 'unread-command-events',
> 'unread-post-input-method-events' and 'unread-input-method-events' in
>
> (message "input-pending-p after: %S" (input-pending-p))

I know 19 (help event) is pending from a patch I have to report what is pending:

modified   src/keyboard.c
@@ -10488,9 +10488,19 @@ DEFUN ("input-pending-p", Finput_pending_p,
Sinput_pending_p, 0, 1, 0,
   /* Process non-user-visible events (Bug#10195).  */
   process_special_events ();

-  return (get_input_pending ((NILP (check_timers)
+  bool input_pending;
+
+  input_pending = get_input_pending ((NILP (check_timers)
                               ? 0 : READABLE_EVENTS_DO_TIMERS_NOW)
-      | READABLE_EVENTS_FILTER_EVENTS)
+      | READABLE_EVENTS_FILTER_EVENTS);
+
+  if (input_pending) {
+    printf ( "Input Pending event type: %d %d\n",
+      kbd_fetch_ptr->kind, HELP_EVENT);
+    fflush(stdout);
+  }
+
+  return (input_pending
    ? Qt : Qnil);

That prints:

Input Pending event type: 19 19

When I reproduce it.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Sat, Oct 16, 2021 at 10:16 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Sat, Oct 16, 2021 at 3:54 AM martin rudalics <rudalics@gmx.at> wrote:
> >
> >  > It's already there, but it doesn't appear to be effective with either
> >  > while-no-input or input-pending-p. Here's an updated repro:
> >
> > You could try to report the values of 'unread-command-events',
> > 'unread-post-input-method-events' and 'unread-input-method-events' in
> >
> > (message "input-pending-p after: %S" (input-pending-p))
>
> I know 19 (help event) is pending from a patch I have to report what is pending:

I wonder if the problem is just that `input-pending-p' does not
respect `while-no-input-ignore-events'? `while-no-input' actually uses
`input-pending-p' before it even attempts to run the body:

          (setq val (or (input-pending-p)
                 (progn ,@body)))

I think that `readable_events' needs to be modified to do exactly what
`kbd_buffer_store_buffered_event' does with regard to ignoring events
if `READABLE_EVENTS_FILTER_EVENTS' is set.

I have no idea why `USE_TOOLKIT_SCROLL_BARS' is considered when
determining whether or not to require `READABLE_EVENTS_FILTER_EVENTS'
when filtering out the focus events. I imagine that that focus event
filtering could be replaced with a check against
`while-no-input-ignore-events'

Does that make sense?

Aaron

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]

On Sat, Oct 16, 2021 at 10:45 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Sat, Oct 16, 2021 at 10:16 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
> >
> > On Sat, Oct 16, 2021 at 3:54 AM martin rudalics <rudalics@gmx.at> wrote:
> > >
> > >  > It's already there, but it doesn't appear to be effective with either
> > >  > while-no-input or input-pending-p. Here's an updated repro:
> > >
> > > You could try to report the values of 'unread-command-events',
> > > 'unread-post-input-method-events' and 'unread-input-method-events' in
> > >
> > > (message "input-pending-p after: %S" (input-pending-p))
> >
> > I know 19 (help event) is pending from a patch I have to report what is pending:
>
> I wonder if the problem is just that `input-pending-p' does not
> respect `while-no-input-ignore-events'? `while-no-input' actually uses
> `input-pending-p' before it even attempts to run the body:
>
>           (setq val (or (input-pending-p)
>                  (progn ,@body)))
>
> I think that `readable_events' needs to be modified to do exactly what
> `kbd_buffer_store_buffered_event' does with regard to ignoring events
> if `READABLE_EVENTS_FILTER_EVENTS' is set.
>
> I have no idea why `USE_TOOLKIT_SCROLL_BARS' is considered when
> determining whether or not to require `READABLE_EVENTS_FILTER_EVENTS'
> when filtering out the focus events. I imagine that that focus event
> filtering could be replaced with a check against
> `while-no-input-ignore-events'
>
> Does that make sense?

Does the attached patch seem reasonable? It fixes the issue for me.
Please review for code style and commit message style, I'm still a
neophyte with that here.

[-- Attachment #2: 0001-Ignore-events-in-input-pending-p.patch --]
[-- Type: application/octet-stream, Size: 3813 bytes --]

From a5b7edd60e4b279f6388762801c9c20f7b8631d5 Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 16 Oct 2021 11:03:50 -0400
Subject: [PATCH] Ignore events in input-pending-p

* keyboard.c (while_no_input_ignored_event): New predicate function.
  (kbd_buffer_store_buffered_event): Use `while_no_input_ignored_event'.
  (readable_events): Use `while_no_input_ignored_event' if
  `READABLE_EVENTS_FILTER_EVENTS' is set. Disregard
  `USE_TOOLKIT_SCROLL_BARS' when considering whether or not to ignore
  events as it is unclear why it was considered in the first place.
---
 src/keyboard.c | 56 +++++++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index be9fad3ac3..1e50f8ff3a 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -375,6 +375,7 @@ #define READABLE_EVENTS_IGNORE_SQUEEZABLES	(1 << 2)
 static void deliver_user_signal (int);
 static char *find_user_signal_name (int);
 static void store_user_signal_events (void);
+static bool while_no_input_ignored_event (union buffered_input_event *event);
 
 /* Advance or retreat a buffered input event pointer.  */
 
@@ -3460,12 +3461,8 @@ readable_events (int flags)
 
 	  do
 	    {
-	      if (!(
-#ifdef USE_TOOLKIT_SCROLL_BARS
-		    (flags & READABLE_EVENTS_FILTER_EVENTS) &&
-#endif
-		    (event->kind == FOCUS_IN_EVENT
-                     || event->kind == FOCUS_OUT_EVENT))
+	      if (!((flags & READABLE_EVENTS_FILTER_EVENTS) &&
+		    while_no_input_ignored_event (event))
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		  && !((flags & READABLE_EVENTS_IGNORE_SQUEEZABLES)
 		       && (event->kind == SCROLL_BAR_CLICK_EVENT
@@ -3647,29 +3644,10 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
 #endif	/* subprocesses */
     }
 
-  Lisp_Object ignore_event;
-
-  switch (event->kind)
-    {
-    case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
-    case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
-    case HELP_EVENT: ignore_event = Qhelp_echo; break;
-    case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
-    case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
-    case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
-#ifdef USE_FILE_NOTIFY
-    case FILE_NOTIFY_EVENT: ignore_event = Qfile_notify; break;
-#endif
-#ifdef HAVE_DBUS
-    case DBUS_EVENT: ignore_event = Qdbus_event; break;
-#endif
-    default: ignore_event = Qnil; break;
-    }
-
   /* If we're inside while-no-input, and this event qualifies
      as input, set quit-flag to cause an interrupt.  */
   if (!NILP (Vthrow_on_input)
-      && NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events)))
+      && !while_no_input_ignored_event (event))
     Vquit_flag = Vthrow_on_input;
 }
 
@@ -11627,6 +11605,32 @@ init_while_no_input_ignore_events (void)
   return events;
 }
 
+static bool
+while_no_input_ignored_event (union buffered_input_event *event)
+{
+  Lisp_Object ignore_event;
+
+  switch (event->kind)
+    {
+    case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
+    case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
+    case HELP_EVENT: ignore_event = Qhelp_echo; break;
+    case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
+    case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
+    case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
+#ifdef USE_FILE_NOTIFY
+    case FILE_NOTIFY_EVENT: ignore_event = Qfile_notify; break;
+#endif
+#ifdef HAVE_DBUS
+    case DBUS_EVENT: ignore_event = Qdbus_event; break;
+#endif
+    default: ignore_event = Qnil; break;
+    }
+
+
+  return !NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events));
+}
+
 static void syms_of_keyboard_for_pdumper (void);
 
 void
-- 
2.33.0

Re: input-pending-p after make-frame-visible

[martin rudalics]

> Does the attached patch seem reasonable? It fixes the issue for me.

It might be what I've been intuitively missing in my prior patch.  But
my ignorance in this matter is too great to make any useful statement.

> Please review for code style and commit message style, I'm still a
> neophyte with that here.

The only thing I can mention is to move the && to the next line in

+	      if (!((flags & READABLE_EVENTS_FILTER_EVENTS) &&
+		    while_no_input_ignored_event (event))

martin

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

On Sat, Oct 16, 2021 at 12:49 PM martin rudalics <rudalics@gmx.at> wrote:
>
> > Does the attached patch seem reasonable? It fixes the issue for me.
>
> It might be what I've been intuitively missing in my prior patch.  But
> my ignorance in this matter is too great to make any useful statement.
>
> > Please review for code style and commit message style, I'm still a
> > neophyte with that here.
>
> The only thing I can mention is to move the && to the next line in
>
> +             if (!((flags & READABLE_EVENTS_FILTER_EVENTS) &&
> +                   while_no_input_ignored_event (event))

Thanks, updated.

[-- Attachment #2: 0001-Ignore-events-in-input-pending-p.patch --]
[-- Type: application/octet-stream, Size: 3813 bytes --]

From 048fb7307fb50828657b7fe3e4ef866c0a8764ed Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 16 Oct 2021 11:03:50 -0400
Subject: [PATCH] Ignore events in input-pending-p

* keyboard.c (while_no_input_ignored_event): New predicate function.
  (kbd_buffer_store_buffered_event): Use `while_no_input_ignored_event'.
  (readable_events): Use `while_no_input_ignored_event' if
  `READABLE_EVENTS_FILTER_EVENTS' is set. Disregard
  `USE_TOOLKIT_SCROLL_BARS' when considering whether or not to ignore
  events as it is unclear why it was considered in the first place.
---
 src/keyboard.c | 56 +++++++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index be9fad3ac3..f6f9b5ded7 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -375,6 +375,7 @@ #define READABLE_EVENTS_IGNORE_SQUEEZABLES	(1 << 2)
 static void deliver_user_signal (int);
 static char *find_user_signal_name (int);
 static void store_user_signal_events (void);
+static bool while_no_input_ignored_event (union buffered_input_event *event);
 
 /* Advance or retreat a buffered input event pointer.  */
 
@@ -3460,12 +3461,8 @@ readable_events (int flags)
 
 	  do
 	    {
-	      if (!(
-#ifdef USE_TOOLKIT_SCROLL_BARS
-		    (flags & READABLE_EVENTS_FILTER_EVENTS) &&
-#endif
-		    (event->kind == FOCUS_IN_EVENT
-                     || event->kind == FOCUS_OUT_EVENT))
+	      if (!((flags & READABLE_EVENTS_FILTER_EVENTS)
+		    && while_no_input_ignored_event (event))
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		  && !((flags & READABLE_EVENTS_IGNORE_SQUEEZABLES)
 		       && (event->kind == SCROLL_BAR_CLICK_EVENT
@@ -3647,29 +3644,10 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
 #endif	/* subprocesses */
     }
 
-  Lisp_Object ignore_event;
-
-  switch (event->kind)
-    {
-    case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
-    case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
-    case HELP_EVENT: ignore_event = Qhelp_echo; break;
-    case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
-    case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
-    case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
-#ifdef USE_FILE_NOTIFY
-    case FILE_NOTIFY_EVENT: ignore_event = Qfile_notify; break;
-#endif
-#ifdef HAVE_DBUS
-    case DBUS_EVENT: ignore_event = Qdbus_event; break;
-#endif
-    default: ignore_event = Qnil; break;
-    }
-
   /* If we're inside while-no-input, and this event qualifies
      as input, set quit-flag to cause an interrupt.  */
   if (!NILP (Vthrow_on_input)
-      && NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events)))
+      && !while_no_input_ignored_event (event))
     Vquit_flag = Vthrow_on_input;
 }
 
@@ -11627,6 +11605,32 @@ init_while_no_input_ignore_events (void)
   return events;
 }
 
+static bool
+while_no_input_ignored_event (union buffered_input_event *event)
+{
+  Lisp_Object ignore_event;
+
+  switch (event->kind)
+    {
+    case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
+    case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
+    case HELP_EVENT: ignore_event = Qhelp_echo; break;
+    case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
+    case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
+    case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
+#ifdef USE_FILE_NOTIFY
+    case FILE_NOTIFY_EVENT: ignore_event = Qfile_notify; break;
+#endif
+#ifdef HAVE_DBUS
+    case DBUS_EVENT: ignore_event = Qdbus_event; break;
+#endif
+    default: ignore_event = Qnil; break;
+    }
+
+
+  return !NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events));
+}
+
 static void syms_of_keyboard_for_pdumper (void);
 
 void
-- 
2.33.0

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]

On Sat, Oct 16, 2021 at 1:14 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Sat, Oct 16, 2021 at 12:49 PM martin rudalics <rudalics@gmx.at> wrote:
> >
> > > Does the attached patch seem reasonable? It fixes the issue for me.
> >
> > It might be what I've been intuitively missing in my prior patch.  But
> > my ignorance in this matter is too great to make any useful statement.
> >
> > > Please review for code style and commit message style, I'm still a
> > > neophyte with that here.
> >
> > The only thing I can mention is to move the && to the next line in
> >
> > +             if (!((flags & READABLE_EVENTS_FILTER_EVENTS) &&
> > +                   while_no_input_ignored_event (event))
>
> Thanks, updated.

Any concerns with installing the attached?

Thanks,

Aaron

[-- Attachment #2: 0001-Ignore-events-in-input-pending-p.patch --]
[-- Type: application/x-patch, Size: 3813 bytes --]

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Wed, 20 Oct 2021 11:27:38 -0400
> Cc: Alan Third <alan@idiocy.org>, Gregory Heytings <gregory@heytings.org>, Eli Zaretskii <eliz@gnu.org>, 
> 	emacs-devel@gnu.org
> 
> Any concerns with installing the attached?

Yes: I don't think we understand what it will do, apart of fixing this
particular problem for you.  The original code included conditions
that depend on use of the toolkit scroll bars (the reasons for which
we don't understand, AFAICT) and ignored just a small group of events,
now we will/might ignore much more, and that's in the main source of
reading events.  Who knows what this will do.

I _might_ be okay with installing this on master, conditioned on some
new variable, so that if some problem reveals itself, it would be easy
to see if it's due to this change.

Thanks.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Wed, Oct 20, 2021 at 12:41 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Wed, 20 Oct 2021 11:27:38 -0400
> > Cc: Alan Third <alan@idiocy.org>, Gregory Heytings <gregory@heytings.org>, Eli Zaretskii <eliz@gnu.org>,
> >       emacs-devel@gnu.org
> >
> > Any concerns with installing the attached?
>
> Yes: I don't think we understand what it will do, apart of fixing this
> particular problem for you.

The primary thing that it does is make input-pending-p respect
while-no-input-ignore-events, which is necessary to make
while-no-input fully respect while-no-input-ignore-events because it
uses input-pending-p.

It begs the question, should while-no-input-ignore-events be renamed,
or should input-pending-p at least document that it respects
while-no-input-ignore-events? It was apparently already the intent
because input-pending-p passes READABLE_EVENTS_FILTER_EVENTS to
get_input_pending.

> The original code included conditions
> that depend on use of the toolkit scroll bars (the reasons for which
> we don't understand, AFAICT) and ignored just a small group of events,
> now we will/might ignore much more, and that's in the main source of
> reading events.  Who knows what this will do.

Yeah, that part is the biggest head scratcher to me. I wonder if
Yamamoto Mitsuharu may know why this was added this way in
354344a20a06e2052bc23a4b92ac44af1075cdbe

>
> I _might_ be okay with installing this on master, conditioned on some
> new variable, so that if some problem reveals itself, it would be easy
> to see if it's due to this change.

Okay, let's see if we get any more clarity first and then I can
introduce a variable. What would you like to call it? And are you
referring to a new ifdef check?

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Wed, 20 Oct 2021 13:15:21 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org, 
> 	YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> 
> On Wed, Oct 20, 2021 at 12:41 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > Any concerns with installing the attached?
> >
> > Yes: I don't think we understand what it will do, apart of fixing this
> > particular problem for you.
> 
> The primary thing that it does is make input-pending-p respect
> while-no-input-ignore-events, which is necessary to make
> while-no-input fully respect while-no-input-ignore-events because it
> uses input-pending-p.

Yes.  But the change you propose is in readable_events, a function
that has many more callers than just input-pending-p.  In particular,
one of its callers is kbd_buffer_get_event, which is the API through
which Emacs reads all of its input.  And now, under some
circumstances, that API will ignore some events.  That's scary, at
least for me.

> > I _might_ be okay with installing this on master, conditioned on some
> > new variable, so that if some problem reveals itself, it would be easy
> > to see if it's due to this change.
> 
> Okay, let's see if we get any more clarity first and then I can
> introduce a variable. What would you like to call it? And are you
> referring to a new ifdef check?

No, that must be a variable exposed to Lisp, so users could fiddle
with it without rebuilding.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Wed, Oct 20, 2021 at 1:40 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Wed, 20 Oct 2021 13:15:21 -0400
> > Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>,
> >       Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org,
> >       YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> >
> > On Wed, Oct 20, 2021 at 12:41 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > >
> > > > Any concerns with installing the attached?
> > >
> > > Yes: I don't think we understand what it will do, apart of fixing this
> > > particular problem for you.
> >
> > The primary thing that it does is make input-pending-p respect
> > while-no-input-ignore-events, which is necessary to make
> > while-no-input fully respect while-no-input-ignore-events because it
> > uses input-pending-p.
>
> Yes.  But the change you propose is in readable_events, a function
> that has many more callers than just input-pending-p.  In particular,
> one of its callers is kbd_buffer_get_event, which is the API through
> which Emacs reads all of its input.  And now, under some
> circumstances, that API will ignore some events.  That's scary, at
> least for me.

AFAICT kbd_buffer_get_event calls `readable_events (0)' only, which
will disregard any of the code I changed or added. The
READABLE_EVENTS_FILTER_EVENTS must be set for it to come into play.
Only input-pending-p sets that flag AFAICT.

> > > I _might_ be okay with installing this on master, conditioned on some
> > > new variable, so that if some problem reveals itself, it would be easy
> > > to see if it's due to this change.
> >
> > Okay, let's see if we get any more clarity first and then I can
> > introduce a variable. What would you like to call it? And are you
> > referring to a new ifdef check?
>
> No, that must be a variable exposed to Lisp, so users could fiddle
> with it without rebuilding.

Ah, I believe that variable exists already and can affect everything I
changed other than the reliance on USE_TOOLKIT_SCROLL_BARS:
while-no-input-ignore-events

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Wed, 20 Oct 2021 13:47:57 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org, 
> 	YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> 
> > Yes.  But the change you propose is in readable_events, a function
> > that has many more callers than just input-pending-p.  In particular,
> > one of its callers is kbd_buffer_get_event, which is the API through
> > which Emacs reads all of its input.  And now, under some
> > circumstances, that API will ignore some events.  That's scary, at
> > least for me.
> 
> AFAICT kbd_buffer_get_event calls `readable_events (0)' only, which
> will disregard any of the code I changed or added. The
> READABLE_EVENTS_FILTER_EVENTS must be set for it to come into play.
> Only input-pending-p sets that flag AFAICT.

I don't see how this helps: who will remember that no caller of
readable_events can ever use that flag without invoking this behavior
whose justification we don't understand?

> > No, that must be a variable exposed to Lisp, so users could fiddle
> > with it without rebuilding.
> 
> Ah, I believe that variable exists already and can affect everything I
> changed other than the reliance on USE_TOOLKIT_SCROLL_BARS:
> while-no-input-ignore-events

No, it must be a separate variable, because it's for people who do
have non-nil while-no-input-ignore-events.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Wed, Oct 20, 2021 at 2:24 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Wed, 20 Oct 2021 13:47:57 -0400
> > Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>,
> >       Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org,
> >       YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> >
> > > Yes.  But the change you propose is in readable_events, a function
> > > that has many more callers than just input-pending-p.  In particular,
> > > one of its callers is kbd_buffer_get_event, which is the API through
> > > which Emacs reads all of its input.  And now, under some
> > > circumstances, that API will ignore some events.  That's scary, at
> > > least for me.
> >
> > AFAICT kbd_buffer_get_event calls `readable_events (0)' only, which
> > will disregard any of the code I changed or added. The
> > READABLE_EVENTS_FILTER_EVENTS must be set for it to come into play.
> > Only input-pending-p sets that flag AFAICT.
>
> I don't see how this helps: who will remember that no caller of
> readable_events can ever use that flag without invoking this behavior
> whose justification we don't understand?

Which behavior are you saying the justification is not understood? If
it's the behavior I introduced, it should be understood as it is
understandable. It is there so that the flag
READABLE_EVENTS_FILTER_EVENTS can be actually respected instead of
partially respected.

Previously, that flag was only respected if USE_TOOLKIT_SCROLL_BARS
was set and the event was focus in or focus out. That was not the sum
total of events that we want to ignore, I believe. That is determined
by while-no-input-ignore-events.

> > > No, that must be a variable exposed to Lisp, so users could fiddle
> > > with it without rebuilding.
> >
> > Ah, I believe that variable exists already and can affect everything I
> > changed other than the reliance on USE_TOOLKIT_SCROLL_BARS:
> > while-no-input-ignore-events
>
> No, it must be a separate variable, because it's for people who do
> have non-nil while-no-input-ignore-events.

Ok, let's see if we can get to where we are comfortable without this first.

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Wed, 20 Oct 2021 14:55:05 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org, 
> 	YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> 
> On Wed, Oct 20, 2021 at 2:24 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > AFAICT kbd_buffer_get_event calls `readable_events (0)' only, which
> > > will disregard any of the code I changed or added. The
> > > READABLE_EVENTS_FILTER_EVENTS must be set for it to come into play.
> > > Only input-pending-p sets that flag AFAICT.
> >
> > I don't see how this helps: who will remember that no caller of
> > readable_events can ever use that flag without invoking this behavior
> > whose justification we don't understand?
> 
> Which behavior are you saying the justification is not understood? If
> it's the behavior I introduced, it should be understood as it is
> understandable.

We are mis-communicating.  What bothers me is that in some more or
less distant future someone will find a good reason to call
readable_events with that flag from some other place, not realizing
that doing so will get some of the events filtered out under some
subtle conditions.  Thus, the fact that this flag has currently just
one user, which just happens to be the function whose behavior you are
interested to change, doesn't help, because we'd still have a ticking
maintenance bomb.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Wed, Oct 20, 2021 at 3:03 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Wed, 20 Oct 2021 14:55:05 -0400
> > Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>,
> >       Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org,
> >       YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> >
> > On Wed, Oct 20, 2021 at 2:24 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > >
> > > > AFAICT kbd_buffer_get_event calls `readable_events (0)' only, which
> > > > will disregard any of the code I changed or added. The
> > > > READABLE_EVENTS_FILTER_EVENTS must be set for it to come into play.
> > > > Only input-pending-p sets that flag AFAICT.
> > >
> > > I don't see how this helps: who will remember that no caller of
> > > readable_events can ever use that flag without invoking this behavior
> > > whose justification we don't understand?
> >
> > Which behavior are you saying the justification is not understood? If
> > it's the behavior I introduced, it should be understood as it is
> > understandable.
>
> We are mis-communicating.  What bothers me is that in some more or
> less distant future someone will find a good reason to call
> readable_events with that flag from some other place, not realizing
> that doing so will get some of the events filtered out under some
> subtle conditions.  Thus, the fact that this flag has currently just
> one user, which just happens to be the function whose behavior you are
> interested to change, doesn't help, because we'd still have a ticking
> maintenance bomb.

Got it, so I guess the question is, what is the expected behavior of
readable_events when READABLE_EVENTS_FILTER_EVENTS is set?

Would adding another flag like
READABLE_EVENTS_FILTER_WHILE_NO_INPUT_IGNORE_EVENTS make it more clear
exactly what it is doing? I could add that w/o changing the behavior
of READABLE_EVENTS_FILTER_EVENTS at all, and remove its only usage
(which should beg the question "Should we keep that flag?"). So, maybe
I could just rename the flag?

It seems unlikely that a person would start using a flag that has a
single use without understanding what that flag does, but I'm somewhat
out of position here and I don't want to create a maintenance burden
for anyone.

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Wed, 20 Oct 2021 16:00:12 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org, 
> 	YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> 
> Got it, so I guess the question is, what is the expected behavior of
> readable_events when READABLE_EVENTS_FILTER_EVENTS is set?

Yes, and the problem is, we don't really know the answer.  Someone at
some point made it ignore some events, and also made it conditional on
whether toolkit scrollbars are or aren't using.  We don't understand
why, and we now want to add more ignored events to the list, which
will change the (partially unknown) behavior even more.

> Would adding another flag like
> READABLE_EVENTS_FILTER_WHILE_NO_INPUT_IGNORE_EVENTS make it more clear
> exactly what it is doing?

A new flag might be a good idea, indeed, but (a) I'd need to see a
patch to have a clear idea what you mean, and (b) the application of
that flag should be dependent on that variable exposed to Lisp, so
that if problems happen, we can ask users to flip the variable and see
if the problems go away.

> I could add that w/o changing the behavior
> of READABLE_EVENTS_FILTER_EVENTS at all, and remove its only usage
> (which should beg the question "Should we keep that flag?"). So, maybe
> I could just rename the flag?

No, it must be a separately controlled behavior, see above.

> It seems unlikely that a person would start using a flag that has a
> single use without understanding what that flag does

In general, yes.  But isn't that precisely what you suggest we do
here? ;-)

Re: input-pending-p after make-frame-visible

[YAMAMOTO Mitsuharu]

On Thu, 21 Oct 2021 02:15:21 +0900,
Aaron Jensen wrote:
> 
> > The original code included conditions
> > that depend on use of the toolkit scroll bars (the reasons for which
> > we don't understand, AFAICT) and ignored just a small group of events,
> > now we will/might ignore much more, and that's in the main source of
> > reading events.  Who knows what this will do.
> 
> Yeah, that part is the biggest head scratcher to me. I wonder if
> Yamamoto Mitsuharu may know why this was added this way in
> 354344a20a06e2052bc23a4b92ac44af1075cdbe

Maybe I've injected some bug with the above change?  The original
intention was to coalesce redisplays caused by successive scroll thumb
dragging events because otherwise we only see partly updated screen
when redisplay-dont-pause is nil, which used to be the default.

See https://lists.gnu.org/r/emacs-devel/2005-05/msg00297.html .

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> Date: Thu, 21 Oct 2021 15:58:39 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: Eli Zaretskii <eliz@gnu.org>,
> 	martin rudalics <rudalics@gmx.at>,
> 	Alan Third <alan@idiocy.org>,
> 	Gregory Heytings <gregory@heytings.org>,
> 	emacs-devel@gnu.org
> 
> > > The original code included conditions
> > > that depend on use of the toolkit scroll bars (the reasons for which
> > > we don't understand, AFAICT) and ignored just a small group of events,
> > > now we will/might ignore much more, and that's in the main source of
> > > reading events.  Who knows what this will do.
> > 
> > Yeah, that part is the biggest head scratcher to me. I wonder if
> > Yamamoto Mitsuharu may know why this was added this way in
> > 354344a20a06e2052bc23a4b92ac44af1075cdbe
> 
> Maybe I've injected some bug with the above change?  The original
> intention was to coalesce redisplays caused by successive scroll thumb
> dragging events because otherwise we only see partly updated screen
> when redisplay-dont-pause is nil, which used to be the default.
> 
> See https://lists.gnu.org/r/emacs-devel/2005-05/msg00297.html .

Thanks.  With this explanation, I think we do need a new flag, and we
should use it controlled by some variable exposed to Lisp.  If this
proves not to have adverse effects, we can in the future discard that
variable and make the additional filtering unconditional.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Thu, Oct 21, 2021 at 2:58 AM YAMAMOTO Mitsuharu
<mituharu@math.s.chiba-u.ac.jp> wrote:
>
> Maybe I've injected some bug with the above change?  The original
> intention was to coalesce redisplays caused by successive scroll thumb
> dragging events because otherwise we only see partly updated screen
> when redisplay-dont-pause is nil, which used to be the default.

I understand the change better now and why the `#ifdef
USE_TOOLKIT_SCROLL_BARS` was added around the `flags &
READABLE_EVENTS_FILTER_EVENTS` check. It was an optimization to avoid
an additional check because there was a previous check already done
before entering the loop. So, I don't think that a bug was introduced,
it just made the code a little more difficult to decipher. I can add
that check back into my patch in whatever its final form is.

On Thu, Oct 21, 2021 at 2:01 AM Eli Zaretskii <eliz@gnu.org> wrote:
> Yes, and the problem is, we don't really know the answer.  Someone at
> some point made it ignore some events, and also made it conditional on
> whether toolkit scrollbars are or aren't using.  We don't understand
> why, and we now want to add more ignored events to the list, which
> will change the (partially unknown) behavior even more.

It appears that FOCUS_IN_EVENT has been ignored since 2002:
a0ba8995e3c579f4c57a674b1b44d60f6a4fb82d
That introduced a bug with X focus handling, so the filtering was made
to only work for input-pending-p:
20057d5215674d9c83b79fafedf37bf36b2fd0ae

The 2nd bug fix thread is here, with key quotes below:

https://lists.gnu.org/archive/html/emacs-devel/2002-06/msg00615.html

Richard Stallman wrote:

>     The code in question ignored FOCUS_IN_EVENT when looking for pending
>     input.
>
> That was the intention.  There was a bug report that input-pending-p
> reported that there was input available, even when it was just a focus
> change.  I made this change to fix that.
>
>             So when a new frame got focus, the modeline face was not
>     changed immediately to the "active" looking face.
>
> It is right to fix that bug, but simply removing the change is not
> correct.  That would reintroduce the other bug.

And:

> I think that the input-pending-p problem needs a real fix.
> These events should be ignored for the sake of input-pending-p
> even if they are not ignored for some other purposes.
>
> Perhaps there should be two different functions for asking
> whether there is input, one for low-level Emacs purposes
> and one for high-level purposes.  Can you try fixing it that way?

So, the original impetus for the filtering code was that
input-pending-p was reporting t when only a filter change was made. In
other words, when there wasn't actually an input pending. This is the
same bug that I am reporting now, there's just a different event
involved (help-echo). The flag was intended to be used by
input-pending-p alone when added.

> > Would adding another flag like
> > READABLE_EVENTS_FILTER_WHILE_NO_INPUT_IGNORE_EVENTS make it more clear
> > exactly what it is doing?
>
> A new flag might be a good idea, indeed, but (a) I'd need to see a
> patch to have a clear idea what you mean, and (b) the application of
> that flag should be dependent on that variable exposed to Lisp, so
> that if problems happen, we can ask users to flip the variable and see
> if the problems go away.

I think that the variable would just change how
READABLE_EVENTS_FILTER_EVENTS worked, so I would not add a new flag
unless you feel that ultimately renaming it would be beneficial. If
the new variable was set to t, it would filter all events in
while-no-input-ignore-events, nil would use the current focus-in-only
filtering. Perhaps the flag should be
input-pending-p-ignores-while-no-input-ignore-events? It's a mouthful,
but it's temporary hopefully. Any other better names?

> > It seems unlikely that a person would start using a flag that has a
> > single use without understanding what that flag does
>
> In general, yes.  But isn't that precisely what you suggest we do
> here? ;-)

Hah, to be fair I understand exactly what the flag does :). I just
didn't until now understand why it came to be. Hopefully the above
context is helpful and makes everyone more comfortable with my
proposed changes (extra variable or no). Please let me know how you
would like me to proceed -- I'm happy to add the extra variable and
vary the behavior based on it for now.

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Thu, Oct 21, 2021 at 7:25 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> It appears that FOCUS_IN_EVENT has been ignored since 2002:
> a0ba8995e3c579f4c57a674b1b44d60f6a4fb82d
> That introduced a bug with X focus handling, so the filtering was made
> to only work for input-pending-p:
> 20057d5215674d9c83b79fafedf37bf36b2fd0ae

One more addition to the tale: 7cfe2dc415d0a5768f9e6800836ff6887079dc30

That started ignoring FOCUS_OUT_EVENT: "This prevents input-pending-p
returning t when one of these events
arrives, and thus obviates an instant termination of sit-for when there's no
"real" event waiting."

Just a little more precedent for changes in this area intended to
affect input-pending-p. All of it's the same, we only want
input-pending-p to return t if there is an input pending.

It does make me wonder if we should rename
while-no-input-ignore-events to something like input-ignore-events or
non-input-events. Thoughts?

Aaron

Re: input-pending-p after make-frame-visible

[Stefan Monnier]

> It does make me wonder if we should rename
> while-no-input-ignore-events to something like input-ignore-events or
> non-input-events.  Thoughts?

I like `non-input-events` whereas I don't like `input-ignore-events`
because it sounds like these are events we'll jut send to /dev/null.


        Stefan

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Thu, 21 Oct 2021 07:33:05 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org
> 
> On Thu, Oct 21, 2021 at 7:25 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
> >
> It does make me wonder if we should rename
> while-no-input-ignore-events to something like input-ignore-events or
> non-input-events. Thoughts?

I don't see why: the list is still used the same as before.  Renaming
a variable is a PITA, so it should be reserved to when we have very
good reasons, or shortly after the variable was introduced, which
isn't the case here.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Thu, Oct 21, 2021 at 9:44 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > On Thu, Oct 21, 2021 at 7:25 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
> > >
> > It does make me wonder if we should rename
> > while-no-input-ignore-events to something like input-ignore-events or
> > non-input-events. Thoughts?
>
> I don't see why: the list is still used the same as before.  Renaming
> a variable is a PITA, so it should be reserved to when we have very
> good reasons, or shortly after the variable was introduced, which
> isn't the case here.

The variable name is while-no-input-ignore-events, which implies that
it is related to while-no-input alone. In any version of the change I
am proposing, that list would also be used to ignore events within
input-pending-p, which is not while-no-input. One may find it
surprising that while-no-input-ignore-events is respected by
input-pending-p, so it seemed worth considering a name change. I'd
rather not undertake a PITA, but at the same time I find it helpful
when the names of variables are not misleading.

> I have no problem with fixing stuff inside input-pending-p.  My
> problem is that your fix is not in input-pending-p, it is in a general
> function called from many places.  So its effects are much more
> general than on input-pending-p alone.  And my concern is that the
> change which you want to make will some day affect code unrelated to
> input-pending-p, with no one the wiser.

I'm not trying to be difficult, but I just want to make sure that I am
not missing something. It seems to me that the *only* way that my
change would adversely affect anything other than input-pending-p is
if someone used the READABLE_EVENTS_FILTER_EVENTS flag in the future
without knowing what it does (and/or assuming that it does what it
used to do, which is filter only focus in and focus out). Is that what
you see as well?

Also, there have already been several changes to readable_events that
were intended to only affect input-pending-p because they were guarded
by READABLE_EVENTS_FILTER_EVENTS (or its predecessor, the
filter_events) flag, so we are choosing now to be more cautious than
we were, which is fine, again, just want to make sure I'm
understanding the reasoning.

Here's another proposal (not code yet, hopefully the explanation is enough):

Introduce a new variable, non-input-events that is initialized to the
same list that while-no-input-ignore-events is
In readable_events: Use non-input-events if it is non-nil rather than
focus in/out when READABLE_EVENTS_FILTER_EVENTS is set. Revert to the
old behavior if it is nil.
In while-no-input: Use non-input-events if it is non-nil and
while-no-input-ignore-events if it is not (alternatively, respect both
lists)
Deprecate while-no-input-ignore-events encouraging use of
non-input-events instead
Update documentation in both input-pending-p and while-no-input to
reference non-input-events

How does that seem?

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Thu, 21 Oct 2021 10:07:28 -0400
> Cc: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>, martin rudalics <rudalics@gmx.at>, 
> 	Alan Third <alan@idiocy.org>, Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org
> 
> On Thu, Oct 21, 2021 at 9:44 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > On Thu, Oct 21, 2021 at 7:25 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
> > > >
> > > It does make me wonder if we should rename
> > > while-no-input-ignore-events to something like input-ignore-events or
> > > non-input-events. Thoughts?
> >
> > I don't see why: the list is still used the same as before.  Renaming
> > a variable is a PITA, so it should be reserved to when we have very
> > good reasons, or shortly after the variable was introduced, which
> > isn't the case here.
> 
> The variable name is while-no-input-ignore-events, which implies that
> it is related to while-no-input alone. In any version of the change I
> am proposing, that list would also be used to ignore events within
> input-pending-p, which is not while-no-input. One may find it
> surprising that while-no-input-ignore-events is respected by
> input-pending-p, so it seemed worth considering a name change. I'd
> rather not undertake a PITA, but at the same time I find it helpful
> when the names of variables are not misleading.

I still don't think this justifies renaming.  Yes, it would have been
better if we used a different name, but that ship has sailed, and the
new uses aren't far enough from the old ones to justify the pain of
renaming.  (I feel like I'm repeating myself, but so do you.)

> > I have no problem with fixing stuff inside input-pending-p.  My
> > problem is that your fix is not in input-pending-p, it is in a general
> > function called from many places.  So its effects are much more
> > general than on input-pending-p alone.  And my concern is that the
> > change which you want to make will some day affect code unrelated to
> > input-pending-p, with no one the wiser.
> 
> I'm not trying to be difficult, but I just want to make sure that I am
> not missing something. It seems to me that the *only* way that my
> change would adversely affect anything other than input-pending-p is
> if someone used the READABLE_EVENTS_FILTER_EVENTS flag in the future
> without knowing what it does (and/or assuming that it does what it
> used to do, which is filter only focus in and focus out). Is that what
> you see as well?

Yes.

> Also, there have already been several changes to readable_events that
> were intended to only affect input-pending-p because they were guarded
> by READABLE_EVENTS_FILTER_EVENTS (or its predecessor, the
> filter_events) flag, so we are choosing now to be more cautious than
> we were, which is fine, again, just want to make sure I'm
> understanding the reasoning.

Yes.

> Here's another proposal (not code yet, hopefully the explanation is enough):
> 
> Introduce a new variable, non-input-events that is initialized to the
> same list that while-no-input-ignore-events is
> In readable_events: Use non-input-events if it is non-nil rather than
> focus in/out when READABLE_EVENTS_FILTER_EVENTS is set. Revert to the
> old behavior if it is nil.
> In while-no-input: Use non-input-events if it is non-nil and
> while-no-input-ignore-events if it is not (alternatively, respect both
> lists)
> Deprecate while-no-input-ignore-events encouraging use of
> non-input-events instead
> Update documentation in both input-pending-p and while-no-input to
> reference non-input-events
> 
> How does that seem?

How is this different from your original proposal, and how does this
address my concerns, which you described above and I just confirmed?
My concern is precisely that using READABLE_EVENTS_FILTER_EVENTS does
NOT tell that this flag is supposed to be used only from
input-pending-p, it just tells that the events are somehow filtered.
But I'm again repeating myself.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Thu, Oct 21, 2021 at 1:36 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> (I feel like I'm repeating myself, but so do you.)

I'm sorry about that. Negotiating meaning over email is tough
sometimes and I don't have as much context as I wish I had, hence my
probing. I appreciate your patience with me.

> > Here's another proposal (not code yet, hopefully the explanation is enough):
> >
> > Introduce a new variable, non-input-events that is initialized to the
> > same list that while-no-input-ignore-events is
> > In readable_events: Use non-input-events if it is non-nil rather than
> > focus in/out when READABLE_EVENTS_FILTER_EVENTS is set. Revert to the
> > old behavior if it is nil.
> > In while-no-input: Use non-input-events if it is non-nil and
> > while-no-input-ignore-events if it is not (alternatively, respect both
> > lists)
> > Deprecate while-no-input-ignore-events encouraging use of
> > non-input-events instead
> > Update documentation in both input-pending-p and while-no-input to
> > reference non-input-events
> >
> > How does that seem?
>
> How is this different from your original proposal, and how does this
> address my concerns, which you described above and I just confirmed?
> My concern is precisely that using READABLE_EVENTS_FILTER_EVENTS does
> NOT tell that this flag is supposed to be used only from
> input-pending-p, it just tells that the events are somehow filtered.
> But I'm again repeating myself.

Right, READABLE_EVENTS_FILTER_EVENTS does not (and should not)
indicate that it is only used from input-pending-p. The flag is named
after what it does, not its use case. What its meaning is is
ambiguous, so it is up to us to define what it is. We know what it
means in code today. We know it is only used in one place today and
why it is used there. I posit that it could eventually come to mean
"filter all events specified by while-no-input-ignore-events", but
since we want to take a cautious approach, the above suggestion is to,
rather than provide a boolean flag to enable/disable the new meaning
of READABLE_EVENTS_FILTER_EVENTS we would use a new variable that was
better named for its use case. A user who discovered problems would be
able to set the new variable to nil and get the exact behavior that
they get today. Once we feel comfortable that this approach did not
introduce any problems, we could deprecate/remove
while-no-input-ignore-events. If there is no appetite for renaming the
variable at all, I can go with some other temporary variable, perhaps
readable-events-filter-events-includes-while-no-input-ignore-events?

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Thu, 21 Oct 2021 13:46:47 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>,
>  Gregory Heytings <gregory@heytings.org>,
>  YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>, emacs-devel@gnu.org
> 
> Right, READABLE_EVENTS_FILTER_EVENTS does not (and should not)
> indicate that it is only used from input-pending-p.

Does not: agree.  Should not: why?  If we decide that this is its only
purpose, why shouldn't limit its use to that caller only?

> The flag is named after what it does, not its use case.

Well, one solution could be renaming the flag, or documenting that it
must not be used by any caller except input-pending-p.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Thu, Oct 21, 2021 at 2:04 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Thu, 21 Oct 2021 13:46:47 -0400
> > Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>,
> >  Gregory Heytings <gregory@heytings.org>,
> >  YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>, emacs-devel@gnu.org
> >
> > Right, READABLE_EVENTS_FILTER_EVENTS does not (and should not)
> > indicate that it is only used from input-pending-p.
>
> Does not: agree.  Should not: why?  If we decide that this is its only
> purpose, why shouldn't limit its use to that caller only?

It's a general principle that that I try to adhere to -- name things
for what they do, rather than their use case. We cannot imagine all of
the use cases that may come in the future and someone else may want to
use readable events with non-input events filtered. If it was named
after input-pending-p, they could either use it anyway and create
confusing code, or rename it back to what it was, or rewrite it. Also,
a reader of the code would no longer get any hints as to what the flag
does from its name. Whereas if it were named after what it did they
can always grep to find its uses -- no information is lost. If
anything, I would suggest renaming it to
READABLE_EVENTS_FILTER_NON_INPUT_EVENTS to go along with the
non-input-events variable.

I'm not here to enforce my norms onto Emacs though, I will conform
with Emacs norms.

> Well, one solution could be renaming the flag, or documenting that it
> must not be used by any caller except input-pending-p.

Documentation seems like it could be useful, would you want it to say
it can't be used by anything but input-pending-p or that it is
currently only used by it?

Does the rest of the plan seem reasonable to you?

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

[-- Attachment #1: Type: text/plain, Size: 184 bytes --]

On Thu, Oct 21, 2021 at 4:27 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
> Does the rest of the plan seem reasonable to you?

Patch attached for your consideration.

Thanks,

Aaron

[-- Attachment #2: 0001-Ignore-non-input-events-in-input-pending-p.patch --]
[-- Type: application/octet-stream, Size: 7833 bytes --]

From 5c58817e164507d57296f8a22d0d8cc4895dc864 Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 16 Oct 2021 11:03:50 -0400
Subject: [PATCH] Ignore non-input events in input-pending-p

* keyboard.c (is_non_input_event): New predicate function.
(non-input-events): New variable.
(while-no-input-ignore-events): Updated documentation.
(kbd_buffer_store_buffered_event): Use `is_non_input_event'.
(READABLE_EVENTS_FILTER_NON_INPUT_EVENTS): Renamed from
`READABLE_EVENTS_FILTER_EVENTS'
(read_char): Use `non-input-events' if non-nil.
(readable_events): Use `is_non_input_event' if `non-input-events' is
non-nil.
---
 src/keyboard.c | 97 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 63 insertions(+), 34 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index be9fad3ac3..d63c6c50ef 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -340,7 +340,7 @@ #define GROW_RAW_KEYBUF							\
 
 /* Flags for readable_events.  */
 #define READABLE_EVENTS_DO_TIMERS_NOW		(1 << 0)
-#define READABLE_EVENTS_FILTER_EVENTS		(1 << 1)
+#define READABLE_EVENTS_FILTER_NON_INPUT_EVENTS	(1 << 1)
 #define READABLE_EVENTS_IGNORE_SQUEEZABLES	(1 << 2)
 
 /* Function for init_keyboard to call with no args (if nonzero).  */
@@ -375,6 +375,7 @@ #define READABLE_EVENTS_IGNORE_SQUEEZABLES	(1 << 2)
 static void deliver_user_signal (int);
 static char *find_user_signal_name (int);
 static void store_user_signal_events (void);
+static bool is_non_input_event (union buffered_input_event *event);
 
 /* Advance or retreat a buffered input event pointer.  */
 
@@ -2940,10 +2941,14 @@ read_char (int commandflag, Lisp_Object map,
   if (!NILP (tem))
     {
       struct buffer *prev_buffer = current_buffer;
+      Lisp_Object non_input_events = Vnon_input_events;
       last_input_event = c;
       call4 (Qcommand_execute, tem, Qnil, Fvector (1, &last_input_event), Qt);
 
-      if (CONSP (c) && !NILP (Fmemq (XCAR (c), Vwhile_no_input_ignore_events))
+      if (NILP (non_input_events))
+	non_input_events = Vwhile_no_input_ignore_events;
+
+      if (CONSP (c) && !NILP (Fmemq (XCAR (c), non_input_events))
 	  && !end_time)
 	/* We stopped being idle for this event; undo that.  This
 	   prevents automatic window selection (under
@@ -3446,11 +3451,12 @@ readable_events (int flags)
   if (flags & READABLE_EVENTS_DO_TIMERS_NOW)
     timer_check ();
 
-  /* If the buffer contains only FOCUS_IN/OUT_EVENT events, and
-     READABLE_EVENTS_FILTER_EVENTS is set, report it as empty.  */
+  /* If the buffer contains only events in Vnon_input_events, or
+     FOCUS_IN/OUT_EVENT events when Vnon_input_events is nil, and
+     READABLE_EVENTS_FILTER_NON_INPUT_EVENTS is set, report it as empty.  */
   if (kbd_fetch_ptr != kbd_store_ptr)
     {
-      if (flags & (READABLE_EVENTS_FILTER_EVENTS
+      if (flags & (READABLE_EVENTS_FILTER_NON_INPUT_EVENTS
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		   | READABLE_EVENTS_IGNORE_SQUEEZABLES
 #endif
@@ -3462,10 +3468,13 @@ readable_events (int flags)
 	    {
 	      if (!(
 #ifdef USE_TOOLKIT_SCROLL_BARS
-		    (flags & READABLE_EVENTS_FILTER_EVENTS) &&
+		    (flags & READABLE_EVENTS_FILTER_NON_INPUT_EVENTS) &&
 #endif
-		    (event->kind == FOCUS_IN_EVENT
-                     || event->kind == FOCUS_OUT_EVENT))
+		    ((NILP (Vnon_input_events)
+		      && event->kind == FOCUS_IN_EVENT
+	              || event->kind == FOCUS_OUT_EVENT)
+		      || (!NILP (Vnon_input_events)
+			  && is_non_input_event (event))))
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		  && !((flags & READABLE_EVENTS_IGNORE_SQUEEZABLES)
 		       && (event->kind == SCROLL_BAR_CLICK_EVENT
@@ -3647,29 +3656,10 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
 #endif	/* subprocesses */
     }
 
-  Lisp_Object ignore_event;
-
-  switch (event->kind)
-    {
-    case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
-    case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
-    case HELP_EVENT: ignore_event = Qhelp_echo; break;
-    case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
-    case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
-    case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
-#ifdef USE_FILE_NOTIFY
-    case FILE_NOTIFY_EVENT: ignore_event = Qfile_notify; break;
-#endif
-#ifdef HAVE_DBUS
-    case DBUS_EVENT: ignore_event = Qdbus_event; break;
-#endif
-    default: ignore_event = Qnil; break;
-    }
-
   /* If we're inside while-no-input, and this event qualifies
      as input, set quit-flag to cause an interrupt.  */
   if (!NILP (Vthrow_on_input)
-      && NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events)))
+      && !is_non_input_event (event))
     Vquit_flag = Vthrow_on_input;
 }
 
@@ -10490,7 +10480,7 @@ DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0,
 
   return (get_input_pending ((NILP (check_timers)
                               ? 0 : READABLE_EVENTS_DO_TIMERS_NOW)
-			     | READABLE_EVENTS_FILTER_EVENTS)
+			     | READABLE_EVENTS_FILTER_NON_INPUT_EVENTS)
 	  ? Qt : Qnil);
 }
 
@@ -11607,7 +11597,7 @@ init_keyboard (void)
 };
 
 static Lisp_Object
-init_while_no_input_ignore_events (void)
+init_non_input_events (void)
 {
   Lisp_Object events = listn (9, Qselect_window, Qhelp_echo, Qmove_frame,
 			      Qiconify_frame, Qmake_frame_visible,
@@ -11627,6 +11617,35 @@ init_while_no_input_ignore_events (void)
   return events;
 }
 
+static bool
+is_non_input_event (union buffered_input_event *event)
+{
+  Lisp_Object lisp_event;
+  Lisp_Object non_input_events = Vnon_input_events;
+
+  if (NILP (non_input_events))
+    non_input_events = Vwhile_no_input_ignore_events;
+
+  switch (event->kind)
+    {
+    case FOCUS_IN_EVENT: lisp_event = Qfocus_in; break;
+    case FOCUS_OUT_EVENT: lisp_event = Qfocus_out; break;
+    case HELP_EVENT: lisp_event = Qhelp_echo; break;
+    case ICONIFY_EVENT: lisp_event = Qiconify_frame; break;
+    case DEICONIFY_EVENT: lisp_event = Qmake_frame_visible; break;
+    case SELECTION_REQUEST_EVENT: lisp_event = Qselection_request; break;
+#ifdef USE_FILE_NOTIFY
+    case FILE_NOTIFY_EVENT: lisp_event = Qfile_notify; break;
+#endif
+#ifdef HAVE_DBUS
+    case DBUS_EVENT: lisp_event = Qdbus_event; break;
+#endif
+    default: lisp_event = Qnil; break;
+    }
+
+  return !NILP (Fmemq (lisp_event, non_input_events));
+}
+
 static void syms_of_keyboard_for_pdumper (void);
 
 void
@@ -12519,13 +12538,23 @@ syms_of_keyboard (void)
 If nil, Emacs crashes immediately in response to fatal signals.  */);
   attempt_orderly_shutdown_on_fatal_signal = true;
 
+  DEFVAR_LISP ("non-input-events",
+               Vnon_input_events,
+               doc: /* Events ignored by input checking.
+Events in this list do not count as pending input while running
+`while-no-input' or `input-pending-p' and do not cause any idle timers to get
+reset when they occur.  Setting this to nil will cause `while-no-input' to
+respect `while-no-input-ignore-events' instead.  */
+);
+  Vnon_input_events = init_non_input_events ();
+
   DEFVAR_LISP ("while-no-input-ignore-events",
                Vwhile_no_input_ignore_events,
                doc: /* Ignored events from `while-no-input'.
-Events in this list do not count as pending input while running
-`while-no-input' and do not cause any idle timers to get reset when they
-occur.  */);
-  Vwhile_no_input_ignore_events = init_while_no_input_ignore_events ();
+If `non-input-events' is non-nil, it will be used instead. Events in this list
+do not count as pending input while running `while-no-input' and do
+not cause any idle timers to get reset when they occur.  */);
+  Vwhile_no_input_ignore_events = init_non_input_events ();
 
   DEFVAR_BOOL ("translate-upper-case-key-bindings",
                translate_upper_case_key_bindings,
-- 
2.33.0

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Thu, 21 Oct 2021 16:27:54 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>, 
> 	emacs-devel@gnu.org
> 
> > Well, one solution could be renaming the flag, or documenting that it
> > must not be used by any caller except input-pending-p.
> 
> Documentation seems like it could be useful, would you want it to say
> it can't be used by anything but input-pending-p or that it is
> currently only used by it?

The former.  Something like "this is meant to be used only by
input-pending-p and similar callers, which aren't interested in all
input events".

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

On Fri, Oct 22, 2021 at 2:10 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Thu, 21 Oct 2021 16:27:54 -0400
> > Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>,
> >       Gregory Heytings <gregory@heytings.org>, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>,
> >       emacs-devel@gnu.org
> >
> > > Well, one solution could be renaming the flag, or documenting that it
> > > must not be used by any caller except input-pending-p.
> >
> > Documentation seems like it could be useful, would you want it to say
> > it can't be used by anything but input-pending-p or that it is
> > currently only used by it?
>
> The former.  Something like "this is meant to be used only by
> input-pending-p and similar callers, which aren't interested in all
> input events".

Thanks, updated the original patch "Ignore-non-input-events..." (and
corrected some parens).

I'm also attaching a second option, "Ignore-more-events..." that is a
more minimal change. It only introduces a new variable and doesn't do
any renaming.

Thanks,

Aaron

[-- Attachment #2: 0001-Ignore-non-input-events-in-input-pending-p.patch --]
[-- Type: application/octet-stream, Size: 7947 bytes --]

From cae2e33ffc8ed7bbc61ad4a6897019bd26d5fb4d Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 16 Oct 2021 11:03:50 -0400
Subject: [PATCH] Ignore non-input events in input-pending-p

* keyboard.c (is_non_input_event): New predicate function.
(non-input-events): New variable.
(while-no-input-ignore-events): Updated documentation.
(kbd_buffer_store_buffered_event): Use `is_non_input_event'.
(READABLE_EVENTS_FILTER_NON_INPUT_EVENTS): Renamed from
`READABLE_EVENTS_FILTER_EVENTS'
(read_char): Use `non-input-events' if non-nil.
(readable_events): Use `is_non_input_event' if `non-input-events' is
non-nil.
---
 src/keyboard.c | 99 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 34 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index be9fad3ac3..1de045b78f 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -340,7 +340,7 @@ #define GROW_RAW_KEYBUF							\
 
 /* Flags for readable_events.  */
 #define READABLE_EVENTS_DO_TIMERS_NOW		(1 << 0)
-#define READABLE_EVENTS_FILTER_EVENTS		(1 << 1)
+#define READABLE_EVENTS_FILTER_NON_INPUT_EVENTS	(1 << 1)
 #define READABLE_EVENTS_IGNORE_SQUEEZABLES	(1 << 2)
 
 /* Function for init_keyboard to call with no args (if nonzero).  */
@@ -375,6 +375,7 @@ #define READABLE_EVENTS_IGNORE_SQUEEZABLES	(1 << 2)
 static void deliver_user_signal (int);
 static char *find_user_signal_name (int);
 static void store_user_signal_events (void);
+static bool is_non_input_event (union buffered_input_event *event);
 
 /* Advance or retreat a buffered input event pointer.  */
 
@@ -2940,10 +2941,14 @@ read_char (int commandflag, Lisp_Object map,
   if (!NILP (tem))
     {
       struct buffer *prev_buffer = current_buffer;
+      Lisp_Object non_input_events = Vnon_input_events;
       last_input_event = c;
       call4 (Qcommand_execute, tem, Qnil, Fvector (1, &last_input_event), Qt);
 
-      if (CONSP (c) && !NILP (Fmemq (XCAR (c), Vwhile_no_input_ignore_events))
+      if (NILP (non_input_events))
+	non_input_events = Vwhile_no_input_ignore_events;
+
+      if (CONSP (c) && !NILP (Fmemq (XCAR (c), non_input_events))
 	  && !end_time)
 	/* We stopped being idle for this event; undo that.  This
 	   prevents automatic window selection (under
@@ -3446,11 +3451,14 @@ readable_events (int flags)
   if (flags & READABLE_EVENTS_DO_TIMERS_NOW)
     timer_check ();
 
-  /* If the buffer contains only FOCUS_IN/OUT_EVENT events, and
-     READABLE_EVENTS_FILTER_EVENTS is set, report it as empty.  */
+  /* If the buffer contains only events in Vnon_input_events, or
+     FOCUS_IN/OUT_EVENT events when Vnon_input_events is nil, and
+     READABLE_EVENTS_FILTER_NON_INPUT_EVENTS is set, report it as empty. This
+     is meant to be used by input-pending-p and similar callers, which aren't
+     interested in all input events.  */
   if (kbd_fetch_ptr != kbd_store_ptr)
     {
-      if (flags & (READABLE_EVENTS_FILTER_EVENTS
+      if (flags & (READABLE_EVENTS_FILTER_NON_INPUT_EVENTS
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		   | READABLE_EVENTS_IGNORE_SQUEEZABLES
 #endif
@@ -3462,10 +3470,13 @@ readable_events (int flags)
 	    {
 	      if (!(
 #ifdef USE_TOOLKIT_SCROLL_BARS
-		    (flags & READABLE_EVENTS_FILTER_EVENTS) &&
+		    (flags & READABLE_EVENTS_FILTER_NON_INPUT_EVENTS) &&
 #endif
-		    (event->kind == FOCUS_IN_EVENT
-                     || event->kind == FOCUS_OUT_EVENT))
+		    ((NILP (Vnon_input_events)
+		      && (event->kind == FOCUS_IN_EVENT
+			  || event->kind == FOCUS_OUT_EVENT))
+		      || (!NILP (Vnon_input_events)
+			  && is_non_input_event (event))))
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		  && !((flags & READABLE_EVENTS_IGNORE_SQUEEZABLES)
 		       && (event->kind == SCROLL_BAR_CLICK_EVENT
@@ -3647,29 +3658,10 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
 #endif	/* subprocesses */
     }
 
-  Lisp_Object ignore_event;
-
-  switch (event->kind)
-    {
-    case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
-    case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
-    case HELP_EVENT: ignore_event = Qhelp_echo; break;
-    case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
-    case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
-    case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
-#ifdef USE_FILE_NOTIFY
-    case FILE_NOTIFY_EVENT: ignore_event = Qfile_notify; break;
-#endif
-#ifdef HAVE_DBUS
-    case DBUS_EVENT: ignore_event = Qdbus_event; break;
-#endif
-    default: ignore_event = Qnil; break;
-    }
-
   /* If we're inside while-no-input, and this event qualifies
      as input, set quit-flag to cause an interrupt.  */
   if (!NILP (Vthrow_on_input)
-      && NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events)))
+      && !is_non_input_event (event))
     Vquit_flag = Vthrow_on_input;
 }
 
@@ -10490,7 +10482,7 @@ DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0,
 
   return (get_input_pending ((NILP (check_timers)
                               ? 0 : READABLE_EVENTS_DO_TIMERS_NOW)
-			     | READABLE_EVENTS_FILTER_EVENTS)
+			     | READABLE_EVENTS_FILTER_NON_INPUT_EVENTS)
 	  ? Qt : Qnil);
 }
 
@@ -11607,7 +11599,7 @@ init_keyboard (void)
 };
 
 static Lisp_Object
-init_while_no_input_ignore_events (void)
+init_non_input_events (void)
 {
   Lisp_Object events = listn (9, Qselect_window, Qhelp_echo, Qmove_frame,
 			      Qiconify_frame, Qmake_frame_visible,
@@ -11627,6 +11619,35 @@ init_while_no_input_ignore_events (void)
   return events;
 }
 
+static bool
+is_non_input_event (union buffered_input_event *event)
+{
+  Lisp_Object lisp_event;
+  Lisp_Object non_input_events = Vnon_input_events;
+
+  if (NILP (non_input_events))
+    non_input_events = Vwhile_no_input_ignore_events;
+
+  switch (event->kind)
+    {
+    case FOCUS_IN_EVENT: lisp_event = Qfocus_in; break;
+    case FOCUS_OUT_EVENT: lisp_event = Qfocus_out; break;
+    case HELP_EVENT: lisp_event = Qhelp_echo; break;
+    case ICONIFY_EVENT: lisp_event = Qiconify_frame; break;
+    case DEICONIFY_EVENT: lisp_event = Qmake_frame_visible; break;
+    case SELECTION_REQUEST_EVENT: lisp_event = Qselection_request; break;
+#ifdef USE_FILE_NOTIFY
+    case FILE_NOTIFY_EVENT: lisp_event = Qfile_notify; break;
+#endif
+#ifdef HAVE_DBUS
+    case DBUS_EVENT: lisp_event = Qdbus_event; break;
+#endif
+    default: lisp_event = Qnil; break;
+    }
+
+  return !NILP (Fmemq (lisp_event, non_input_events));
+}
+
 static void syms_of_keyboard_for_pdumper (void);
 
 void
@@ -12519,13 +12540,23 @@ syms_of_keyboard (void)
 If nil, Emacs crashes immediately in response to fatal signals.  */);
   attempt_orderly_shutdown_on_fatal_signal = true;
 
+  DEFVAR_LISP ("non-input-events",
+               Vnon_input_events,
+               doc: /* Events ignored by input checking.
+Events in this list do not count as pending input while running
+`while-no-input' or `input-pending-p' and do not cause any idle timers to get
+reset when they occur.  Setting this to nil will cause `while-no-input' to
+respect `while-no-input-ignore-events' instead.  */
+);
+  Vnon_input_events = init_non_input_events ();
+
   DEFVAR_LISP ("while-no-input-ignore-events",
                Vwhile_no_input_ignore_events,
                doc: /* Ignored events from `while-no-input'.
-Events in this list do not count as pending input while running
-`while-no-input' and do not cause any idle timers to get reset when they
-occur.  */);
-  Vwhile_no_input_ignore_events = init_while_no_input_ignore_events ();
+If `non-input-events' is non-nil, it will be used instead. Events in this list
+do not count as pending input while running `while-no-input' and do
+not cause any idle timers to get reset when they occur.  */);
+  Vwhile_no_input_ignore_events = init_non_input_events ();
 
   DEFVAR_BOOL ("translate-upper-case-key-bindings",
                translate_upper_case_key_bindings,
-- 
2.33.0


[-- Attachment #3: 0001-Ignore-more-events-in-input-pending-p.patch --]
[-- Type: application/octet-stream, Size: 5805 bytes --]

From 1f6ee151079dd374e71ac98a4fef1ccb05fc9a80 Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Fri, 22 Oct 2021 09:53:24 -0400
Subject: [PATCH] Ignore more events in input-pending-p

* keyboard.c (is_while_no_input_ignored_event): New predicate function.
(input-pending-p-ignores-while-no-input-ignore-events): New variable.
(kbd_buffer_store_buffered_event): Use `is_while_no_input_ignored_event'.
(readable_events): Use `is_while_no_input_ignored_event' if
`input-pending-p-ignores-while-no-input-ignore-events' is non-nil.
Improved documentation.
---
 src/keyboard.c | 74 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 24 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index be9fad3ac3..4e0d9469ee 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -375,6 +375,7 @@ #define READABLE_EVENTS_IGNORE_SQUEEZABLES	(1 << 2)
 static void deliver_user_signal (int);
 static char *find_user_signal_name (int);
 static void store_user_signal_events (void);
+static bool is_while_no_input_ignored_event (union buffered_input_event *event);
 
 /* Advance or retreat a buffered input event pointer.  */
 
@@ -3446,8 +3447,13 @@ readable_events (int flags)
   if (flags & READABLE_EVENTS_DO_TIMERS_NOW)
     timer_check ();
 
-  /* If the buffer contains only FOCUS_IN/OUT_EVENT events, and
-     READABLE_EVENTS_FILTER_EVENTS is set, report it as empty.  */
+  /* READABLE_EVENTS_FILTER_EVENTS is meant to bu used by input-pending-p and
+     similar callers, which aren't interested in all input events.  If it is
+     set, and input-pending-p-ignores-while-no-input-ignore-events is non-nil,
+     and the buffer contains only events in while-no-input-ignore-events,
+     report it as empty. If it is set and
+     input-pending-p-ignores-while-no-input-ignore-events is nil, and the
+     buffer contains only FOCUS_IN/OUT_EVENT events, report it as empty.  */
   if (kbd_fetch_ptr != kbd_store_ptr)
     {
       if (flags & (READABLE_EVENTS_FILTER_EVENTS
@@ -3457,6 +3463,8 @@ readable_events (int flags)
 		   ))
         {
           union buffered_input_event *event = kbd_fetch_ptr;
+	  bool focus_event_only =
+	    NILP (Vinput_pending_p_ignores_while_no_input_ignore_events);
 
 	  do
 	    {
@@ -3464,8 +3472,11 @@ readable_events (int flags)
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		    (flags & READABLE_EVENTS_FILTER_EVENTS) &&
 #endif
-		    (event->kind == FOCUS_IN_EVENT
-                     || event->kind == FOCUS_OUT_EVENT))
+		    ((focus_event_only
+		      && event->kind == FOCUS_IN_EVENT
+	              || event->kind == FOCUS_OUT_EVENT)
+		      || (!focus_event_only
+			  && is_while_no_input_ignored_event (event))))
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		  && !((flags & READABLE_EVENTS_IGNORE_SQUEEZABLES)
 		       && (event->kind == SCROLL_BAR_CLICK_EVENT
@@ -3647,29 +3658,10 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
 #endif	/* subprocesses */
     }
 
-  Lisp_Object ignore_event;
-
-  switch (event->kind)
-    {
-    case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
-    case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
-    case HELP_EVENT: ignore_event = Qhelp_echo; break;
-    case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
-    case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
-    case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
-#ifdef USE_FILE_NOTIFY
-    case FILE_NOTIFY_EVENT: ignore_event = Qfile_notify; break;
-#endif
-#ifdef HAVE_DBUS
-    case DBUS_EVENT: ignore_event = Qdbus_event; break;
-#endif
-    default: ignore_event = Qnil; break;
-    }
-
   /* If we're inside while-no-input, and this event qualifies
      as input, set quit-flag to cause an interrupt.  */
   if (!NILP (Vthrow_on_input)
-      && NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events)))
+      && !is_while_no_input_ignored_event (event))
     Vquit_flag = Vthrow_on_input;
 }
 
@@ -11627,6 +11619,31 @@ init_while_no_input_ignore_events (void)
   return events;
 }
 
+static bool
+is_while_no_input_ignored_event (union buffered_input_event *event)
+{
+  Lisp_Object ignore_event;
+
+  switch (event->kind)
+    {
+    case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
+    case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
+    case HELP_EVENT: ignore_event = Qhelp_echo; break;
+    case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
+    case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
+    case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
+#ifdef USE_FILE_NOTIFY
+    case FILE_NOTIFY_EVENT: ignore_event = Qfile_notify; break;
+#endif
+#ifdef HAVE_DBUS
+    case DBUS_EVENT: ignore_event = Qdbus_event; break;
+#endif
+    default: ignore_event = Qnil; break;
+    }
+
+  return !NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events));
+}
+
 static void syms_of_keyboard_for_pdumper (void);
 
 void
@@ -12519,6 +12536,15 @@ syms_of_keyboard (void)
 If nil, Emacs crashes immediately in response to fatal signals.  */);
   attempt_orderly_shutdown_on_fatal_signal = true;
 
+  DEFVAR_BOOL ("input-pending-p-ignores-while-no-input-ignore-events",
+               Vinput_pending_p_ignores_while_no_input_ignore_events,
+               doc: /* If non-nil, `input-pending-p' and anything else that
+uses `readable_events' with the flag meant to filter events will use
+`while-no-input-ignore-events' as the list of events to filter.  This flag
+may eventually be removed once this behavior is deemed safe.  */
+);
+  Vinput_pending_p_ignores_while_no_input_ignore_events = true;
+
   DEFVAR_LISP ("while-no-input-ignore-events",
                Vwhile_no_input_ignore_events,
                doc: /* Ignored events from `while-no-input'.
-- 
2.33.0

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Fri, Oct 22, 2021 at 9:58 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> Thanks, updated the original patch "Ignore-non-input-events..." (and
> corrected some parens).
>
> I'm also attaching a second option, "Ignore-more-events..." that is a
> more minimal change. It only introduces a new variable and doesn't do
> any renaming.

Hi Eli,

Do either of the previously attached patches look like they could
work? Please let me know which you'd prefer and if you have any
feedback on them.

Thanks,

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Tue, 26 Oct 2021 09:23:47 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>, 
> 	emacs-devel@gnu.org
> 
> On Fri, Oct 22, 2021 at 9:58 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
> >
> > Thanks, updated the original patch "Ignore-non-input-events..." (and
> > corrected some parens).
> >
> > I'm also attaching a second option, "Ignore-more-events..." that is a
> > more minimal change. It only introduces a new variable and doesn't do
> > any renaming.
> 
> Hi Eli,
> 
> Do either of the previously attached patches look like they could
> work? Please let me know which you'd prefer and if you have any
> feedback on them.

I didn't forget, and will look at the patches as soon as I have enough
time.  Sorry for the delay.

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Fri, 22 Oct 2021 09:58:28 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>, 
> 	emacs-devel@gnu.org
> 
> > > Documentation seems like it could be useful, would you want it to say
> > > it can't be used by anything but input-pending-p or that it is
> > > currently only used by it?
> >
> > The former.  Something like "this is meant to be used only by
> > input-pending-p and similar callers, which aren't interested in all
> > input events".
> 
> Thanks, updated the original patch "Ignore-non-input-events..." (and
> corrected some parens).
> 
> I'm also attaching a second option, "Ignore-more-events..." that is a
> more minimal change. It only introduces a new variable and doesn't do
> any renaming.

Thanks, I've installed the minimal change, with some minor changes.
The patch didn't apply, so I applied by hand, and made some too-long
variables have shorter names while at that.  Also, a variable defined
via DEFVAR_BOOL conventionally has its C counterpart's name without
the leading "V", since it's actually a C int.

Please see if the current master solves your problem.

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Thu, Oct 28, 2021 at 11:51 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Fri, 22 Oct 2021 09:58:28 -0400
> > Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>,
> >       Gregory Heytings <gregory@heytings.org>, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>,
> >       emacs-devel@gnu.org
> >
> > > > Documentation seems like it could be useful, would you want it to say
> > > > it can't be used by anything but input-pending-p or that it is
> > > > currently only used by it?
> > >
> > > The former.  Something like "this is meant to be used only by
> > > input-pending-p and similar callers, which aren't interested in all
> > > input events".
> >
> > Thanks, updated the original patch "Ignore-non-input-events..." (and
> > corrected some parens).
> >
> > I'm also attaching a second option, "Ignore-more-events..." that is a
> > more minimal change. It only introduces a new variable and doesn't do
> > any renaming.
>
> Thanks, I've installed the minimal change, with some minor changes.
> The patch didn't apply, so I applied by hand, and made some too-long
> variables have shorter names while at that.  Also, a variable defined
> via DEFVAR_BOOL conventionally has its C counterpart's name without
> the leading "V", since it's actually a C int.
>
> Please see if the current master solves your problem.

Thank you for making these corrections and describing what was wrong.
It's odd that the patch didn't apply, but thanks for doing it by hand.
It looks like you inadvertently reversed the condition for the
variable check, so I need to set it to nil to get the fixed behavior.
This should fix it:

diff --git a/src/keyboard.c b/src/keyboard.c
index c5400023e6..3933ea84e0 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3472,10 +3472,10 @@ readable_events (int flags)
 #ifdef USE_TOOLKIT_SCROLL_BARS
      (flags & READABLE_EVENTS_FILTER_EVENTS) &&
 #endif
-     ((input_pending_p_filter_events
+     ((!input_pending_p_filter_events
        && (event->kind == FOCUS_IN_EVENT
    || event->kind == FOCUS_OUT_EVENT))
-      || (!input_pending_p_filter_events
+      || (input_pending_p_filter_events
  && is_ignored_event (event))))
 #ifdef USE_TOOLKIT_SCROLL_BARS
    && !((flags & READABLE_EVENTS_IGNORE_SQUEEZABLES)



Can you or Alan also please install this related change:

diff --git a/src/nsterm.m b/src/nsterm.m
index aa29c13eb2..f4dbe95965 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7073,6 +7073,7 @@ - (void)windowDidResignKey: (NSNotification *)notification
       XSETFRAME (frame, emacsframe);
       help_echo_string = Qnil;
       gen_help_event (Qnil, frame, Qnil, Qnil, 0);
+      any_help_event_p = NO;
     }

   if (emacs_event && is_focus_frame)

It is referenced earlier in this thread. It's probably not the best
way to handle clearing help echos in the long term, but it will help
for now. The commit message can be something like "Help echos are only
cleared once in NS port"

Thanks,

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Thu, 28 Oct 2021 14:12:17 -0400
> Cc: martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>, 
> 	emacs-devel@gnu.org
> 
> It looks like you inadvertently reversed the condition for the
> variable check, so I need to set it to nil to get the fixed behavior.
> This should fix it:

Thanks, installed.

> Can you or Alan also please install this related change:

I'll leave this to Alan.

Re: input-pending-p after make-frame-visible

[Alan Third]

On Thu, Oct 28, 2021 at 02:12:17PM -0400, Aaron Jensen wrote:
> 
> Can you or Alan also please install this related change:
> 
> diff --git a/src/nsterm.m b/src/nsterm.m
> index aa29c13eb2..f4dbe95965 100644
> --- a/src/nsterm.m
> +++ b/src/nsterm.m
> @@ -7073,6 +7073,7 @@ - (void)windowDidResignKey: (NSNotification *)notification
>        XSETFRAME (frame, emacsframe);
>        help_echo_string = Qnil;
>        gen_help_event (Qnil, frame, Qnil, Qnil, 0);
> +      any_help_event_p = NO;
>      }
> 
>    if (emacs_event && is_focus_frame)
> 
> It is referenced earlier in this thread. It's probably not the best
> way to handle clearing help echos in the long term, but it will help
> for now. The commit message can be something like "Help echos are only
> cleared once in NS port"

I've pushed this to master.

-- 
Alan Third

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Sun, Oct 31, 2021 at 6:33 AM Alan Third <alan@idiocy.org> wrote:
>
> I've pushed this to master.

Thank you Alan and Eli.

Aaron

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Thu, 21 Oct 2021 07:25:44 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>, Alan Third <alan@idiocy.org>, 
> 	Gregory Heytings <gregory@heytings.org>, emacs-devel@gnu.org
> 
> I understand the change better now and why the `#ifdef
> USE_TOOLKIT_SCROLL_BARS` was added around the `flags &
> READABLE_EVENTS_FILTER_EVENTS` check. It was an optimization to avoid
> an additional check because there was a previous check already done
> before entering the loop. So, I don't think that a bug was introduced,
> it just made the code a little more difficult to decipher. I can add
> that check back into my patch in whatever its final form is.

Whatever the original reasons, that patch has proven its utility by
fixing the original problem and not generating new ones in all the
years that passed since then.

> > I think that the input-pending-p problem needs a real fix.
> > These events should be ignored for the sake of input-pending-p
> > even if they are not ignored for some other purposes.
> >
> > Perhaps there should be two different functions for asking
> > whether there is input, one for low-level Emacs purposes
> > and one for high-level purposes.  Can you try fixing it that way?
> 
> So, the original impetus for the filtering code was that
> input-pending-p was reporting t when only a filter change was made.

I have no problem with fixing stuff inside input-pending-p.  My
problem is that your fix is not in input-pending-p, it is in a general
function called from many places.  So its effects are much more
general than on input-pending-p alone.  And my concern is that the
change which you want to make will some day affect code unrelated to
input-pending-p, with no one the wiser.

Which is why I must insist on making the change either limited to
input-pending-p, come what may, or to have a variable that controls
the change if its effect is more general.  If the change proves to not
cause any harm, in some distant future we can consider removing the
condition.

> > A new flag might be a good idea, indeed, but (a) I'd need to see a
> > patch to have a clear idea what you mean, and (b) the application of
> > that flag should be dependent on that variable exposed to Lisp, so
> > that if problems happen, we can ask users to flip the variable and see
> > if the problems go away.
> 
> I think that the variable would just change how
> READABLE_EVENTS_FILTER_EVENTS worked, so I would not add a new flag
> unless you feel that ultimately renaming it would be beneficial.

The change you propose _does_ change how READABLE_EVENTS_FILTER_EVENTS
behaves.  I want that change to be controlled by a Lisp variable.  I'm
not wedded to the specific implementation I suggested.  If you can
think of a different way to reach that goal, it's fine with me; show
me the code.

> > > It seems unlikely that a person would start using a flag that has a
> > > single use without understanding what that flag does
> >
> > In general, yes.  But isn't that precisely what you suggest we do
> > here? ;-)
> 
> Hah, to be fair I understand exactly what the flag does :).

Famous last words.

> Please let me know how you would like me to proceed -- I'm happy to
> add the extra variable and vary the behavior based on it for now.

I hope the above answers that question.

Thanks.

Re: input-pending-p after make-frame-visible

[Alan Third]

On Thu, Oct 14, 2021 at 07:15:28AM -0400, Aaron Jensen wrote:
> On Tue, Oct 5, 2021 at 11:47 AM Alan Third <alan@idiocy.org> wrote:
> > > Not as far as I can tell. Can you reproduce it with Martin's patch?
> >
> > I don't have access to a Mac just now, so no. ;)
> 
> Ah, that's not permanent, is it?

No.

> Any other ideas on this?

I have absolutely no idea what the point if that any_help_event_p
variable is. The comment that goes along with it is:

/* Non-zero means that a HELP_EVENT has been generated since Emacs
   start.  */

but why would that be helpful?

What is a HELP_EVENT anyway?
-- 
Alan Third

Re: input-pending-p after make-frame-visible

[Stefan Monnier]

> /* Non-zero means that a HELP_EVENT has been generated since Emacs
>    start.  */
>
> but why would that be helpful?

It seems to be a hack to avoid clearing some help-echo message related
to the startup screen.

> What is a HELP_EVENT anyway?

It's the event generated when the mouse hovers above text with the
`help-echo` property.  In response to this event with typically popup
a tooltip, tho it can also be a message in the echo area.


        Stefan

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Fri, Oct 15, 2021 at 1:32 PM Alan Third <alan@idiocy.org> wrote:
>
> > Any other ideas on this?
>
> I have absolutely no idea what the point if that any_help_event_p
> variable is. The comment that goes along with it is:
>
> /* Non-zero means that a HELP_EVENT has been generated since Emacs
>    start.  */
>
> but why would that be helpful?

From the commit that introduced it, it was a performance improvement.
I think it would write an event on every mouse move instead just when
it was needed. After the change, if a help echo was shown then it
would set a flag so that when the NSWindow lost focus it would clear
it. Your change was good (to reset the flag in that case) and should
probably be installed but it wasn't sufficient to solve my original
problem.

Aaron

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > Ok, here is a repro:
 >
 > emacs -Q
 >
 > Eval:
 >
 > (defvar mini-frame-frame nil)
 >
 > (defun repro ()
 >    (interactive)
 >    (setq mini-frame-frame
 >          (or mini-frame-frame
 >              (make-frame `((parent-frame . ,(selected-frame))))))
 >    (message "input-pending-p before: %S" (input-pending-p))
 >    (make-frame-visible mini-frame-frame)
 >    (message "input-pending-p after:  %S" (input-pending-p))
 >    (make-frame-invisible mini-frame-frame))
 >
 > (global-set-key (kbd "C-x C-x") 'repro)

So IIUC 'mini-frame-frame' is always invisible in this scenario.

 > Then:
 >
 > Switch to messages buffer
 > C-x C-x
 > Focus the original frame

There is nothing else to focus here.

 > C-x C-x
 >
 > Note that input-pending-p remains nil
 >
 > Click the echo area

Is this "click" supposed to pop up the *Messages* buffer?

 > C-x C-x
 >
 > Note that input-pending-p is t after showing. You have to focus the
 > original frame each time, but it is consistent after clicking

Not here.  But as I said above the original frame always has focus here.
I get

input-pending-p before: t
input-pending-p after:  t
input-pending-p before: nil
input-pending-p after:  nil
input-pending-p before: nil
input-pending-p after:  nil

with the *Messages* buffer focused before the second C-x C-x and the
same if I switch back to *scratch* where you say "Focus the original
frame".

martin

Re: input-pending-p after make-frame-visible

[Stefan Monnier]

Aaron Jensen [2021-09-27 05:46:38] wrote:

> On Mon, Sep 27, 2021 at 4:51 AM martin rudalics <rudalics@gmx.at> wrote:
>>
>>  > I only see my problem sometimes. It doesn't reproduce if I restart
>>  > Emacs until it does (some random amount of time later)
>>
>> So you have to trigger it, somehow.
>
> Right, and I have no idea how at the moment.
>
>>  > When I do the above, if I focus away from the 2nd frame and back I see
>>  > nothing.
>>
>> With and/or without my patch?  Please compare the behaviors.
>
> Same w/o the patch, I don't know if application focus events are not
> handled on macos, only window (frame) focus changes
>
>>  > If I minimize it (I'm on macOS, so not sure if that is the
>>  > closest thing to lowering) then I either get a focus-out-event or
>>  > switch-frame-event because it ends up giving the focus to the 1st
>>  > frame, that's just how macOS works.
>>
>> That's why I told you to provide for a third window (using another Emacs
>> process or another application) that gets focus when you minimize the
>> Emacs frame.  But getting a focus-out event with the patch applied _is_
>> bad - these should be caught by the patch.
>
> Right, there is a 3rd window, but that's not how macOS works. I think
> I found how to do it though. If I minimize the messages frame, then
> eval the form, then minimize frame that and raise it, I do get a
> focus-out on that frame, with or without the patch.
>
>>  > Maybe I can find a place to put
>>  > last-input-event if it reproduces again.
>>  >
>>  > I'm using your patch now, so I can let you know if it happens again.
>>
>> You can also set 'while-no-input-ignore-events' to include or rule out
>> specific events.
>
> Ok, I'll see if I can do that once I get it reproducing again.
>
> Thanks,
>
> Aaron

FWIW, last time I had a similar problem I cooked up the following patch
in the hope of catching the offending event in `throw-on-input--trace`.

It's not in an acceptable form to be installed into `master`, but maybe
someone can be inspired to clean it up (and avoid the implicit memory
leak, etc...).


        Stefan


diff --git a/src/keyboard.c b/src/keyboard.c
index bc6f97586dd..4e1a1f51ef5 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3682,7 +3730,11 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
      as input, set quit-flag to cause an interrupt.  */
   if (!NILP (Vthrow_on_input)
       && NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events)))
-    Vquit_flag = Vthrow_on_input;
+    {
+      Vquit_flag = Vthrow_on_input;
+      Vthrow_on_input__trace = Fcons (make_lispy_event (&event->ie),
+                                      Vthrow_on_input__trace);
+    }
 }
 
 
@@ -12412,6 +12387,10 @@ syms_of_keyboard (void)
 peculiar kind of quitting.  */);
   Vthrow_on_input = Qnil;
 
+  DEFVAR_LISP ("throw-on-input--trace", Vthrow_on_input__trace,
+               doc: /* Trace of events that caused a `throw-on-input`. */);
+  Vthrow_on_input__trace = Qnil;
+
   DEFVAR_LISP ("command-error-function", Vcommand_error_function,
 	       doc: /* Function to output error messages.
 Called with three arguments:

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Mon, Sep 27, 2021 at 4:51 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  > I only see my problem sometimes. It doesn't reproduce if I restart
>  > Emacs until it does (some random amount of time later)
>
> So you have to trigger it, somehow.
>
>  > When I do the above, if I focus away from the 2nd frame and back I see
>  > nothing.
>
> With and/or without my patch?  Please compare the behaviors.
>
>  > If I minimize it (I'm on macOS, so not sure if that is the
>  > closest thing to lowering) then I either get a focus-out-event or
>  > switch-frame-event because it ends up giving the focus to the 1st
>  > frame, that's just how macOS works.
>
> That's why I told you to provide for a third window (using another Emacs
> process or another application) that gets focus when you minimize the
> Emacs frame.  But getting a focus-out event with the patch applied _is_
> bad - these should be caught by the patch.
>
>  > Maybe I can find a place to put
>  > last-input-event if it reproduces again.
>  >
>  > I'm using your patch now, so I can let you know if it happens again.
>
> You can also set 'while-no-input-ignore-events' to include or rule out
> specific events.

last-input-event is 134217848 after I get into that situation w/ M-x.
I'm guessing that's M-x, because if I do C-h v, I end up with 118 (v).

That won't show non-input events, will it?

Aaron

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Mon, Sep 27, 2021 at 7:02 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> last-input-event is 134217848 after I get into that situation w/ M-x.
> I'm guessing that's M-x, because if I do C-h v, I end up with 118 (v).
>
> That won't show non-input events, will it?

To be clear, this is with your patch, so that patch did not appear to
fix this issue. I will try out the throw on input patch to see if I
can identify the event


Aaron

Re: input-pending-p after make-frame-visible

[Aaron Jensen]

On Mon, Sep 27, 2021 at 10:29 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Mon, Sep 27, 2021 at 7:02 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
> >
> > last-input-event is 134217848 after I get into that situation w/ M-x.
> > I'm guessing that's M-x, because if I do C-h v, I end up with 118 (v).
> >
> > That won't show non-input events, will it?
>
> To be clear, this is with your patch, so that patch did not appear to
> fix this issue. I will try out the throw on input patch to see if I
> can identify the event

It reproduced again and whatever is causing the event, it does not
trigger throw on input (and so is not traced).

Aaron

Re: input-pending-p after make-frame-visible

[martin rudalics]

 > It reproduced again and whatever is causing the event, it does not
 > trigger throw on input (and so is not traced).

Could you please try to tell in a few sentences what you did and what
happened so maybe someone else can understand the issue and chime in.
I'm meanwhile completely lost.

Thanks, martin

Re: input-pending-p after make-frame-visible

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Mon, 27 Sep 2021 19:02:04 -0400
> Cc: emacs-devel@gnu.org
> 
> >  > I'm using your patch now, so I can let you know if it happens again.
> >
> > You can also set 'while-no-input-ignore-events' to include or rule out
> > specific events.
> 
> last-input-event is 134217848 after I get into that situation w/ M-x.
> I'm guessing that's M-x, because if I do C-h v, I end up with 118 (v).

Yes, it's M-x.  The easiest way to see that is

   M-: 134217848 RET

which will show:

   134217848 (#o1000000170, #x8000078)

And the last representation tells you it's 'x' (#x78) with 27th bit
set.  And in lisp.h we have:

  enum char_bits
    {
      CHAR_ALT = 0x0400000,
      CHAR_SUPER = 0x0800000,
      CHAR_HYPER = 0x1000000,
      CHAR_SHIFT = 0x2000000,
      CHAR_CTL = 0x4000000,
      CHAR_META = 0x8000000,  <<<<<<<<<<<<<<<<<<<<<<<<

But where does that M-x come from in the recipe in question?