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