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