unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
@ 2023-12-15 16:48 Spencer Baugh
  2023-12-15 16:50 ` Spencer Baugh
  0 siblings, 1 reply; 24+ messages in thread
From: Spencer Baugh @ 2023-12-15 16:48 UTC (permalink / raw)
  To: 67837; +Cc: Lars Ingebrigtsen


If we run a keyboard macro (including ones which never interact with the
user) while inhibit-interaction=t, and the keyboard macro invokes
anything which reads input (which can be provided by the keyboard
macro), then the keyboard macro will error.

For example:
(let ((inhibit-interaction t))
  (execute-kbd-macro (read-kbd-macro "M-: 1 RET")))

So, for example, if a user has defined a function using keyboard macros,
and that function is invoked within inhibit-interaction=t, the function
will unnecessarily fail.

This is because inhibit-interaction is checked too early: it should be
checked after the keyboard macro code has a chance to provide input.  A
patch which does this follows.


In GNU Emacs 29.1.90 (build 5, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2023-12-14 built on
 igm-qws-u22796a
Repository revision: 57fa5a53f74e489702825045832f52730c5d550f
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.9 (Green Obsidian)

Configured using:
 'configure --config-cache --with-x-toolkit=lucid
 --with-gif=ifavailable'

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM
XINPUT2 XPM LUCID ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Magit

Minor modes in effect:
  delete-selection-mode: t
  global-so-long-mode: t
  pixel-scroll-precision-mode: t
  jane-fe-minor-mode: t
  editorconfig-mode: t
  which-function-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  server-mode: t
  windmove-mode: t
  savehist-mode: t
  save-place-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tab-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  context-menu-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane/packages hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane-dap/packages
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane/config hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane-dap/config
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane/config hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane-fzf/config
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane/packages hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane-fzf/packages
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/jane/packages hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/spacemacs-layers/org-fe/packages
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-types hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-types
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-macros hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-macros
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-repeat hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-repeat
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-search hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-search
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-commands hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-commands
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-test-helpers hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-test-helpers
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-command-window hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-command-window
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-common hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-common
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-jumps hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-jumps
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-ex hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-ex
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-pkg hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-pkg
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-digraphs hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-digraphs
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-integration hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-integration
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-vars hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-vars
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-maps hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-maps
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-core hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-core
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-tests hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-tests
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/evil/evil-states hides /home/sbaugh/.emacs.d/elpa/evil-1.15.0/evil-states
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/csharp-mode hides /home/sbaugh/src/emacs/emacs-29/lisp/progmodes/csharp-mode
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/xref hides /home/sbaugh/src/emacs/emacs-29/lisp/progmodes/xref
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/project hides /home/sbaugh/src/emacs/emacs-29/lisp/progmodes/project
/home/sbaugh/.emacs.d/elpa/popup-0.5.9/popup-autoloads hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/popup/popup-autoloads
/home/sbaugh/.emacs.d/elpa/popup-0.5.9/popup hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/popup/popup
/home/sbaugh/.emacs.d/elpa/async-1.9.7/dired-async hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/async/dired-async
/home/sbaugh/.emacs.d/elpa/async-1.9.7/async hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/async/async
/home/sbaugh/.emacs.d/elpa/async-1.9.7/smtpmail-async hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/async/smtpmail-async
/home/sbaugh/.emacs.d/elpa/async-1.9.7/async-test hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/async/async-test
/home/sbaugh/.emacs.d/elpa/async-1.9.7/async-bytecomp hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/async/async-bytecomp
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/auctex/lpath hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/dictionary/lpath
/home/sbaugh/src/emacs/emacs-29/lisp/net/dictionary hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/dictionary/dictionary
/home/sbaugh/src/emacs/emacs-29/lisp/progmodes/eglot hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/eglot/eglot
/home/sbaugh/src/emacs/emacs-29/lisp/external-completion hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/eglot/external-completion
/home/sbaugh/src/emacs/emacs-29/lisp/jsonrpc hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/eglot/jsonrpc
/usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/caml-font hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/ocaml/caml-font
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-autoloads hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-autoloads
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-imenu hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-imenu
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-grep hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-grep
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-mode hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-mode
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-files hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-files
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-net hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-net
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-semantic hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-semantic
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-ring hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-ring
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-help hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-help
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-regexp hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-regexp
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-locate hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-locate
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-utils hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-utils
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-misc hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-misc
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-types hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-types
/home/sbaugh/.emacs.d/elpa/helm-core-3.9.0/helm-source hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-source
/home/sbaugh/.emacs.d/elpa/helm-core-3.9.0/helm hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-eval hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-eval
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-elisp hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-elisp
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-command hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-command
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-bookmark hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-bookmark
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-info hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-info
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-elisp-package hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-elisp-package
/home/sbaugh/.emacs.d/elpa/helm-core-3.9.0/helm-lib hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-lib
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-comint hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-comint
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-id-utils hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-id-utils
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-sys hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-sys
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-color hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-color
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-eshell hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-eshell
/home/sbaugh/.emacs.d/elpa/helm-core-3.9.0/helm-core hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-core
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-x-files hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-x-files
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-shell hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-shell
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-tags hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-tags
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-for-files hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-for-files
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-find hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-find
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-font hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-font
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-dabbrev hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-dabbrev
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-epa hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-epa
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-config hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-config
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-easymenu hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-easymenu
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-global-bindings hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-global-bindings
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-external hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-external
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-occur hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-occur
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-man hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-man
/home/sbaugh/.emacs.d/elpa/helm-core-3.9.0/helm-multi-match hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-multi-match
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-buffers hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-buffers
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-fd hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-fd
/home/sbaugh/.emacs.d/elpa/helm-3.9.0/helm-adaptive hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/helm/helm-adaptive
/home/sbaugh/.emacs.d/elpa/dash-2.19.1/dash hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/dash/dash
/home/sbaugh/.emacs.d/elpa/dash-2.19.1/dash-functional hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/dash/dash-functional
/home/sbaugh/.emacs.d/elpa/ivy-0.14.0/colir hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/swiper/colir
/home/sbaugh/.emacs.d/elpa/ivy-0.14.0/ivy hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/swiper/ivy
/home/sbaugh/.emacs.d/elpa/ivy-0.14.0/ivy-overlay hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/swiper/ivy-overlay
/home/sbaugh/.emacs.d/elpa/ivy-0.14.0/ivy-faces hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/swiper/ivy-faces
/home/sbaugh/.emacs.d/elpa/with-editor-3.2.0/with-editor hides /usr/local/home/sbaugh/workspaces/fe-310581/+share+/app/emacs/elisp/contrib/with-editor/lisp/with-editor

Features:
(shadow sort mail-extr emacsbug goto-addr vc-bzr vc-src vc-sccs vc-svn
vc-cvs vc-rcs log-view vc-dir shortdoc pcmpl-unix pcmpl-gnu etags
fileloop magit-imenu git-rebase face-remap pulse vc-fe vc-hg vc cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs vc-git vc-dispatcher misc dabbrev misearch multi-isearch
help-fns radix-tree cl-print macros sh-script treesit executable
bug-reference dired-aux org-element org-persist org-id org-refile
avl-tree generator oc-basic ol-eww eww xdg url-queue mm-url ol-rmail
ol-mhe ol-irc ol-info ol-gnus nnselect gnus-art mm-uu mml2015 mm-view
mml-smime smime gnutls dig gnus-sum shr pixel-fill kinsoku url-file svg
dom gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud nnimap
nnmail mail-source utf7 nnoo gnus-spec gnus-int gnus-range gnus-win
ol-docview doc-view jka-compr image-mode exif ol-bibtex bibtex ol-bbdb
ol-w3m ol-doi org-link-doi let-alist delsel so-long jane-fe-read-feature
pixel-scroll cua-base tramp tramp-loaddefs trampver tramp-integration
tramp-compat parse-time iso8601 ffap jane-merlin merlin-imenu
merlin-xref merlin-cap merlin jane-async-merlin jane-completion grep
jane-common site-start jane-customization jane-comint org-protocol
jane-fe-project xref files-x jane-fe-menu ecaml_plugin view gopcaml
magit-bookmark bookmark image+ advice image-file image-converter
editorconfig editorconfig-core editorconfig-core-handle
editorconfig-fnmatch whitespace jane-auto-modes vba-mode markdown-mode
color jane jane-yasnippet jane-micro-features ert ewoc debug backtrace
jane-diff unified-test-mode shell-file core core-buffer core-error
jane-sexp jane-python jane-ocaml jane-tuareg-theme tuareg tuareg-compat
tuareg-opam skeleton flymake-proc flymake warnings thingatpt smie
caml-types caml-help caml-emacs find-file compile jane-cr jane-codeium
jane-align jane-deprecated jane-smerge gnu-elpa-keyring-update
jane-ocp-indent ocp-indent jane-eglot yasnippet-autoloads
swiper-autoloads htmlize-autoloads eglot-autoloads
editorconfig-autoloads codeium-autoloads jane-autoloads jane-util
ob-shell page-ext dired-x magit-extras project magit-submodule
magit-obsolete magit-blame magit-stash magit-reflog magit-bisect
magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit
magit-sequence magit-notes magit-worktree magit-tag magit-merge
magit-branch magit-reset magit-files magit-refs magit-status magit
magit-repos magit-apply magit-wip magit-log which-func imenu magit-diff
smerge-mode diff diff-mode git-commit log-edit message sendmail
yank-media puny dired dired-loaddefs rfc822 mml mml-sec epa derived epg
rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231
rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils mailheader pcvs-util
add-log magit-core magit-autorevert autorevert filenotify magit-margin
magit-transient magit-process with-editor shell server magit-mode
transient edmacro kmacro magit-git magit-section magit-utils crm dash
gnus nnheader gnus-util text-property-search mail-utils range mm-util
mail-prsvr cl-extra help-mode windmove org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-src ob-comint org-pcomplete pcomplete
org-list org-footnote org-faces org-entities time-date noutline outline
ob-emacs-lisp ob-core ob-eval org-cycle org-table ol rx org-fold
org-fold-core org-keys oc org-loaddefs find-func cal-menu calendar
cal-loaddefs org-version org-compat org-macs format-spec gdb-mi bindat
gud easy-mmode comint ansi-osc ansi-color ring vundo modus-vivendi-theme
modus-themes pcase savehist saveplace cus-edit pp cus-load icons
wid-edit adaptive-wrap-autoloads csv-mode-autoloads
cyberpunk-theme-autoloads evil-autoloads exwm-autoloads helm-autoloads
helm-core-autoloads async-autoloads ivy-autoloads magit-autoloads
git-commit-autoloads finder-inf magit-section-autoloads dash-autoloads
popup-autoloads url-http-ntlm-autoloads url-auth vc-hgcmd-autoloads
vundo-autoloads info with-editor-autoloads xelb-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs
cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 811046 110142)
 (symbols 48 53807 0)
 (strings 32 189876 13112)
 (string-bytes 1 7243691)
 (vectors 16 87904)
 (vector-slots 8 1801483 167316)
 (floats 8 601 738)
 (intervals 56 17985 1740)
 (buffers 976 53))





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 16:48 bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros Spencer Baugh
@ 2023-12-15 16:50 ` Spencer Baugh
  2023-12-15 18:54   ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Spencer Baugh @ 2023-12-15 16:50 UTC (permalink / raw)
  To: 67837; +Cc: Lars Ingebrigtsen

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


Patch to fix:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-inhibit-interaction-work-properly-with-keyboard.patch --]
[-- Type: text/x-patch, Size: 3487 bytes --]

From b0f680393991d9ccbd888be8f754a85775196799 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Fri, 15 Dec 2023 11:39:24 -0500
Subject: [PATCH] Make inhibit-interaction work properly with keyboard macros

Previously, inhibit-interaction=t prevented keyboard macros from
running, even when those macros did not result in user interaction,
since it was checked before the keyboard macro code had a chance to
provide input.

Now, if there's a running keyboard macro which can provide input, that
keyboard macro is allowed to provide input even if
inhibit-interaction=t.  This is achieved by moving the check on
inhibit-interaction to run after checking executing-kbd-macro in the
low-level input handling mechanism, read_char.

inhibit-interaction also suppresses reading from stdin in batch mode,
so we also must add a check on inhibit-interaction to
read_minibuf_noninteractive, which again is only called after checking
executing-kbd-macro.

* src/keyboard.c (read_char): Add call to
barf_if_interaction_inhibited. (bug#67837)
* src/lread.c (Fread_char, Fread_event, Fread_char_exclusive): Remove
call to barf_if_interaction_inhibited.
* src/minibuf.c (Fread_from_minibuffer): Remove call to
barf_if_interaction_inhibited.
(read_minibuf_noninteractive): Add call to barf_if_interaction_inhibited.
---
 src/keyboard.c | 5 ++++-
 src/lread.c    | 6 ------
 src/minibuf.c  | 5 +++--
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index 81605e75ba2..9ec39c89699 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2632,7 +2632,10 @@ read_char (int commandflag, Lisp_Object map,
       executing_kbd_macro_index++;
 
       goto from_macro;
-    }
+    } else
+    /* Getting input from a keyboard macro doesn't count as
+       interacting with the user.  */
+    barf_if_interaction_inhibited ();
 
   if (!NILP (unread_switch_frame))
     {
diff --git a/src/lread.c b/src/lread.c
index 255b6e914d9..154c751cbe9 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 ();
diff --git a/src/minibuf.c b/src/minibuf.c
index 58adde1bf66..bf21a8d9cad 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -316,6 +316,9 @@ read_minibuf_noninteractive (Lisp_Object prompt, bool expflag,
   struct emacs_tty etty;
   bool etty_valid UNINIT;
 
+  /* inhibit-interaction also prevents reading from stdin. */
+  barf_if_interaction_inhibited ();
+
   /* Check, whether we need to suppress echoing.  */
   if (CHARACTERP (Vread_hide_char))
     hide_char = XFIXNAT (Vread_hide_char);
@@ -1344,8 +1347,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;
-- 
2.39.3


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 16:50 ` Spencer Baugh
@ 2023-12-15 18:54   ` Eli Zaretskii
  2023-12-15 19:48     ` Spencer Baugh
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-12-15 18:54 UTC (permalink / raw)
  To: Spencer Baugh, Stefan Monnier; +Cc: 67837, larsi

> Cc: Lars Ingebrigtsen <larsi@gnus.org>
> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Fri, 15 Dec 2023 11:50:29 -0500
> 
> >From b0f680393991d9ccbd888be8f754a85775196799 Mon Sep 17 00:00:00 2001
> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Fri, 15 Dec 2023 11:39:24 -0500
> Subject: [PATCH] Make inhibit-interaction work properly with keyboard macros
> 
> Previously, inhibit-interaction=t prevented keyboard macros from
> running, even when those macros did not result in user interaction,
> since it was checked before the keyboard macro code had a chance to
> provide input.
> 
> Now, if there's a running keyboard macro which can provide input, that
> keyboard macro is allowed to provide input even if
> inhibit-interaction=t.  This is achieved by moving the check on
> inhibit-interaction to run after checking executing-kbd-macro in the
> low-level input handling mechanism, read_char.
> 
> inhibit-interaction also suppresses reading from stdin in batch mode,
> so we also must add a check on inhibit-interaction to
> read_minibuf_noninteractive, which again is only called after checking
> executing-kbd-macro.
> 
> * src/keyboard.c (read_char): Add call to
> barf_if_interaction_inhibited. (bug#67837)
> * src/lread.c (Fread_char, Fread_event, Fread_char_exclusive): Remove
> call to barf_if_interaction_inhibited.
> * src/minibuf.c (Fread_from_minibuffer): Remove call to
> barf_if_interaction_inhibited.
> (read_minibuf_noninteractive): Add call to barf_if_interaction_inhibited.

Please explain why you are removing the calls to
barf_if_interaction_inhibited from many functions.  It looks like they
will now do some work instead of barfing right at the beginning.  Why
is that TRT?

And I don't think I understand why we should care about a case when
inhibit-interaction is non-nil, and Emacs needs to execute a keyboard
macro, since executing keyboard macros is basically similar to
interactive invocations of commands.  What are the real-life use cases
for that?

> +    } else

This is against our style in C sources.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 18:54   ` Eli Zaretskii
@ 2023-12-15 19:48     ` Spencer Baugh
  2023-12-15 20:01       ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Spencer Baugh @ 2023-12-15 19:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 67837, Stefan Monnier

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Lars Ingebrigtsen <larsi@gnus.org>
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Fri, 15 Dec 2023 11:50:29 -0500
>> 
>> >From b0f680393991d9ccbd888be8f754a85775196799 Mon Sep 17 00:00:00 2001
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Fri, 15 Dec 2023 11:39:24 -0500
>> Subject: [PATCH] Make inhibit-interaction work properly with keyboard macros
>> 
>> Previously, inhibit-interaction=t prevented keyboard macros from
>> running, even when those macros did not result in user interaction,
>> since it was checked before the keyboard macro code had a chance to
>> provide input.
>> 
>> Now, if there's a running keyboard macro which can provide input, that
>> keyboard macro is allowed to provide input even if
>> inhibit-interaction=t.  This is achieved by moving the check on
>> inhibit-interaction to run after checking executing-kbd-macro in the
>> low-level input handling mechanism, read_char.
>> 
>> inhibit-interaction also suppresses reading from stdin in batch mode,
>> so we also must add a check on inhibit-interaction to
>> read_minibuf_noninteractive, which again is only called after checking
>> executing-kbd-macro.
>> 
>> * src/keyboard.c (read_char): Add call to
>> barf_if_interaction_inhibited. (bug#67837)
>> * src/lread.c (Fread_char, Fread_event, Fread_char_exclusive): Remove
>> call to barf_if_interaction_inhibited.
>> * src/minibuf.c (Fread_from_minibuffer): Remove call to
>> barf_if_interaction_inhibited.
>> (read_minibuf_noninteractive): Add call to barf_if_interaction_inhibited.
>
> Please explain why you are removing the calls to
> barf_if_interaction_inhibited from many functions.  It looks like they
> will now do some work instead of barfing right at the beginning.  Why
> is that TRT?

Those calls to barf_if_interaction_inhibited meant inhibit-interaction
was checked before the keyboard macro code had a chance to provide
input.

I am moving the check on inhibit-interaction to run after checking
executing-kbd-macro in the low-level input handling mechanism,
read_char.

This allows the keyboard macro is allowed to provide input even if
inhibit-interaction=t.

>
> And I don't think I understand why we should care about a case when
> inhibit-interaction is non-nil, and Emacs needs to execute a keyboard
> macro, since executing keyboard macros is basically similar to
> interactive invocations of commands.  What are the real-life use cases
> for that?

Two concrete, real-life use cases:

- Users write functions using keyboard macros and put them in hooks,
  which happen to get invoked by packages which use inhibit-interaction.
  Those functions don't actually require interaction, but because they
  break, ultimately no code can use inhibit-interaction.

- I run tests in a batch Emacs, frequently using keyboard macros to
  provide input.  Sometimes a bug causes code to run which calls
  read-char outside of a keyboard macro.  I would like such read-char
  calls to error (instead of hanging, which is what they do by default
  in batch mode).  If I bind inhibit-interaction=t, then read-char will
  exit with an error, but my keyboard macros will also immediately
  error.

>> +    } else
>
> This is against our style in C sources.

Will fix in next patch.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 19:48     ` Spencer Baugh
@ 2023-12-15 20:01       ` Eli Zaretskii
  2023-12-15 20:09         ` Spencer Baugh
  2023-12-15 20:14         ` Eli Zaretskii
  0 siblings, 2 replies; 24+ messages in thread
From: Eli Zaretskii @ 2023-12-15 20:01 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: larsi, 67837, monnier

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  67837@debbugs.gnu.org,
>    larsi@gnus.org
> Date: Fri, 15 Dec 2023 14:48:51 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Please explain why you are removing the calls to
> > barf_if_interaction_inhibited from many functions.  It looks like they
> > will now do some work instead of barfing right at the beginning.  Why
> > is that TRT?
> 
> Those calls to barf_if_interaction_inhibited meant inhibit-interaction
> was checked before the keyboard macro code had a chance to provide
> input.
> 
> I am moving the check on inhibit-interaction to run after checking
> executing-kbd-macro in the low-level input handling mechanism,
> read_char.

I'm saying that your proposal of fixing this will cause these
functions to do some parts of their jobs before they realize that they
can barf, and this will now happen even when they run not from a
keyboard macro, and even if the keyboard macro doesn't actually
provide any input.  This is definitely not TRT.  It affects use cases
completely unrelated to the ones you wanted to fix, and affects them
in adverse ways.

> This allows the keyboard macro is allowed to provide input even if
> inhibit-interaction=t.

Please find a way of fixing the case of a keyboard macro that provides
input without adversely affecting the other cases where these
functions are called with inhibit-interaction=t.

> > And I don't think I understand why we should care about a case when
> > inhibit-interaction is non-nil, and Emacs needs to execute a keyboard
> > macro, since executing keyboard macros is basically similar to
> > interactive invocations of commands.  What are the real-life use cases
> > for that?
> 
> Two concrete, real-life use cases:
> 
> - Users write functions using keyboard macros and put them in hooks,
>   which happen to get invoked by packages which use inhibit-interaction.
>   Those functions don't actually require interaction, but because they
>   break, ultimately no code can use inhibit-interaction.
> 
> - I run tests in a batch Emacs, frequently using keyboard macros to
>   provide input.  Sometimes a bug causes code to run which calls
>   read-char outside of a keyboard macro.  I would like such read-char
>   calls to error (instead of hanging, which is what they do by default
>   in batch mode).  If I bind inhibit-interaction=t, then read-char will
>   exit with an error, but my keyboard macros will also immediately
>   error.

In both cases, using a function would solve the problem.  So I'm not
convinced we need to support those marginal cases, unless you can come
up with a solution that will be both simple and will not affect
unrelated use cases.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 20:01       ` Eli Zaretskii
@ 2023-12-15 20:09         ` Spencer Baugh
  2023-12-16  7:02           ` Eli Zaretskii
  2023-12-15 20:14         ` Eli Zaretskii
  1 sibling, 1 reply; 24+ messages in thread
From: Spencer Baugh @ 2023-12-15 20:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 67837, monnier

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  67837@debbugs.gnu.org,
>>    larsi@gnus.org
>> Date: Fri, 15 Dec 2023 14:48:51 -0500
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Please explain why you are removing the calls to
>> > barf_if_interaction_inhibited from many functions.  It looks like they
>> > will now do some work instead of barfing right at the beginning.  Why
>> > is that TRT?
>> 
>> Those calls to barf_if_interaction_inhibited meant inhibit-interaction
>> was checked before the keyboard macro code had a chance to provide
>> input.
>> 
>> I am moving the check on inhibit-interaction to run after checking
>> executing-kbd-macro in the low-level input handling mechanism,
>> read_char.
>
> I'm saying that your proposal of fixing this will cause these
> functions to do some parts of their jobs before they realize that they
> can barf, and this will now happen even when they run not from a
> keyboard macro, and even if the keyboard macro doesn't actually
> provide any input.  This is definitely not TRT.  It affects use cases
> completely unrelated to the ones you wanted to fix, and affects them
> in adverse ways.

I think the effects on other use cases are only positive.  If, for
example, read-char would fail due to reasons other than
inhibit-interaction, it will now fail for those reasons.  Which is good,
because it reduces the need for all code everywhere to think about the
possibility that inhibit-interaction is non-nil.

>> This allows the keyboard macro is allowed to provide input even if
>> inhibit-interaction=t.
>
> Please find a way of fixing the case of a keyboard macro that provides
> input without adversely affecting the other cases where these
> functions are called with inhibit-interaction=t.

How about if those original barf_if_interaction_inhibited calls only
signal if executing-kbd-macro is nil?

>> > And I don't think I understand why we should care about a case when
>> > inhibit-interaction is non-nil, and Emacs needs to execute a keyboard
>> > macro, since executing keyboard macros is basically similar to
>> > interactive invocations of commands.  What are the real-life use cases
>> > for that?
>> 
>> Two concrete, real-life use cases:
>> 
>> - Users write functions using keyboard macros and put them in hooks,
>>   which happen to get invoked by packages which use inhibit-interaction.
>>   Those functions don't actually require interaction, but because they
>>   break, ultimately no code can use inhibit-interaction.
>> 
>> - I run tests in a batch Emacs, frequently using keyboard macros to
>>   provide input.  Sometimes a bug causes code to run which calls
>>   read-char outside of a keyboard macro.  I would like such read-char
>>   calls to error (instead of hanging, which is what they do by default
>>   in batch mode).  If I bind inhibit-interaction=t, then read-char will
>>   exit with an error, but my keyboard macros will also immediately
>>   error.
>
> In both cases, using a function would solve the problem.  So I'm not
> convinced we need to support those marginal cases, unless you can come
> up with a solution that will be both simple and will not affect
> unrelated use cases.

- Are you suggesting that novice users should have to rewrite all their
  keyboard macros in Lisp?  That sounds impractical.

- How can I provide keyboard input to the interactive spec of a command
  I am testing, other than by using keyboard macros?  I'd be pleased to
  have an alternative solution.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 20:01       ` Eli Zaretskii
  2023-12-15 20:09         ` Spencer Baugh
@ 2023-12-15 20:14         ` Eli Zaretskii
  2023-12-15 20:39           ` Spencer Baugh
  2023-12-16 15:52           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 2 replies; 24+ messages in thread
From: Eli Zaretskii @ 2023-12-15 20:14 UTC (permalink / raw)
  To: sbaugh; +Cc: larsi, 67837, monnier

> Cc: larsi@gnus.org, 67837@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Fri, 15 Dec 2023 22:01:01 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > This allows the keyboard macro is allowed to provide input even if
> > inhibit-interaction=t.
> 
> Please find a way of fixing the case of a keyboard macro that provides
> input without adversely affecting the other cases where these
> functions are called with inhibit-interaction=t.

I'm actually tend to think that this proposal is fundamentally wrong,
not just problematic implementation-wise.  Providing input from a
keyboard macro is still input, and inhibit-interaction=t means asking
for input signals an error.  So your suggestion subverts this feature,
and therefore it is simply wrong to install something like that.

IOW, signaling an error in these cases is exactly TRT, and we should
not let keyboard macros circumvent this mechanism.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 20:14         ` Eli Zaretskii
@ 2023-12-15 20:39           ` Spencer Baugh
  2023-12-16  7:14             ` Eli Zaretskii
  2023-12-16 15:52           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 24+ messages in thread
From: Spencer Baugh @ 2023-12-15 20:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 67837, monnier

Eli Zaretskii <eliz@gnu.org> writes:
>> Cc: larsi@gnus.org, 67837@debbugs.gnu.org, monnier@iro.umontreal.ca
>> Date: Fri, 15 Dec 2023 22:01:01 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>> > This allows the keyboard macro is allowed to provide input even if
>> > inhibit-interaction=t.
>> 
>> Please find a way of fixing the case of a keyboard macro that provides
>> input without adversely affecting the other cases where these
>> functions are called with inhibit-interaction=t.
>
> I'm actually tend to think that this proposal is fundamentally wrong,
> not just problematic implementation-wise.  Providing input from a
> keyboard macro is still input, and inhibit-interaction=t means asking
> for input signals an error.  So your suggestion subverts this feature,
> and therefore it is simply wrong to install something like that.
>
> IOW, signaling an error in these cases is exactly TRT, and we should
> not let keyboard macros circumvent this mechanism.

If that's the case, then could we add another variable which does behave
like I propose with these patches?  That is, it allows input from
keyboard macros, but not from the user?  That is personally what I want
in my packages, because it doesn't break users who use keyboard macros,
and it supports my use case of making read-char error in batch mode.

Or perhaps, another possible value of inhibit-interaction, which behaves
like that?

BTW, I'm curious to hear what Lars thinks, because I expect that
"keyboard macros should not work within inhibit-interaction=t" was not
at all part of the plan.

Although, I guess with my change, a keyboard macro which calls a
function which internally sets inhibit-interaction=t will effectively
ignore the inhibit-interaction setting.  Which is definitely not
correct.

The correct behavior is a bit subtle; also important to consider are
kbd-macro-query and recursive-edit.

Here are some nesting situations, and what I suggest read-char should do
in each of them:

kmacro: get kmacro input
i-i=t: error
i-i=t, then kmacro: get kmacro input
kmacro, then i-i=t: error
i-i=t, kmacro, i-i=t: error
kmacro, i-i=t, kmacro: get inner kmacro input
kmacro, recursive-edit: get user input
i-i=t, recursive-edit: error
i-i=t, kmacro, recursive-edit: error
kmacro, i-i=t, recursive-edit: error

So basically, i-i=t means any request for user input should fail, which
my change achieves; but also, if i-i=t was bound *after* the kmacro,
then any request for kmacro input should also fail.  Not sure how to
achieve that.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 20:09         ` Spencer Baugh
@ 2023-12-16  7:02           ` Eli Zaretskii
  2023-12-16 13:22             ` sbaugh
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-12-16  7:02 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: larsi, 67837, monnier

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: larsi@gnus.org,  67837@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Fri, 15 Dec 2023 15:09:59 -0500
> 
> > I'm saying that your proposal of fixing this will cause these
> > functions to do some parts of their jobs before they realize that they
> > can barf, and this will now happen even when they run not from a
> > keyboard macro, and even if the keyboard macro doesn't actually
> > provide any input.  This is definitely not TRT.  It affects use cases
> > completely unrelated to the ones you wanted to fix, and affects them
> > in adverse ways.
> 
> I think the effects on other use cases are only positive.  If, for
> example, read-char would fail due to reasons other than
> inhibit-interaction, it will now fail for those reasons.  Which is good,
> because it reduces the need for all code everywhere to think about the
> possibility that inhibit-interaction is non-nil.

Most calls don't signal errors, so the part that is important is when
they don't.

> > Please find a way of fixing the case of a keyboard macro that provides
> > input without adversely affecting the other cases where these
> > functions are called with inhibit-interaction=t.
> 
> How about if those original barf_if_interaction_inhibited calls only
> signal if executing-kbd-macro is nil?

We could perhaps do that, but see my other message: I think it is
fundamentally wrong to allow keyboard macros be exempt from this
feature.

> >> - Users write functions using keyboard macros and put them in hooks,
> >>   which happen to get invoked by packages which use inhibit-interaction.
> >>   Those functions don't actually require interaction, but because they
> >>   break, ultimately no code can use inhibit-interaction.
> >> 
> >> - I run tests in a batch Emacs, frequently using keyboard macros to
> >>   provide input.  Sometimes a bug causes code to run which calls
> >>   read-char outside of a keyboard macro.  I would like such read-char
> >>   calls to error (instead of hanging, which is what they do by default
> >>   in batch mode).  If I bind inhibit-interaction=t, then read-char will
> >>   exit with an error, but my keyboard macros will also immediately
> >>   error.
> >
> > In both cases, using a function would solve the problem.  So I'm not
> > convinced we need to support those marginal cases, unless you can come
> > up with a solution that will be both simple and will not affect
> > unrelated use cases.
> 
> - Are you suggesting that novice users should have to rewrite all their
>   keyboard macros in Lisp?  That sounds impractical.

I don't see anything impractical here.

> - How can I provide keyboard input to the interactive spec of a command
>   I am testing, other than by using keyboard macros?  I'd be pleased to
>   have an alternative solution.

Why do you need to do that when inhibit-interaction is non-nil in the
first place?  Code that needs interaction shouldn't be run or tested
in those conditions.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 20:39           ` Spencer Baugh
@ 2023-12-16  7:14             ` Eli Zaretskii
  0 siblings, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2023-12-16  7:14 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: larsi, 67837, monnier

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: larsi@gnus.org,  67837@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Fri, 15 Dec 2023 15:39:31 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >
> > I'm actually tend to think that this proposal is fundamentally wrong,
> > not just problematic implementation-wise.  Providing input from a
> > keyboard macro is still input, and inhibit-interaction=t means asking
> > for input signals an error.  So your suggestion subverts this feature,
> > and therefore it is simply wrong to install something like that.
> >
> > IOW, signaling an error in these cases is exactly TRT, and we should
> > not let keyboard macros circumvent this mechanism.
> 
> If that's the case, then could we add another variable which does behave
> like I propose with these patches?

Why would we want to do that?

The inhibit-interaction variable was introduced for very special
circumstances, and its purpose is clear: signal an error when any user
interaction is requested.  To introduce some kind of override of that
behavior, in those situations where some Lisp program binds
inhibit-interaction non-nil, would require serious justifications,
since the easiest way of avoiding these problems is either not to bind
inhibit-interaction non-nil in the first place, or provide a signal
handler that will catch the error and DTRT.  Emacs itself never sets
this variable non-nil, it's entirely up to Lisp programs to use it.
And Lisp programs that do bind it actually mean for interactions to
signal an error, so making exceptions from that requires very good
reasons.  I don't think you presented such reasons; the use cases you
described are frankly quite obscure, and can have other solutions.

So I'm against complicating this feature that is currently very simple
and understandable, and also not used widely enough for us to bother
about such contrived circumstances, not enough for non-trivial
internal changes anyway.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-16  7:02           ` Eli Zaretskii
@ 2023-12-16 13:22             ` sbaugh
  2023-12-16 13:57               ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: sbaugh @ 2023-12-16 13:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Spencer Baugh, larsi, 67837, monnier

Eli Zaretskii <eliz@gnu.org> writes:
>> >> - Users write functions using keyboard macros and put them in hooks,
>> >>   which happen to get invoked by packages which use inhibit-interaction.
>> >>   Those functions don't actually require interaction, but because they
>> >>   break, ultimately no code can use inhibit-interaction.
>> >> 
>> >> - I run tests in a batch Emacs, frequently using keyboard macros to
>> >>   provide input.  Sometimes a bug causes code to run which calls
>> >>   read-char outside of a keyboard macro.  I would like such read-char
>> >>   calls to error (instead of hanging, which is what they do by default
>> >>   in batch mode).  If I bind inhibit-interaction=t, then read-char will
>> >>   exit with an error, but my keyboard macros will also immediately
>> >>   error.
>> >
>> > In both cases, using a function would solve the problem.  So I'm not
>> > convinced we need to support those marginal cases, unless you can come
>> > up with a solution that will be both simple and will not affect
>> > unrelated use cases.
>> 
>> - Are you suggesting that novice users should have to rewrite all their
>>   keyboard macros in Lisp?  That sounds impractical.
>
> I don't see anything impractical here.

Many users of Emacs are not programmers.  They are able to use keyboard
macros as a simple, non-programming way to make reusable functions and
commands.  Are you saying they should learn to program so that they can
rewrite their keyboard macros by hand into Lisp?

>> - How can I provide keyboard input to the interactive spec of a command
>>   I am testing, other than by using keyboard macros?  I'd be pleased to
>>   have an alternative solution.
>
> Why do you need to do that when inhibit-interaction is non-nil in the
> first place?  Code that needs interaction shouldn't be run or tested
> in those conditions.

As I said before: because otherwise read-char outside a keyboard macro
will hang, and I want my test to fail instead of hang in that case.

Let me phrase the use case differently:

I have some tests which I'd like to run in batch Emacs.  By default, if
any of the code under test runs read-char, Emacs will simply hang
forever (that's what read-char does in batch mode).

I would prefer instead that my tests to fail immediately if any code
runs read-char.  How would you suggest I do that?

AFAIK the only way to achieve this currently is inhibit-interaction,
although I'd be happy to add an alternative way to do that.

So, to achieve this I'll use inhibit-interaction=t over my entire test
suite.  But then tests which make any use of a keyboard macro, for
testing interactive specs for example, will fail!





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-16 13:22             ` sbaugh
@ 2023-12-16 13:57               ` Eli Zaretskii
  0 siblings, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2023-12-16 13:57 UTC (permalink / raw)
  To: sbaugh; +Cc: sbaugh, larsi, 67837, monnier

> From: sbaugh@catern.com
> Date: Sat, 16 Dec 2023 13:22:23 +0000 (UTC)
> Cc: Spencer Baugh <sbaugh@janestreet.com>, larsi@gnus.org,
> 	67837@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Why do you need to do that when inhibit-interaction is non-nil in the
> > first place?  Code that needs interaction shouldn't be run or tested
> > in those conditions.
> 
> As I said before: because otherwise read-char outside a keyboard macro
> will hang, and I want my test to fail instead of hang in that case.

The usual way of dealing with this in test frameworks is to mock
read-char (or whatever function that causes trouble when running
tests).  As long as read-char is not the API you are testing, you
don't really need read-char as it works in Emacs, you need something
that will provide the same output.

> Let me phrase the use case differently:
> 
> I have some tests which I'd like to run in batch Emacs.  By default, if
> any of the code under test runs read-char, Emacs will simply hang
> forever (that's what read-char does in batch mode).
> 
> I would prefer instead that my tests to fail immediately if any code
> runs read-char.  How would you suggest I do that?

Define a replacement read-char in the test harness that accepts the
same arguments and signals an error.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-15 20:14         ` Eli Zaretskii
  2023-12-15 20:39           ` Spencer Baugh
@ 2023-12-16 15:52           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-16 16:08             ` Eli Zaretskii
  2024-02-16 22:26             ` Spencer Baugh
  1 sibling, 2 replies; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-12-16 15:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, larsi, control, 67837

merge 67837 65291
thanks

AFAICT this is the same bug as bug#65291 and the suggested patch is similar.

> I'm actually tend to think that this proposal is fundamentally wrong,
> not just problematic implementation-wise.  Providing input from a
> keyboard macro is still input, and inhibit-interaction=t means asking
> for input signals an error.  So your suggestion subverts this feature,
> and therefore it is simply wrong to install something like that.

I guess it begs the question: what is the purpose of
`inhibit-interaction`?

The way I see it, the purpose is to avoid Emacs waiting for user input
when we know there's no user, and thus signal an error if we ever get to
this point.

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).

Note that trying to make the whole test suite runs with
`inhibit-interaction` non-nil is not at all straightforward, sadly:
there are several places where we do call things like `read-event`
without providing any keyboard input (i.e. without
`unread-command-event` or keyboard macros) and instead use a timeout
because this `read-event` is just there to force Emacs to wait while
some external process sends us some reply.  Should these be considered
"interaction"?  If not, then we open up a whole where some code may call
`read-event` with a relatively short timeout within a tight loop where
the purpose *is* to get user input and where the timeout is only present
to keep something else updated while we wait for that user's input.


        Stefan






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-16 15:52           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-12-16 16:08             ` Eli Zaretskii
  2023-12-16 17:18               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-16 22:26             ` Spencer Baugh
  1 sibling, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2023-12-16 16:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: sbaugh, larsi, control, 67837

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: sbaugh@janestreet.com,  larsi@gnus.org,  67837@debbugs.gnu.org,
>  control@debbugs.gnu.org
> Date: Sat, 16 Dec 2023 10:52:45 -0500
> 
> merge 67837 65291
> thanks
> 
> AFAICT this is the same bug as bug#65291 and the suggested patch is similar.
> 
> > I'm actually tend to think that this proposal is fundamentally wrong,
> > not just problematic implementation-wise.  Providing input from a
> > keyboard macro is still input, and inhibit-interaction=t means asking
> > for input signals an error.  So your suggestion subverts this feature,
> > and therefore it is simply wrong to install something like that.
> 
> I guess it begs the question: what is the purpose of
> `inhibit-interaction`?
> 
> The way I see it, the purpose is to avoid Emacs waiting for user input
> when we know there's no user, and thus signal an error if we ever get to
> this point.

I agree.  And signaling an error could be useful for either aborting
some code which shouldn't interact with the user, or for catching the
error and doing something alternative to user interaction.  In the
latter case, disabling the error can actually break some code.

> 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).

In general, yes.  But the test suite can also be run interactively,
and sometimes the ability to do so is very important for investigating
test failures.

> Note that trying to make the whole test suite runs with
> `inhibit-interaction` non-nil is not at all straightforward, sadly:
> there are several places where we do call things like `read-event`
> without providing any keyboard input (i.e. without
> `unread-command-event` or keyboard macros) and instead use a timeout
> because this `read-event` is just there to force Emacs to wait while
> some external process sends us some reply.  Should these be considered
> "interaction"?  If not, then we open up a whole where some code may call
> `read-event` with a relatively short timeout within a tight loop where
> the purpose *is* to get user input and where the timeout is only present
> to keep something else updated while we wait for that user's input.

I see no reason to insist that everything in the test suite _must_ be
runnable with inhibit-interaction non-nil.  The only purpose of the
test suite is to test whatever each test is testing, there are no
other requirements.  The code could be not very clean; if it does the
job, that is fine from where I stand.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-16 16:08             ` Eli Zaretskii
@ 2023-12-16 17:18               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-12-16 17:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, larsi, control, 67837

>> 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).
> In general, yes.  But the test suite can also be run interactively,

That's fine.  My goal is to bind it in `ert-run-tests-batch-and-exit` or
as close to that as possible.

>> Note that trying to make the whole test suite runs with
>> `inhibit-interaction` non-nil is not at all straightforward, sadly:
>> there are several places where we do call things like `read-event`
>> without providing any keyboard input (i.e. without
>> `unread-command-event` or keyboard macros) and instead use a timeout
>> because this `read-event` is just there to force Emacs to wait while
>> some external process sends us some reply.  Should these be considered
>> "interaction"?  If not, then we open up a whole where some code may call
>> `read-event` with a relatively short timeout within a tight loop where
>> the purpose *is* to get user input and where the timeout is only present
>> to keep something else updated while we wait for that user's input.
>
> I see no reason to insist that everything in the test suite _must_ be
> runnable with inhibit-interaction non-nil.

As mentioned, my motivation is to better handle tests that hang instead
of failing.  It's hard to know beforehand which ones of those tests
will/may do that.  Also, where could we let-bind `inhibit-interaction`
such that it only affects those tests we decide need it?

> The only purpose of the test suite is to test whatever each test is
> testing, there are no other requirements.  The code could be not very
> clean; if it does the job, that is fine from where I stand.

That's my opinion as well, and in my opinion `inhibit-interaction` is
mostly meant for tests, so in my current local patch that tries to make
it work for the whole test suite, I made that variable fairly lenient
(it doesn't signal an error if you `read-event` with a timeout).


        Stefan






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2023-12-16 15:52           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-16 16:08             ` Eli Zaretskii
@ 2024-02-16 22:26             ` Spencer Baugh
  2024-02-16 23:27               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-17  7:37               ` Eli Zaretskii
  1 sibling, 2 replies; 24+ messages in thread
From: Spencer Baugh @ 2024-02-16 22:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 67837, Eli Zaretskii, larsi

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> merge 67837 65291
> thanks
>
> AFAICT this is the same bug as bug#65291 and the suggested patch is similar.
>
>> I'm actually tend to think that this proposal is fundamentally wrong,
>> not just problematic implementation-wise.  Providing input from a
>> keyboard macro is still input, and inhibit-interaction=t means asking
>> for input signals an error.  So your suggestion subverts this feature,
>> and therefore it is simply wrong to install something like that.
>
> I guess it begs the question: what is the purpose of
> `inhibit-interaction`?
>
> The way I see it, the purpose is to avoid Emacs waiting for user input
> when we know there's no user, and thus signal an error if we ever get to
> this point.
>
> 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)

> Note that trying to make the whole test suite runs with
> `inhibit-interaction` non-nil is not at all straightforward, sadly:
> there are several places where we do call things like `read-event`
> without providing any keyboard input (i.e. without
> `unread-command-event` or keyboard macros) and instead use a timeout
> because this `read-event` is just there to force Emacs to wait while
> some external process sends us some reply.  Should these be considered
> "interaction"?  If not, then we open up a whole where some code may call
> `read-event` with a relatively short timeout within a tight loop where
> the purpose *is* to get user input and where the timeout is only present
> to keep something else updated while we wait for that user's input.

What places are these?

I think that does sound like interaction to me.  In which case, could we
change those places to not use read-event?  Those places would break
anyway if they ran inside a user-defined keyboard macro, if I understand
correctly, so it's useful to fix them.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2024-02-16 22:26             ` Spencer Baugh
@ 2024-02-16 23:27               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-17  7:53                 ` Eli Zaretskii
  2024-02-17  7:37               ` Eli Zaretskii
  1 sibling, 1 reply; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-16 23:27 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: 67837, Eli Zaretskii, larsi

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

>> 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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: inhibit-interaction.patch --]
[-- Type: text/x-diff, Size: 13152 bytes --]

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;
 }
 \f
-/* 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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2024-02-16 22:26             ` Spencer Baugh
  2024-02-16 23:27               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-17  7:37               ` Eli Zaretskii
  1 sibling, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2024-02-17  7:37 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: 67837, larsi, monnier

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  larsi@gnus.org,  67837@debbugs.gnu.org
> Date: Fri, 16 Feb 2024 17:26:45 -0500
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > merge 67837 65291
> > thanks
> >
> > AFAICT this is the same bug as bug#65291 and the suggested patch is similar.

Alas, bug#65291 doesn't have any justification for the change, so I
agree that they should be merged, but doing so doesn't add any useful
information to this discussion, IMO.

> >> I'm actually tend to think that this proposal is fundamentally wrong,
> >> not just problematic implementation-wise.  Providing input from a
> >> keyboard macro is still input, and inhibit-interaction=t means asking
> >> for input signals an error.  So your suggestion subverts this feature,
> >> and therefore it is simply wrong to install something like that.
> >
> > I guess it begs the question: what is the purpose of
> > `inhibit-interaction`?
> >
> > The way I see it, the purpose is to avoid Emacs waiting for user input
> > when we know there's no user, and thus signal an error if we ever get to
> > this point.
> >
> > 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)

And I'm still concerned that making these changes will be an
incompatible change, because the functions that signaled the error
right at the beginning will now do part of their job before signaling,
and that could affect the net result of calling them in those cases.

> > Note that trying to make the whole test suite runs with
> > `inhibit-interaction` non-nil is not at all straightforward, sadly:
> > there are several places where we do call things like `read-event`
> > without providing any keyboard input (i.e. without
> > `unread-command-event` or keyboard macros) and instead use a timeout
> > because this `read-event` is just there to force Emacs to wait while
> > some external process sends us some reply.  Should these be considered
> > "interaction"?  If not, then we open up a whole where some code may call
> > `read-event` with a relatively short timeout within a tight loop where
> > the purpose *is* to get user input and where the timeout is only present
> > to keep something else updated while we wait for that user's input.
> 
> What places are these?
> 
> I think that does sound like interaction to me.  In which case, could we
> change those places to not use read-event?  Those places would break
> anyway if they ran inside a user-defined keyboard macro, if I understand
> correctly, so it's useful to fix them.

Btw, I think calling read-event with a timeout should not signal an
error when interaction-inhibited is non-nil.

But more generally, the above issues with our test suite are examples
of non-trivial code which could be affected by the proposed changes.
The justifications for these changes that were presented until now,
such as they are, are very weak, and I therefore still object to
making them, because the use cases where those changes are supposed to
be useful can be taken care of by other means, like API mocking.

From where I stand, this is another example of taking marginal obscure
use cases and promoting them to the level of a general problem that
needs significant low-level changes whose effect is
backward-incompatible behavior changes.  Instead, those use cases
should be handled with other techniques already available in Emacs
that don't require any changes to our infrastructure.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2024-02-16 23:27               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-17  7:53                 ` Eli Zaretskii
  2024-02-17 14:13                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2024-02-17  7:53 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: sbaugh, larsi, 67837

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  larsi@gnus.org,  67837@debbugs.gnu.org
> Date: Fri, 16 Feb 2024 18:27:44 -0500
> 
> IIUC, Eli is opposed to changing the behavior of `inhibit-interaction`,

I'm mostly opposed to making this kind of changes for reasons that are
very weak, and basically are based on a special use case Spencer
bumped into in his practice, a use case that can be resolved in a
different way without any changes.  I'm talking about Spencer's code
which uses keyboard macros for mocking user input.  I've seen no other
justification for these behavior changes.

> 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.

Frankly, I don't understand the underlying ideas here.  And it seems
to include unrelated change(s)?  E.g., what is this about:

> +  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");

?

> 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)))))

So we are supposed to replace all calls to read-event in our test
suite with sit-for, and to be able to do that, we are now changing how
sit-for behaves in non-interactive sessions?  That sounds like a very
serious incompatible change in behavior for a very weak reason -- the
possibility to run the test suite with inhibit-interaction non-nil.  I
sincerely hope we will not make such changes for such reasons.  If we
do, we shouldn't be surprised that Emacs' stability does not improve
with newer releases.

With all due respect to our test suite, let's not forget that its main
purpose is to allow us to test the various Emacs features.  How we do
that and whether we do it cleanly or not is completely unimportant, as
long as the features get tested and tested well.  It follows that the
needs of the test suite should generally not require any
infrastructure changes in Emacs; instead, we should jump through as
many hoops as needed to test Emacs as it is.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2024-02-17  7:53                 ` Eli Zaretskii
@ 2024-02-17 14:13                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-17 14:35                     ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-17 14:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, larsi, 67837

> I'm mostly opposed to making this kind of changes for reasons that are
> very weak, and basically are based on a special use case Spencer
> bumped into in his practice, a use case that can be resolved in a
> different way without any changes.

I can't remember what Spencer's use case was, but for me the use case
was simply that I was getting tired of having to kill Emacs processes
that were hanging waiting for input while running the test suite.

The specific reason why Emacs was waiting for input varied, it was
usually linked to some bug I had introduced.

AFAIK, `inhibit-interaction` is meant for those kinds of circumstances,
but currently it can't be used for that because it basically rules out
tests that have to wait for some async operation.

You say:

    And I'm still concerned that making these changes will be an
    incompatible change, because the functions that signaled the error
    right at the beginning will now do part of their job before
    signaling, and that could affect the net result of calling them in
    those cases.

Why are you concerned?  Which code do you think will break?
More specifically, have you ever seen or heard about a piece of code
using `inhibit-interaction`?  I have not, *except* in the context of
bugs like this one.  IOW, AFAICT, `inhibit-interaction` fails at
providing the only feature for which it's useful.

> And it seems to include unrelated change(s)?  E.g., what is this
> about:
>
>> +  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");
>
> ?

My changes are WiP in my big-hunk-of-unrelated changes.
So yes, I failed to excise every bit of the unrelated changes.
[ FWIW, the above change is one I couldn't remember I had in my tree,
  and while I vaguely remember making it, I can't remember what it was
  for. 🙂  ]

They're meant for Spencer or whoever else wants to work on this.
They're definitely not meant to be anywhere near ready for inclusion.
I can't even remember if I had convinced myself that this was the right
way to go about it.  Its main quality is that it pretty much works, so
it's a useful starting point.

>> 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)))))
>
> So we are supposed to replace all calls to read-event in our test
> suite with sit-for, and to be able to do that, we are now changing how
> sit-for behaves in non-interactive sessions?

Can't remember why I did it this way, TBH.  Apparently using
`read-event` was a problem I bumped into along the way.  I probably made
or kept that change thinking it's better anyway because it more clearly
reflects the intention.

> That sounds like a very serious incompatible change in behavior for
> a very weak reason -- the possibility to run the test suite with
> inhibit-interaction non-nil.

IIUC you're talking about the `sit-for` change to not use `sleep-for`?
Yes, it's an incompatible change.  I think I made it thinking that it's
probably a good idea anyway.

We have a kind of a mess when it comes to waiting for external events:
if you look at various pieces of code that have to do such waits, you'll
tend to see that they all do it a bit differently, and if you look at
the surrounding comments and Git history, you'll tend to see that it's
the result of frustrating trial-and-failure bumping into various corner
cases.  The fact that `sit-for` currently allows async processes in
interactive mode but not in batch mode is probably one of the sources of
such problems.

> I sincerely hope we will not make such changes for such reasons.

We'll cross that bridge when we get there.  Right now, I just sent
Spencer a patch that kinds of summarizes what I've learned when I looked
at that problem.  If/when Spencer comes back with a patch he thinks is
appropriate for `master` and it includes such changes, we'll
discuss it then.

> With all due respect to our test suite, let's not forget that its main
> purpose is to allow us to test the various Emacs features.

That doesn't mean that problems encountered while writing the test suite
(e.g. the fact that `sit-for` doesn't read from async processes when
used in batch) can't reflect real problems that also affect real code
and thus deserve changes in Emacs proper.


        Stefan






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2024-02-17 14:13                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-17 14:35                     ` Eli Zaretskii
  2024-02-17 14:43                       ` Eli Zaretskii
  2024-02-17 15:15                       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 24+ messages in thread
From: Eli Zaretskii @ 2024-02-17 14:35 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: sbaugh, larsi, 67837

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: sbaugh@janestreet.com,  larsi@gnus.org,  67837@debbugs.gnu.org
> Date: Sat, 17 Feb 2024 09:13:07 -0500
> 
> > I'm mostly opposed to making this kind of changes for reasons that are
> > very weak, and basically are based on a special use case Spencer
> > bumped into in his practice, a use case that can be resolved in a
> > different way without any changes.
> 
> I can't remember what Spencer's use case was, but for me the use case
> was simply that I was getting tired of having to kill Emacs processes
> that were hanging waiting for input while running the test suite.

?? I bump into this from time to time, but a simple Ctrl-C usually
solves the problem on the spot.

>     And I'm still concerned that making these changes will be an
>     incompatible change, because the functions that signaled the error
>     right at the beginning will now do part of their job before
>     signaling, and that could affect the net result of calling them in
>     those cases.
> 
> Why are you concerned?  Which code do you think will break?

The code which doesn't expect some of the jobs of those functions to
be done when they eventually signal an error.

> More specifically, have you ever seen or heard about a piece of code
> using `inhibit-interaction`?  I have not, *except* in the context of
> bugs like this one.  IOW, AFAICT, `inhibit-interaction` fails at
> providing the only feature for which it's useful.

See bug#13697, which was the motivation for introducing this variable.
It mentions a couple of use cases, bug#6567 and bug#25214.

> We have a kind of a mess when it comes to waiting for external events:
> if you look at various pieces of code that have to do such waits, you'll
> tend to see that they all do it a bit differently, and if you look at
> the surrounding comments and Git history, you'll tend to see that it's
> the result of frustrating trial-and-failure bumping into various corner
> cases.  The fact that `sit-for` currently allows async processes in
> interactive mode but not in batch mode is probably one of the sources of
> such problems.

I agree that we have a mess, but disagree that we should try to fix
it.  It's a mess we have been living with for decades, and I don't
doubt that gobs of programs out there depend on this mess.

> > With all due respect to our test suite, let's not forget that its main
> > purpose is to allow us to test the various Emacs features.
> 
> That doesn't mean that problems encountered while writing the test suite
> (e.g. the fact that `sit-for` doesn't read from async processes when
> used in batch) can't reflect real problems that also affect real code
> and thus deserve changes in Emacs proper.

Yes, but we need to be able to define those real problems.  Just the
fact that something interferes with the test suite itself is not
enough.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2024-02-17 14:35                     ` Eli Zaretskii
@ 2024-02-17 14:43                       ` Eli Zaretskii
  2024-02-17 15:15                       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2024-02-17 14:43 UTC (permalink / raw)
  To: monnier, sbaugh; +Cc: larsi, 67837

> Cc: sbaugh@janestreet.com, larsi@gnus.org, 67837@debbugs.gnu.org
> Date: Sat, 17 Feb 2024 16:35:42 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > More specifically, have you ever seen or heard about a piece of code
> > using `inhibit-interaction`?  I have not, *except* in the context of
> > bugs like this one.  IOW, AFAICT, `inhibit-interaction` fails at
> > providing the only feature for which it's useful.
> 
> See bug#13697, which was the motivation for introducing this variable.
> It mentions a couple of use cases, bug#6567 and bug#25214.

Btw, I'm perfectly fine if we want to decide that the
inhibit-interaction experiment failed, and remove it from Emacs.  If
you are right, and no one uses it out there yet, doing so will not
cause any harm.  We could then reopen the above bug, and either close
it as wontfix or wait for a better solution to present itself.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2024-02-17 14:35                     ` Eli Zaretskii
  2024-02-17 14:43                       ` Eli Zaretskii
@ 2024-02-17 15:15                       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-17 16:07                         ` Eli Zaretskii
  1 sibling, 1 reply; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-17 15:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, larsi, 67837

>> I can't remember what Spencer's use case was, but for me the use case
>> was simply that I was getting tired of having to kill Emacs processes
>> that were hanging waiting for input while running the test suite.
> ?? I bump into this from time to time, but a simple Ctrl-C usually
> solves the problem on the spot.

That works if you started the test suite directly from the command line
"in the foreground", but I usually run it in the background detached from
the tty.
Also, there's the problem of having to decide whether it's hanging
or whether it's just slow.
Also, killing the hung Emacs process allows further tests to be run,
whereas `C-c` would interrupt the whole testsuite.
And of course, there's also the CI case where there really is noone
to hit `C-c`.

>> Why are you concerned?  Which code do you think will break?
> The code which doesn't expect some of the jobs of those functions to
> be done when they eventually signal an error.

Sorry, it's still too vague for me to understand what kind of scenario
you're thinking if.

For me, when we hit this error the only important thing to do is to
report the problem and stop what we're doing.  Rather than signal a Lisp
error, I'd be OK with exiting the process.  So the details of exactly
when&where the error is signaled don't seem very relevant.

>> More specifically, have you ever seen or heard about a piece of code
>> using `inhibit-interaction`?  I have not, *except* in the context of
>> bugs like this one.  IOW, AFAICT, `inhibit-interaction` fails at
>> providing the only feature for which it's useful.
> See bug#13697, which was the motivation for introducing this variable.
> It mentions a couple of use cases, bug#6567 and bug#25214.

Thanks.  I don't see anything in these examples that shows they depend
on the specific way the code currently works.  Maybe for `bug#25214` we
wouldn't want to exit the process altogether, but for everything else,
the use case seems pretty much exactly the same as mine, with the same
overarching goal: report the problem and stop what we're doing.

> I agree that we have a mess, but disagree that we should try to fix
> it.  It's a mess we have been living with for decades, and I don't
> doubt that gobs of programs out there depend on this mess.

Should we rename Emacs-29.2 as Emacs-final, then?  🙂

>> > With all due respect to our test suite, let's not forget that its main
>> > purpose is to allow us to test the various Emacs features.
>> That doesn't mean that problems encountered while writing the test suite
>> (e.g. the fact that `sit-for` doesn't read from async processes when
>> used in batch) can't reflect real problems that also affect real code
>> and thus deserve changes in Emacs proper.
> Yes, but we need to be able to define those real problems.

Of course.  That's why I've been sitting on this `sit-for` change
without reporting the problem, waiting to find a more real case
where it bites us.


        Stefan






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros
  2024-02-17 15:15                       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-17 16:07                         ` Eli Zaretskii
  0 siblings, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2024-02-17 16:07 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: sbaugh, larsi, 67837

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: sbaugh@janestreet.com,  larsi@gnus.org,  67837@debbugs.gnu.org
> Date: Sat, 17 Feb 2024 10:15:39 -0500
> 
> >> I can't remember what Spencer's use case was, but for me the use case
> >> was simply that I was getting tired of having to kill Emacs processes
> >> that were hanging waiting for input while running the test suite.
> > ?? I bump into this from time to time, but a simple Ctrl-C usually
> > solves the problem on the spot.
> 
> That works if you started the test suite directly from the command line
> "in the foreground", but I usually run it in the background detached from
> the tty.

<Rolls the eyes> So now we need to pay for the strange ways you run
the test suite?

> Also, there's the problem of having to decide whether it's hanging
> or whether it's just slow.

That is never a problem for me: just look at the "top" display or
similar.

> Also, killing the hung Emacs process allows further tests to be run,
> whereas `C-c` would interrupt the whole testsuite.

Not necessarily.  IME, it usually interrupts only the test that's
being run.

> And of course, there's also the CI case where there really is noone
> to hit `C-c`.

Maybe we should ask Michael, but AFAIK CI frameworks are prepared to
deal with such contingencies.

> >> Why are you concerned?  Which code do you think will break?
> > The code which doesn't expect some of the jobs of those functions to
> > be done when they eventually signal an error.
> 
> Sorry, it's still too vague for me to understand what kind of scenario
> you're thinking if.
> 
> For me, when we hit this error the only important thing to do is to
> report the problem and stop what we're doing.  Rather than signal a Lisp
> error, I'd be OK with exiting the process.  So the details of exactly
> when&where the error is signaled don't seem very relevant.

If you exit the process, then I agree, the rest is not important.  But
exiting is too drastic, and will be frowned upon, so we shouldn't do
that.  And if we only signal an error, then any code that runs before
it could have effect that previously didn't happen -- that's the
scenario I'm thinking of.

> > I agree that we have a mess, but disagree that we should try to fix
> > it.  It's a mess we have been living with for decades, and I don't
> > doubt that gobs of programs out there depend on this mess.
> 
> Should we rename Emacs-29.2 as Emacs-final, then?  🙂

No.  Risking breakage when adding new useful features is justified.
Risking breakage to cater to oddball use cases isn't.  Emacs is still
being developed, but it isn't a student project, where people should
be free to experiment all they like.  We have users out there who rely
on us to not destabilize Emacs unless we absolutely have to, for much
greater good.





^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2024-02-17 16:07 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-15 16:48 bug#67837: 29.1.90; inhibit-interaction breaks keyboard macros Spencer Baugh
2023-12-15 16:50 ` Spencer Baugh
2023-12-15 18:54   ` Eli Zaretskii
2023-12-15 19:48     ` Spencer Baugh
2023-12-15 20:01       ` Eli Zaretskii
2023-12-15 20:09         ` Spencer Baugh
2023-12-16  7:02           ` Eli Zaretskii
2023-12-16 13:22             ` sbaugh
2023-12-16 13:57               ` Eli Zaretskii
2023-12-15 20:14         ` Eli Zaretskii
2023-12-15 20:39           ` Spencer Baugh
2023-12-16  7:14             ` Eli Zaretskii
2023-12-16 15:52           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-16 16:08             ` Eli Zaretskii
2023-12-16 17:18               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-16 22:26             ` Spencer Baugh
2024-02-16 23:27               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-17  7:53                 ` Eli Zaretskii
2024-02-17 14:13                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-17 14:35                     ` Eli Zaretskii
2024-02-17 14:43                       ` Eli Zaretskii
2024-02-17 15:15                       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-17 16:07                         ` Eli Zaretskii
2024-02-17  7:37               ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).