bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros

[Stefan Monnier]

>> Basically, I think since our test suite runs just fine in batch, we
>> should be able to run it with inhibit-interaction=t as well (which
>> would fix annoying problems when some test fails and ends up waiting
>> for user input).
>
> Yes, I agree.  I'm interested in making this possible and willing to put
> in the work to do it for the Emacs test suite.  (since it will help make
> my own tests reliable)

IIUC, Eli is opposed to changing the behavior of `inhibit-interaction`,
so I think this will require a new variable, whose purpose is similar
but will allow more "quasi interactions" (most importantly calling
`read-event` with a timeout).

If you're interested, here's my WiP patch (which modifies the way
`inhibit-interaction` behaves).

The most delicate point might be the change to `sit-for` in batch mode:
I don't have a clear idea why we use `sleep-for` there and what is the
impact of using `read-event` instead.


        Stefan

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 8ab57d2b238..1ce84a1a2a9 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -1522,7 +1522,9 @@ ert-run-tests-batch-and-exit
   (or noninteractive
       (user-error "This function is only for use in batch mode"))
   (let ((eln-dir (and (featurep 'native-compile)
-                      (make-temp-file "test-nativecomp-cache-" t))))
+                  (make-temp-file "test-nativecomp-cache-" t)))
+        ;; Don't ever wait for user input.
+        (inhibit-interaction t))
     (when eln-dir
       (startup-redirect-eln-cache eln-dir))
     ;; Better crash loudly than attempting to recover from undefined
diff --git a/lisp/subr.el b/lisp/subr.el
index c317d558e24..aaeb8d0fb00 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3570,9 +3570,6 @@ sit-for
   ;; Then it was moved here using an implementation based on an idle timer,
   ;; which was then replaced by the use of read-event.
   (cond
-   (noninteractive
-    (sleep-for seconds)
-    t)
    ((input-pending-p t)
     nil)
    ((or (<= seconds 0)
diff --git a/src/keyboard.c b/src/keyboard.c
index 4b5e20fb24c..5d29b1bfe8b 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2815,7 +2819,10 @@ read_char (int commandflag, Lisp_Object map,
      situation where an idle timer calls `sit-for'.  */
 
   if (!end_time)
-    timer_start_idle ();
+    {
+      barf_if_interaction_inhibited ();
+      timer_start_idle ();
+    }
 
   /* If in middle of key sequence and minibuffer not active,
      start echoing if enough time elapses.  */
@@ -2875,8 +2882,12 @@ read_char (int commandflag, Lisp_Object map,
      because the recursive call of read_char in read_char_minibuf_menu_prompt
      does not pass on any keymaps.  */
 
+  /* FIXME: If the keymap is *not* a menu, this fails miserably,
+     signaling (error "Empty menu") instead!  */
   if (KEYMAPP (map) && INTERACTIVE
       && !NILP (prev_event)
+      /* FIXME: These conditions are re-tested redundantly inside
+         read_char_x_menu_prompt!  */
       && EVENT_HAS_PARAMETERS (prev_event)
       && !EQ (XCAR (prev_event), Qmenu_bar)
       && !EQ (XCAR (prev_event), Qtab_bar)
@@ -2884,7 +2895,11 @@ read_char (int commandflag, Lisp_Object map,
       /* Don't bring up a menu if we already have another event.  */
       && !CONSP (Vunread_command_events))
     {
+      barf_if_interaction_inhibited ();
       c = read_char_x_menu_prompt (map, prev_event, used_mouse_menu);
+      /* FIXME: read_char_x_menu_prompt can sometimes do nothing and just
+         return nil, should we really stop the idle timer and jump to `exit`
+         in that case?  */
 
       /* Now that we have read an event, Emacs is not idle.  */
       if (!end_time)
@@ -9872,17 +9872,13 @@
   if (! menu_prompting)
     return Qnil;
 
-  /* If we got to this point via a mouse click,
-     use a real menu for mouse selection.  */
-  if (EVENT_HAS_PARAMETERS (prev_event)
+  eassert (EVENT_HAS_PARAMETERS (prev_event)
       && !EQ (XCAR (prev_event), Qmenu_bar)
       && !EQ (XCAR (prev_event), Qtab_bar)
-      && !EQ (XCAR (prev_event), Qtool_bar))
-    {
-      /* Display the menu and get the selection.  */
-      Lisp_Object value;
+           && !EQ (XCAR (prev_event), Qtool_bar));
 
-      value = x_popup_menu_1 (prev_event, get_keymap (map, 0, 1));
+  /* Display the menu and get the selection.  */
+  Lisp_Object value = x_popup_menu_1 (prev_event, get_keymap (map, 0, 1));
       if (CONSP (value))
        {
          Lisp_Object tem;
@@ -9917,8 +9917,6 @@
       if (used_mouse_menu)
        *used_mouse_menu = true;
       return value;
-    }
-  return Qnil ;
 }
 
 static Lisp_Object
@@ -13116,6 +13125,15 @@ syms_of_keyboard (void)
   defsubr (&Sposn_at_point);
   defsubr (&Sposn_at_x_y);
 
+  defsubr (&Sadjust_point_for_property);
+  DEFVAR_LISP ("point-adjustment-function",
+               Vpoint_adjustment_function,
+               doc: /* Function called to adjust point after commands.
+This function is run after each command that moved point,
+unless `disable-point-adjustment' or `global-disable-point-adjustment'
+is non-nil.  */);
+  Vpoint_adjustment_function = intern ("adjust-point-for-property");
+
   DEFVAR_LISP ("last-command-event", last_command_event,
                     doc: /* Last input event of a key sequence that called a 
command.
 See Info node `(elisp)Command Loop Info'.*/);
diff --git a/src/lread.c b/src/lread.c
index c11c641440d..aa301980904 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -950,8 +950,6 @@ DEFUN ("read-char", Fread_char, Sread_char, 0, 3, 0,
 {
   Lisp_Object val;
 
-  barf_if_interaction_inhibited ();
-
   if (! NILP (prompt))
     {
       cancel_echoing ();
@@ -988,8 +986,6 @@ DEFUN ("read-event", Fread_event, Sread_event, 0, 3, 0,
 `inhibited-interaction' error.  */)
   (Lisp_Object prompt, Lisp_Object inherit_input_method, Lisp_Object seconds)
 {
-  barf_if_interaction_inhibited ();
-
   if (! NILP (prompt))
     {
       cancel_echoing ();
@@ -1027,8 +1023,6 @@ DEFUN ("read-char-exclusive", Fread_char_exclusive, 
Sread_char_exclusive, 0, 3,
 {
   Lisp_Object val;
 
-  barf_if_interaction_inhibited ();
-
   if (! NILP (prompt))
     {
       cancel_echoing ();
@@ -5157,7 +5151,7 @@ DEFUN ("unintern", Funintern, Sunintern, 1, 2, 0,
   return Qt;
 }
 
-/* Return the symbol in OBARRAY whose names matches the string
+/* Return the symbol in OBARRAY whose name matches the string
    of SIZE characters (SIZE_BYTE bytes) at PTR.
    If there is no such symbol, return the integer bucket number of
    where the symbol would be if it were present.
diff --git a/src/minibuf.c b/src/minibuf.c
index 7c0c9799a60..1cb91ec2636 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -316,6 +316,8 @@ read_minibuf_noninteractive (Lisp_Object prompt, bool 
expflag,
   struct emacs_tty etty;
   bool etty_valid UNINIT;
 
+  barf_if_interaction_inhibited ();
+
   /* Check, whether we need to suppress echoing.  */
   if (CHARACTERP (Vread_hide_char))
     hide_char = XFIXNAT (Vread_hide_char);
@@ -1344,8 +1346,6 @@ DEFUN ("read-from-minibuffer", Fread_from_minibuffer,
 {
   Lisp_Object histvar, histpos, val;
 
-  barf_if_interaction_inhibited ();
-
   CHECK_STRING (prompt);
   if (NILP (keymap))
     keymap = Vminibuffer_local_map;
diff --git a/test/lisp/autorevert-tests.el b/test/lisp/autorevert-tests.el
index c202970e0b2..58002d597f0 100644
--- a/test/lisp/autorevert-tests.el
+++ b/test/lisp/autorevert-tests.el
@@ -110,7 +110,7 @@ auto-revert--wait-for-revert
       (if (and (or file-notify--library
                    (file-remote-p temporary-file-directory))
                (with-current-buffer buffer auto-revert-use-notify))
-          (read-event nil nil 0.05)
+          (sit-for 0.05)
         (sleep-for 0.05)))))
 
 (defmacro auto-revert--deftest-remote (test docstring)
@@ -429,7 +429,7 @@ auto-revert-test--wait-for
   (let ((ct (current-time)))
     (while (and (< (float-time (time-subtract nil ct)) max-wait)
                 (not (funcall pred)))
-      (read-event nil nil 0.1))))
+      (sit-for 0.1))))
 
 (defun auto-revert-test--wait-for-buffer-text (buffer string max-wait)
   "Wait until BUFFER has the contents STRING, or MAX-WAIT seconds elapsed."
diff --git a/test/lisp/filenotify-tests.el b/test/lisp/filenotify-tests.el
index 28f4d5fa181..75d43a5c75d 100644
--- a/test/lisp/filenotify-tests.el
+++ b/test/lisp/filenotify-tests.el
@@ -77,8 +77,7 @@ file-notify--test-monitors
 (defun file-notify--test-wait-event ()
   "Wait for one event.
 There are different timeouts for local and remote file notification libraries."
-  (read-event
-   nil nil
+  (sit-for
    (cond
     ;; gio/gpollfilemonitor.c declares POLL_TIME_SECS 5.  So we must
     ;; wait at least this time in the GPollFileMonitor case.  A
diff --git a/test/lisp/net/dbus-tests.el b/test/lisp/net/dbus-tests.el
index fec252e12dd..dd8200b345e 100644
--- a/test/lisp/net/dbus-tests.el
+++ b/test/lisp/net/dbus-tests.el
@@ -758,7 +758,7 @@ dbus-test05-register-signal
          dbus--test-interface member "foo")
        (with-timeout (1 (dbus--test-timeout-handler))
           (while (null dbus--test-signal-received)
-            (read-event nil nil 0.1)))
+            (sit-for 0.1)))
         (should (equal dbus--test-signal-received '("foo")))
 
         ;; Send two arguments, compound types.
@@ -769,7 +769,7 @@ dbus-test05-register-signal
          '(:array :byte 1 :byte 2 :byte 3) '(:variant :string "bar"))
        (with-timeout (1 (dbus--test-timeout-handler))
           (while (null dbus--test-signal-received)
-            (read-event nil nil 0.1)))
+            (sit-for 0.1)))
         (should (equal dbus--test-signal-received '((1 2 3) ("bar"))))
 
         ;; Unregister signal.
@@ -1124,7 +1124,7 @@ dbus-test06-register-property-emits-signal
             (,dbus--test-service ,dbus--test-path))))
        (with-timeout (1 (dbus--test-timeout-handler))
           (while (null dbus--test-signal-received)
-            (read-event nil nil 0.1)))
+            (sit-for 0.1)))
         ;; It returns three arguments, "interface" (a string),
         ;; "changed_properties" (an array of dict entries) and
         ;; "invalidated_properties" (an array of strings).
@@ -1150,7 +1150,7 @@ dbus-test06-register-property-emits-signal
           '(1 2 3)))
        (with-timeout (1 (dbus--test-timeout-handler))
           (while (null dbus--test-signal-received)
-            (read-event nil nil 0.1)))
+            (sit-for 0.1)))
         (should
          (equal
           dbus--test-signal-received
@@ -1883,7 +1883,7 @@ dbus-test08-register-monitor
          dbus--test-interface "Foo" "foo")
        (with-timeout (1 (dbus--test-timeout-handler))
           (while (null dbus--test-signal-received)
-            (read-event nil nil 0.1)))
+            (sit-for 0.1)))
 
         ;; Unregister monitor.
         (should (dbus-unregister-object registered))
@@ -1896,7 +1896,7 @@ dbus-test08-register-monitor
          dbus--test-interface "Foo" "foo")
        (with-timeout (1 (ignore))
           (while (null dbus--test-signal-received)
-            (read-event nil nil 0.1)))
+            (sit-for 0.1)))
         (should-not dbus--test-signal-received))
 
     ;; Cleanup.
diff --git a/test/lisp/progmodes/eglot-tests.el 
b/test/lisp/progmodes/eglot-tests.el
index 4725885038e..d05851c8f4c 100644
--- a/test/lisp/progmodes/eglot-tests.el
+++ b/test/lisp/progmodes/eglot-tests.el
@@ -260,7 +260,7 @@ eglot--wait-for
                      ;; `read-event' is essential to have the file
                      ;; watchers come through.
                      (cond ((fboundp 'flush-standard-output)
-                            (read-event nil nil 0.1) (princ ".")
+                            (sit-for 0.1) (princ ".")
                             (flush-standard-output))
                            (t
                             (read-event "." nil 0.1)))
diff --git a/test/lisp/progmodes/flymake-tests.el 
b/test/lisp/progmodes/flymake-tests.el
index 21dbb0711d2..043efcbd33c 100644
--- a/test/lisp/progmodes/flymake-tests.el
+++ b/test/lisp/progmodes/flymake-tests.el
@@ -47,12 +47,12 @@ flymake-tests--wait-for-backends
   ;; that will indeed unblock pending process output is
   ;; reading an input event, so, as a workaround, use a dummy
   ;; `read-event' with a very short timeout.
-  (unless noninteractive (read-event "" nil 0.1))
+  (unless noninteractive (sit-for 0.1))
   (cl-loop repeat 5
            for notdone = (cl-set-difference (flymake-running-backends)
                                             (flymake-reporting-backends))
            while notdone
-           unless noninteractive do (read-event "" nil 0.1)
+           unless noninteractive do (sit-for 0.1)
            do (sleep-for (+ 0.5 (or flymake-no-changes-timeout 0)))
            finally (when notdone (ert-skip
                                   (format "Some backends not reporting yet %s"
diff --git a/test/src/inotify-tests.el b/test/src/inotify-tests.el
index 0c078627786..9c2c957c049 100644
--- a/test/src/inotify-tests.el
+++ b/test/src/inotify-tests.el
@@ -61,7 +61,7 @@ inotify-file-watch-simple
             (progn
               (with-temp-file temp-file
                 (insert "Foo\n"))
-              (read-event nil nil 5)
+              (sit-for 5)
               (should (> events 0)))
           (should (inotify-valid-p wd))
           (inotify-rm-watch wd)
diff --git a/test/src/thread-tests.el b/test/src/thread-tests.el
index d9048dcf287..b41a89c834a 100644
--- a/test/src/thread-tests.el
+++ b/test/src/thread-tests.el
@@ -328,13 +328,13 @@ threads-signal-main-thread
   ;; We cannot use `ert-with-message-capture', because threads do not
   ;; know let-bound variables.
   (with-current-buffer "*Messages*"
-    (let (buffer-read-only)
+    (let ((inhibit-read-only t))
       (erase-buffer))
     (let ((thread
            (make-thread (lambda () (thread-signal main-thread 'error nil)))))
       (while (thread-live-p thread)
         (thread-yield))
-      (read-event nil nil 0.1)
+      (sit-for 0.1)
       ;; No error has been raised, which is part of the test.
       (should
        (string-match