Re: BUFFER_SWITCH_EVENT

[Stefan Monnier]

>> >> We already look them up presumably in the right place.  I don't
>> >> understand why we'd need to look them up again after running
>> >> a process filter.
>> > Because a filter could have switched buffers, I presume.
>> 
>> I don't follow:
>> 
>> 1- we enter read-key-sequence
>> 2- we read the set of keymaps
>> 3- some process filter is executed, changing the current buffer
>> 4- the set of keymaps that we need to use is still the same we computed
>>    at 2 and the keymaps themselves are also still the same
>>
>> What am I missing?
>
> You are asking the wrong guy, but I will try anyway: since the buffer
> changed, we need to replay the key sequence using the keymaps local to
> that buffer, not the ones we computed at 2.  Right?

I don't think so: we shouldn't use the keymaps in the buffer where the
process filter ran but the keymaps in the buffer of the selected window
(or the window into which the user pressed the mouse button).

I suspect that the main original use for this special input event has
been made redundant when we changed read-key-sequence to recompute the
set of active keymaps when it receives the first event.

I just changed my local Emacs by removing this special event (and
related code, see patch below).  I'll see if I notice any consequence.


        Stefan


commit eace674910f89f2f2b685cf93c0c152460c11347
Author: Stefan Monnier <address@hidden>
Date:   Tue Sep 25 12:11:00 2018 -0400

    Get rid of BUFFER_SWITCH_EVENT

diff --git a/src/keyboard.c b/src/keyboard.c
index f62f93dc22..2ca09f394c 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -726,8 +726,9 @@ void
 force_auto_save_soon (void)
 {
   last_auto_save = - auto_save_interval - 1;
-
-  record_asynch_buffer_change ();
+  /* FIXME: What does record_asynch_buffer_change have to do with forcing
+     an auto-save?   */
+  /* record_asynch_buffer_change (); */
 }
 #endif
 
@@ -2836,12 +2837,6 @@ read_char (int commandflag, Lisp_Object map,
       goto wrong_kboard;
     }
 
-  /* Buffer switch events are only for internal wakeups
-     so don't show them to the user.
-     Also, don't record a key if we already did.  */
-  if (BUFFERP (c))
-    goto exit;
-
   /* Process special events within read_char
      and loop around to read another event.  */
   save = Vquit_flag;
@@ -3125,7 +3120,7 @@ read_char (int commandflag, Lisp_Object map,
              && EQ (EVENT_HEAD_KIND (EVENT_HEAD (c)), Qmouse_click))
            XSETCAR (help_form_saved_window_configs, Qnil);
        }
-      while (BUFFERP (c));
+      while (false);
       /* Remove the help from the frame.  */
       unbind_to (count, Qnil);
 
@@ -3135,7 +3130,7 @@ read_char (int commandflag, Lisp_Object map,
          cancel_echoing ();
          do
            c = read_char (0, Qnil, Qnil, 0, NULL);
-         while (BUFFERP (c));
+         while (false);
        }
     }
 
@@ -3375,8 +3370,7 @@ readable_events (int flags)
               if (event == kbd_buffer + KBD_BUFFER_SIZE)
                 event = kbd_buffer;
              if (!((flags & READABLE_EVENTS_FILTER_EVENTS)
-                   && (event->kind == FOCUS_IN_EVENT
-                       || event->kind == BUFFER_SWITCH_EVENT))
+                   && event->kind == FOCUS_IN_EVENT)
                  && !((flags & READABLE_EVENTS_IGNORE_SQUEEZABLES)
                       && (event->kind == SCROLL_BAR_CLICK_EVENT
                           || event->kind == HORIZONTAL_SCROLL_BAR_CLICK_EVENT)
@@ -3538,14 +3532,6 @@ kbd_buffer_store_buffered_event (union 
buffered_input_event *event,
          return;
        }
     }
-  /* Don't insert two BUFFER_SWITCH_EVENT's in a row.
-     Just ignore the second one.  */
-  else if (event->kind == BUFFER_SWITCH_EVENT
-          && kbd_fetch_ptr != kbd_store_ptr
-          && ((kbd_store_ptr == kbd_buffer
-               ? kbd_buffer + KBD_BUFFER_SIZE - 1
-               : kbd_store_ptr - 1)->kind) == BUFFER_SWITCH_EVENT)
-    return;
 
   if (kbd_store_ptr - kbd_buffer == KBD_BUFFER_SIZE)
     kbd_store_ptr = kbd_buffer;
@@ -3945,7 +3931,6 @@ kbd_buffer_get_event (KBOARD **kbp,
 #ifdef HAVE_XWIDGETS
       case XWIDGET_EVENT:
 #endif
-      case BUFFER_SWITCH_EVENT:
       case SAVE_SESSION_EVENT:
       case NO_EVENT:
       case HELP_EVENT:
@@ -5348,14 +5333,6 @@ make_lispy_event (struct input_event *event)
       return list2 (Qmove_frame, list1 (event->frame_or_window));
 #endif
 
-    case BUFFER_SWITCH_EVENT:
-      {
-       /* The value doesn't matter here; only the type is tested.  */
-       Lisp_Object obj;
-        XSETBUFFER (obj, current_buffer);
-        return obj;
-      }
-
     /* Just discard these, by returning nil.
        With MULTI_KBOARD, these events are used as placeholders
        when we need to randomly delete events from the queue.
@@ -6830,41 +6807,6 @@ get_input_pending (int flags)
   return input_pending;
 }
 
-/* Put a BUFFER_SWITCH_EVENT in the buffer
-   so that read_key_sequence will notice the new current buffer.  */
-
-void
-record_asynch_buffer_change (void)
-{
-  /* We don't need a buffer-switch event unless Emacs is waiting for input.
-     The purpose of the event is to make read_key_sequence look up the
-     keymaps again.  If we aren't in read_key_sequence, we don't need one,
-     and the event could cause trouble by messing up (input-pending-p).
-     Note: Fwaiting_for_user_input_p always returns nil when async
-     subprocesses aren't supported.  */
-  if (!NILP (Fwaiting_for_user_input_p ()))
-    {
-      struct input_event event;
-
-      EVENT_INIT (event);
-      event.kind = BUFFER_SWITCH_EVENT;
-      event.frame_or_window = Qnil;
-      event.arg = Qnil;
-
-      /* Make sure no interrupt happens while storing the event.  */
-#ifdef USABLE_SIGIO
-      if (interrupt_input)
-       kbd_buffer_store_event (&event);
-      else
-#endif
-       {
-         stop_polling ();
-         kbd_buffer_store_event (&event);
-         start_polling ();
-       }
-    }
-}
-
 /* Read any terminal input already buffered up by the system
    into the kbd_buffer, but do not wait.
 
@@ -8690,7 +8632,7 @@ read_char_minibuf_menu_prompt (int commandflag,
       kset_defining_kbd_macro (current_kboard, Qnil);
       do
        obj = read_char (commandflag, Qnil, Qt, 0, NULL);
-      while (BUFFERP (obj));
+      while (false);
       kset_defining_kbd_macro (current_kboard, orig_defn_macro);
 
       if (!FIXNUMP (obj) || XFIXNUM (obj) == -2
@@ -9242,29 +9184,6 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object 
prompt,
              break;
            }
 
-         /* If the current buffer has been changed from under us, the
-            keymap may have changed, so replay the sequence.  */
-         if (BUFFERP (key))
-           {
-             timer_resume_idle ();
-
-             mock_input = t;
-             /* Reset the current buffer from the selected window
-                in case something changed the former and not the latter.
-                This is to be more consistent with the behavior
-                of the command_loop_1.  */
-             if (fix_current_buffer)
-               {
-                 if (! FRAME_LIVE_P (XFRAME (selected_frame)))
-                   Fkill_emacs (Qnil);
-                 if (XBUFFER (XWINDOW (selected_window)->contents)
-                     != current_buffer)
-                   Fset_buffer (XWINDOW (selected_window)->contents);
-               }
-
-             goto replay_sequence;
-           }
-
          /* If we have a quit that was typed in another frame, and
             quit_throw_to_read_char switched buffers,
             replay to get the right keymap.  */
diff --git a/src/keyboard.h b/src/keyboard.h
index ce4630b8a3..d4b5811298 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -446,7 +446,6 @@ extern void push_kboard (struct kboard *);
 extern void push_frame_kboard (struct frame *);
 extern void pop_kboard (void);
 extern void temporarily_switch_to_single_kboard (struct frame *);
-extern void record_asynch_buffer_change (void);
 extern void input_poll_signal (int);
 extern void start_polling (void);
 extern void stop_polling (void);
diff --git a/src/process.c b/src/process.c
index d35ea84945..6d5d92560a 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5150,14 +5150,6 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
 
              timer_delay = timer_check ();
 
-             /* If a timer has run, this might have changed buffers
-                an alike.  Make read_key_sequence aware of that.  */
-             if (timers_run != old_timers_run
-                 && (old_buffer != current_buffer
-                     || !EQ (old_window, selected_window))
-                 && waiting_for_user_input_p == -1)
-               record_asynch_buffer_change ();
-
              if (timers_run != old_timers_run && do_display)
                /* We must retry, since a timer may have requeued itself
                   and that could alter the time_delay.  */
@@ -5527,14 +5519,6 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
                leave = true;
            }
 
-         /* If a timer has run, this might have changed buffers
-            an alike.  Make read_key_sequence aware of that.  */
-         if (timers_run != old_timers_run
-             && waiting_for_user_input_p == -1
-             && (old_buffer != current_buffer
-             || !EQ (old_window, selected_window)))
-           record_asynch_buffer_change ();
-
          if (leave)
            break;
        }
@@ -6024,18 +6008,6 @@ read_and_dispose_of_process_output (struct Lisp_Process 
*p, char *chars,
   /* Restore waiting_for_user_input_p as it was
      when we were called, in case the filter clobbered it.  */
   waiting_for_user_input_p = waiting;
-
-#if 0 /* Call record_asynch_buffer_change unconditionally,
-        because we might have changed minor modes or other things
-        that affect key bindings.  */
-  if (! EQ (Fcurrent_buffer (), obuffer)
-      || ! EQ (current_buffer->keymap, okeymap))
-#endif
-    /* But do it only if the caller is actually going to read events.
-       Otherwise there's no need to make him wake up, and it could
-       cause trouble (for example it would make sit_for return).  */
-    if (waiting_for_user_input_p == -1)
-      record_asynch_buffer_change ();
 }
 
 DEFUN ("internal-default-process-filter", Finternal_default_process_filter,
@@ -7188,16 +7160,6 @@ exec_sentinel (Lisp_Object proc, Lisp_Object reason)
      when we were called, in case the filter clobbered it.  */
   waiting_for_user_input_p = waiting;
 
-#if 0
-  if (! EQ (Fcurrent_buffer (), obuffer)
-      || ! EQ (current_buffer->keymap, okeymap))
-#endif
-    /* But do it only if the caller is actually going to read events.
-       Otherwise there's no need to make him wake up, and it could
-       cause trouble (for example it would make sit_for return).  */
-    if (waiting_for_user_input_p == -1)
-      record_asynch_buffer_change ();
-
   unbind_to (count, Qnil);
 }
 
diff --git a/src/termhooks.h b/src/termhooks.h
index b90ebc66e4..3cf5fb036e 100644
--- a/src/termhooks.h
+++ b/src/termhooks.h
@@ -160,7 +160,6 @@ enum event_kind
   SELECTION_REQUEST_EVENT,     /* Another X client wants a selection from us.
                                   See `struct selection_input_event'.  */
   SELECTION_CLEAR_EVENT,       /* Another X client cleared our selection.  */
-  BUFFER_SWITCH_EVENT,         /* A process filter has switched buffers.  */
   DELETE_WINDOW_EVENT,         /* An X client said "delete this window".  */
 #ifdef HAVE_NTGUI
   END_SESSION_EVENT,           /* The user is logging out or shutting down.  */
diff --git a/src/w32term.c b/src/w32term.c
index 8d189ae32c..259361f738 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -4718,8 +4718,10 @@ w32_read_socket (struct terminal *terminal,
                    }
                  else if (!NILP (Vframe_list) && !NILP (XCDR (Vframe_list)))
                    /* Force a redisplay sooner or later to update the
-                      frame titles in case this is the second frame.  */
-                   record_asynch_buffer_change ();
+                      frame titles in case this is the second frame.
+                       FIXME: What does record_asynch_buffer_change have
+                       to do with forcing a redisplay?  */
+                   /* record_asynch_buffer_change () */;
                }
              else
                {
@@ -5277,8 +5279,10 @@ w32_read_socket (struct terminal *terminal,
                             && ! NILP (XCDR (Vframe_list)))
                      /* Force a redisplay sooner or later
                         to update the frame titles
-                        in case this is the second frame.  */
-                     record_asynch_buffer_change ();
+                        in case this is the second frame.
+                         FIXME: What does record_asynch_buffer_change have
+                         to do with forcing a redisplay?  */
+                   /* record_asynch_buffer_change () */;
 
                  /* Windows can send us a SIZE_MAXIMIZED message even
                     when fullscreen is fullboth.  The following is a
@@ -5330,8 +5334,10 @@ w32_read_socket (struct terminal *terminal,
                             && ! NILP (XCDR (Vframe_list)))
                      /* Force a redisplay sooner or later
                         to update the frame titles
-                        in case this is the second frame.  */
-                     record_asynch_buffer_change ();
+                        in case this is the second frame.
+                         FIXME: What does record_asynch_buffer_change have
+                         to do with forcing a redisplay?  */
+                   /* record_asynch_buffer_change () */;
                  }
 
                  if (EQ (get_frame_param (f, Qfullscreen), Qmaximized))
@@ -5623,8 +5629,10 @@ w32_read_socket (struct terminal *terminal,
                    DebPrint (("obscured frame %p (%s) found to be visible\n",
                               f, SDATA (f->name)));
 
-                   /* Force a redisplay sooner or later.  */
-                   record_asynch_buffer_change ();
+                   /* Force a redisplay sooner or later.
+                       FIXME: What does record_asynch_buffer_change have
+                       to do with forcing a redisplay?  */
+                   /* record_asynch_buffer_change () */;
                  }
              }
          }
diff --git a/src/xterm.c b/src/xterm.c
index 08d40ac109..8f788626b6 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -8192,8 +8192,10 @@ handle_one_xevent (struct x_display_info *dpyinfo,
             }
           else if (! NILP (Vframe_list) && ! NILP (XCDR (Vframe_list)))
             /* Force a redisplay sooner or later to update the
-              frame titles in case this is the second frame.  */
-            record_asynch_buffer_change ();
+              frame titles in case this is the second frame.
+               FIXME: What does record_asynch_buffer_change have
+               to do with forcing a redisplay?  */
+         /* record_asynch_buffer_change () */;
         }
       goto OTHER;