Package: emacs;
Reported by: dick.r.chiang <at> gmail.com
Date: Thu, 2 Feb 2023 23:45:02 UTC
Severity: wishlist
Tags: patch
Found in version 30.0.50
Done: Stefan Kangas <stefankangas <at> gmail.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 61244 in the body.
You can then email your comments to 61244 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org:bug#61244; Package emacs.
(Thu, 02 Feb 2023 23:45:02 GMT) Full text and rfc822 format available.dick.r.chiang <at> gmail.com:bug-gnu-emacs <at> gnu.org.
(Thu, 02 Feb 2023 23:45:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: dick.r.chiang <at> gmail.com To: bug-gnu-emacs <bug-gnu-emacs <at> gnu.org> Subject: 30.0.50; [PATCH] Promote called-interactively-p Date: Thu, 02 Feb 2023 16:32:32 -0500
[Message part 1 (text/plain, inline)]
FUD surrounding `called-interactively-p` continues to saddle functions with an incongruous "interactive-p" optional argument. `called-interactively-p` is safe enough, and if not, no one is going to miss whatever trivial behaviors hinge on its correctness.
[0001-Promote-called-interactively-p.patch (text/x-diff, attachment)]
[Message part 3 (text/plain, inline)]
In Commercial Emacs 0.3.1snapshot b8701a3 in dev (upstream 30.0.50, x86_64-pc-linux-gnu) built on dick Repository revision: b8701a32dad52e26fcf72ce39c9b631c777a1927 Repository branch: dev Windowing system distributor 'The X.Org Foundation', version 11.0.12101003 System Description: Ubuntu 22.04.1 LTS Configured using: 'configure WERROR_CFLAGS=-Werror --prefix=/home/dick/.local --with-tree-sitter CC=gcc-10 PKG_CONFIG_PATH=/home/dick/.local/lib/pkgconfig CXX=gcc-10' Configured features: CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON TREE_SITTER LCMS2 LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB Important settings: value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix Major mode: Magit Log Minor modes in effect: global-git-commit-mode: t shell-dirtrack-mode: t server-mode: t projectile-mode: t global-xlsp-mode: t global-hl-line-mode: t hl-line-mode: t global-auto-revert-mode: t flx-ido-mode: t winner-mode: t override-global-mode: t tooltip-mode: t global-eldoc-mode: t show-paren-mode: t mouse-wheel-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t buffer-read-only: t column-number-mode: t line-number-mode: t transient-mark-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t Load-path shadows: /home/dick/gomacro-mode/gomacro-mode hides /home/dick/.emacs.d/elpa/gomacro-mode-20200326.1103/gomacro-mode /home/dick/org-gcal.el/org-gcal hides /home/dick/.emacs.d/elpa/org-gcal-0.3/org-gcal /home/dick/.emacs.d/elpa/request-deferred-0.2.0/request-deferred hides /home/dick/.emacs.d/elpa/request-0.3.3/request-deferred /home/dick/.emacs.d/elpa/go-rename-20190805.2101/go-rename hides /home/dick/.emacs.d/elpa/go-mode-1.6.0/go-rename /home/dick/.emacs.d/elpa/go-guru-20181012.330/go-guru hides /home/dick/.emacs.d/elpa/go-mode-1.6.0/go-guru /home/dick/.emacs.d/elpa/chess-2.0.5/_pkg hides /home/dick/.local/share/emacs/site-lisp/_pkg /home/dick/.emacs.d/elpa/chess-2.0.5/chess-pos hides /home/dick/.local/share/emacs/site-lisp/chess-pos /home/dick/.emacs.d/elpa/chess-2.0.5/chess-module hides /home/dick/.local/share/emacs/site-lisp/chess-module /home/dick/.emacs.d/elpa/chess-2.0.5/chess-ucb hides /home/dick/.local/share/emacs/site-lisp/chess-ucb /home/dick/.emacs.d/elpa/chess-2.0.5/chess-scid hides /home/dick/.local/share/emacs/site-lisp/chess-scid /home/dick/.emacs.d/elpa/chess-2.0.5/chess-puzzle hides /home/dick/.local/share/emacs/site-lisp/chess-puzzle /home/dick/.emacs.d/elpa/chess-2.0.5/chess-irc hides /home/dick/.local/share/emacs/site-lisp/chess-irc /home/dick/.emacs.d/elpa/chess-2.0.5/chess-network hides /home/dick/.local/share/emacs/site-lisp/chess-network /home/dick/.emacs.d/elpa/chess-2.0.5/chess-autosave hides /home/dick/.local/share/emacs/site-lisp/chess-autosave /home/dick/.emacs.d/elpa/chess-2.0.5/chess-engine hides /home/dick/.local/share/emacs/site-lisp/chess-engine /home/dick/.emacs.d/elpa/chess-2.0.5/chess-tutorial hides /home/dick/.local/share/emacs/site-lisp/chess-tutorial /home/dick/.emacs.d/elpa/chess-2.0.5/chess-german hides /home/dick/.local/share/emacs/site-lisp/chess-german /home/dick/.emacs.d/elpa/chess-2.0.5/chess-file hides /home/dick/.local/share/emacs/site-lisp/chess-file /home/dick/.emacs.d/elpa/chess-2.0.5/chess-random hides /home/dick/.local/share/emacs/site-lisp/chess-random /home/dick/.emacs.d/elpa/chess-2.0.5/chess-stockfish hides /home/dick/.local/share/emacs/site-lisp/chess-stockfish /home/dick/.emacs.d/elpa/chess-2.0.5/chess-pgn hides /home/dick/.local/share/emacs/site-lisp/chess-pgn /home/dick/.emacs.d/elpa/chess-2.0.5/chess-kibitz hides /home/dick/.local/share/emacs/site-lisp/chess-kibitz /home/dick/.emacs.d/elpa/chess-2.0.5/chess-eco hides /home/dick/.local/share/emacs/site-lisp/chess-eco /home/dick/.emacs.d/elpa/chess-2.0.5/chess-display hides /home/dick/.local/share/emacs/site-lisp/chess-display /home/dick/.emacs.d/elpa/chess-2.0.5/chess-var hides /home/dick/.local/share/emacs/site-lisp/chess-var /home/dick/.emacs.d/elpa/chess-2.0.5/chess-test hides /home/dick/.local/share/emacs/site-lisp/chess-test /home/dick/.emacs.d/elpa/chess-2.0.5/chess-ply hides /home/dick/.local/share/emacs/site-lisp/chess-ply /home/dick/.emacs.d/elpa/chess-2.0.5/chess-message hides /home/dick/.local/share/emacs/site-lisp/chess-message /home/dick/.emacs.d/elpa/chess-2.0.5/chess-ics1 hides /home/dick/.local/share/emacs/site-lisp/chess-ics1 /home/dick/.emacs.d/elpa/chess-2.0.5/chess-phalanx hides /home/dick/.local/share/emacs/site-lisp/chess-phalanx /home/dick/.emacs.d/elpa/chess-2.0.5/chess-game hides /home/dick/.local/share/emacs/site-lisp/chess-game /home/dick/.emacs.d/elpa/chess-2.0.5/chess-log hides /home/dick/.local/share/emacs/site-lisp/chess-log /home/dick/.emacs.d/elpa/chess-2.0.5/chess-plain hides /home/dick/.local/share/emacs/site-lisp/chess-plain /home/dick/.emacs.d/elpa/chess-2.0.5/chess-perft hides /home/dick/.local/share/emacs/site-lisp/chess-perft /home/dick/.emacs.d/elpa/chess-2.0.5/chess-glaurung hides /home/dick/.local/share/emacs/site-lisp/chess-glaurung /home/dick/.emacs.d/elpa/chess-2.0.5/chess-ai hides /home/dick/.local/share/emacs/site-lisp/chess-ai /home/dick/.emacs.d/elpa/chess-2.0.5/chess-fruit hides /home/dick/.local/share/emacs/site-lisp/chess-fruit /home/dick/.emacs.d/elpa/chess-2.0.5/chess-uci hides /home/dick/.local/share/emacs/site-lisp/chess-uci /home/dick/.emacs.d/elpa/chess-2.0.5/chess-epd hides /home/dick/.local/share/emacs/site-lisp/chess-epd /home/dick/.emacs.d/elpa/chess-2.0.5/chess-database hides /home/dick/.local/share/emacs/site-lisp/chess-database /home/dick/.emacs.d/elpa/chess-2.0.5/chess-link hides /home/dick/.local/share/emacs/site-lisp/chess-link /home/dick/.emacs.d/elpa/chess-2.0.5/chess-transport hides /home/dick/.local/share/emacs/site-lisp/chess-transport /home/dick/.emacs.d/elpa/chess-2.0.5/chess-none hides /home/dick/.local/share/emacs/site-lisp/chess-none /home/dick/.emacs.d/elpa/chess-2.0.5/chess-polyglot hides /home/dick/.local/share/emacs/site-lisp/chess-polyglot /home/dick/.emacs.d/elpa/chess-2.0.5/chess-crafty hides /home/dick/.local/share/emacs/site-lisp/chess-crafty /home/dick/.emacs.d/elpa/chess-2.0.5/chess-chat hides /home/dick/.local/share/emacs/site-lisp/chess-chat /home/dick/.emacs.d/elpa/chess-2.0.5/chess hides /home/dick/.local/share/emacs/site-lisp/chess /home/dick/.emacs.d/elpa/chess-2.0.5/chess-images hides /home/dick/.local/share/emacs/site-lisp/chess-images /home/dick/.emacs.d/elpa/chess-2.0.5/chess-gnuchess hides /home/dick/.local/share/emacs/site-lisp/chess-gnuchess /home/dick/.emacs.d/elpa/chess-2.0.5/chess-fen hides /home/dick/.local/share/emacs/site-lisp/chess-fen /home/dick/.emacs.d/elpa/chess-2.0.5/chess-ics hides /home/dick/.local/share/emacs/site-lisp/chess-ics /home/dick/.emacs.d/elpa/chess-2.0.5/chess-ics2 hides /home/dick/.local/share/emacs/site-lisp/chess-ics2 /home/dick/.emacs.d/elpa/chess-2.0.5/chess-common hides /home/dick/.local/share/emacs/site-lisp/chess-common /home/dick/.emacs.d/elpa/chess-2.0.5/chess-input hides /home/dick/.local/share/emacs/site-lisp/chess-input /home/dick/.emacs.d/elpa/chess-2.0.5/chess-announce hides /home/dick/.local/share/emacs/site-lisp/chess-announce /home/dick/.emacs.d/elpa/chess-2.0.5/chess-clock hides /home/dick/.local/share/emacs/site-lisp/chess-clock /home/dick/.emacs.d/elpa/chess-2.0.5/chess-sound hides /home/dick/.local/share/emacs/site-lisp/chess-sound /home/dick/.emacs.d/elpa/chess-2.0.5/chess-sjeng hides /home/dick/.local/share/emacs/site-lisp/chess-sjeng /home/dick/.emacs.d/elpa/chess-2.0.5/chess-algebraic hides /home/dick/.local/share/emacs/site-lisp/chess-algebraic /home/dick/.emacs.d/elpa/transient-0.3.7snapshot/transient hides /home/dick/.local/share/emacs/0.3.1/lisp/transient /home/dick/.emacs.d/elpa/use-package-20200520.2305/use-package-bind-key hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/use-package-bind-key /home/dick/.emacs.d/elpa/use-package-20200520.2305/use-package-lint hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/use-package-lint /home/dick/.emacs.d/elpa/use-package-20200520.2305/use-package-core hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/use-package-core /home/dick/.emacs.d/elpa/use-package-20200520.2305/use-package-ensure hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/use-package-ensure /home/dick/.emacs.d/elpa/bind-key-20161218.1520/bind-key hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/bind-key /home/dick/.emacs.d/elpa/use-package-20200520.2305/use-package-jump hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/use-package-jump /home/dick/.emacs.d/elpa/use-package-20200520.2305/use-package-diminish hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/use-package-diminish /home/dick/.emacs.d/elpa/use-package-20200520.2305/use-package hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/use-package /home/dick/.emacs.d/elpa/use-package-20200520.2305/use-package-delight hides /home/dick/.local/share/emacs/0.3.1/lisp/use-package/use-package-delight /home/dick/.emacs.d/elpa/eglot-1.8/eglot hides /home/dick/.local/share/emacs/0.3.1/lisp/progmodes/eglot /home/dick/.emacs.d/elpa/soap-client-3.1.5/soap-client hides /home/dick/.local/share/emacs/0.3.1/lisp/net/soap-client /home/dick/.emacs.d/elpa/soap-client-3.1.5/soap-inspect hides /home/dick/.local/share/emacs/0.3.1/lisp/net/soap-inspect /home/dick/.emacs.d/elpa/let-alist-1.0.6/let-alist hides /home/dick/.local/share/emacs/0.3.1/lisp/emacs-lisp/let-alist Features: (shadow bbdb-message footnote emacsbug company-oddmuse company-keywords company-etags company-gtags company-dabbrev-code company-dabbrev company-files company-clang company-cmake company-semantic company-template company-bbdb cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs texinfo texinfo-loaddefs git-rebase vc tramp-archive tramp-gvfs tramp-cache time-stamp zeroconf tramp tramp-loaddefs trampver tramp-integration cus-start files-x tramp-compat ls-lisp shr-color qp goto-addr gravatar dns magit-extras mule-util jka-compr face-remap magit-patch-changelog magit-patch 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 git-commit log-edit pcvs-util add-log magit-core magit-autorevert magit-margin magit-transient magit-process with-editor shell pcomplete server magit-mode transient magit-git magit-base magit-section format-spec misearch multi-isearch vc-git diff-mode vc-dispatcher tree-sitter bug-reference vc-svn elpaso elpaso-admin elpaso-milky elpaso-defs shortdoc cal-menu calendar cal-loaddefs gnus-html url-queue help-fns radix-tree sort smiley flow-fill mm-archive mail-extr textsec uni-scripts idna-mapping ucs-normalize uni-confusable textsec-check gnus-async gnus-ml gnus-notifications gnus-fun notifications gnus-kill gnus-dup disp-table utf-7 url-cache benchmark nnrss nnfolder nndiscourse rbenv nnhackernews nntwitter nntwitter-api bbdb-gnus gnus-demon nntp nnmairix nnml nnreddit gnus-topic url-http url-auth url-gw network-stream nsm request virtualenvwrapper gud s dash json-rpc python compat gnus-score score-mode gnus-bcklg gnus-srvr gnus-cite anaphora bbdb-mua bbdb-com crm bbdb bbdb-site timezone gnus-delay gnus-draft gnus-cache gnus-agent gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum shr pixel-fill kinsoku url-file svg dom nndraft nnmh gnus-group mm-url gnus-undo use-package use-package-ensure use-package-delight use-package-diminish gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7 nnoo parse-time iso8601 gnus-spec gnus-int gnus-range message sendmail yank-media puny dired-x dired dired-loaddefs rfc822 mml mml-sec epa epg rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus-win paredit-ext paredit inf-ruby ruby-mode smie haskell-interactive-mode haskell-presentation-mode haskell-process haskell-session haskell-compile haskell-mode haskell-cabal haskell-utils haskell-font-lock haskell-indentation haskell-string haskell-sort-imports haskell-lexeme rx haskell-align-imports haskell-complete-module haskell-ghc-support noutline outline flymake-proc flymake etags fileloop generator dabbrev haskell-customize solarized-theme solarized-definitions projectile lisp-mnt ibuf-ext ibuffer ibuffer-loaddefs thingatpt grep compile comint ansi-osc ansi-color gnus nnheader range mail-utils mm-util mail-prsvr gnus-util text-property-search time-date xlsp xlsp-xref xlsp-server xlsp-company company-capf company xlsp-handle-notification xlsp-handle-request xlsp-struct xlsp-utils jsonrpc pcase warnings hl-line autorevert filenotify flx-ido flx google-translate-default-ui google-translate-core-ui facemenu color ido google-translate-core google-translate-tk google-translate-backend auto-complete advice popup cus-edit pp cus-load icons wid-edit emms-player-mplayer emms-player-simple emms emms-compat winner edmacro kmacro cl-extra help-mode xref project ring use-package-bind-key bind-key easy-mmode use-package-core derived company-go-autoloads wordnut-autoloads quelpa-autoloads haskell-mode-autoloads xlsp-autoloads debbugs-autoloads eglot-autoloads emacsql-autoloads corfu-autoloads elpaso-disc-autoloads elpaso-autoloads find-func sml-mode-autoloads json-reformat-autoloads typescript-mode-autoloads projectile-autoloads nnreddit-autoloads json-snatcher-autoloads yasnippet-autoloads tornado-template-mode-autoloads flycheck-autoloads request-autoloads lsp-mode-autoloads lv-autoloads lsp-bridge-autoloads posframe-autoloads magit-autoloads magit-section-autoloads cask-autoloads epl-autoloads markdown-mode-autoloads go-mode-autoloads dash-autoloads company-autoloads git-commit-autoloads info compat-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 cldefs 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 lcms2 dynamic-setting system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process emacs) Memory information: ((conses 16 1137562 104422) (symbols 48 48318 2) (strings 32 221450 44809) (string-bytes 1 6892398) (vectors 16 136619) (vector-slots 8 3121891 105090) (floats 8 1271 1705) (intervals 56 19769 1249) (buffers 984 46))
bug-gnu-emacs <at> gnu.org:bug#61244; Package emacs.
(Fri, 03 Feb 2023 07:36:02 GMT) Full text and rfc822 format available.Message #8 received at 61244 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: dick.r.chiang <at> gmail.com, Stefan Monnier <monnier <at> iro.umontreal.ca> Cc: 61244 <at> debbugs.gnu.org Subject: Re: bug#61244: 30.0.50; [PATCH] Promote called-interactively-p Date: Fri, 03 Feb 2023 09:35:41 +0200
> From: dick.r.chiang <at> gmail.com > Date: Thu, 02 Feb 2023 16:32:32 -0500 > > FUD surrounding `called-interactively-p` continues to saddle > functions with an incongruous "interactive-p" optional > argument. `called-interactively-p` is safe enough, and if not, > no one is going to miss whatever trivial behaviors hinge > on its correctness. Please explain the rationale and the effects of the code in more detail, otherwise this is impossible to consider. Stefan, any comments or insights?
bug-gnu-emacs <at> gnu.org:bug#61244; Package emacs.
(Fri, 03 Feb 2023 15:53:01 GMT) Full text and rfc822 format available.Message #11 received at 61244 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: dick.r.chiang <at> gmail.com Cc: 61244 <at> debbugs.gnu.org Subject: Re: bug#61244: 30.0.50; [PATCH] Promote called-interactively-p Date: Fri, 03 Feb 2023 10:52:13 -0500
> FUD surrounding `called-interactively-p` continues to saddle functions
> with an incongruous "interactive-p" optional argument.
If the arg is called `interactive-p` it's usually because the programmer
was too lazy to think about a good name for that arg (one which actually
describes the effect it has).
Maybe in our recommendation for the use of an extra argument, we should
make an effort to mention that (i.e. that we recommend that the arg's
name reflect the effect, and that the docstring document it
accordingly).
> `called-interactively-p` is safe enough,
I agree that `called-interactively-p` is safe: nothing bad will result
if you call it. But its return value is unreliable (when it returns
non-nil, I think it *is* reliable, but in many cases it will return nil
even tho it "should" return non-nil).
> +runtime links one to the other. When @code{called-interactively-p} is
> +invoked, tracing the stack frame of the subject function is fraught
> +given how many intervening function calls could result from arbitrary
> +macro expansions, special forms, and advices including those for
> +debugging instrumentation. The heuristics applied cannot guarantee a
> +correct result under all conceivable conditions.
Clearly, we agree on the technical aspect.
We just disagree about what conclusion to draw from it.
I personally can't recommend the use of a function for which we "cannot
guarantee a correct result under all conceivable conditions" when there
is a good alternative which does.
> + "Return t if the containing function was called interactively.
> +Be warned the function may yield an incorrect result when the
> +containing function is advised or instrumented for debugging, or
> +when the call to `called-interactively-p' is enclosed in
> +macros or special forms which wrap it in a lambda closure.
> +
> +If knowing the calling context is critical, one must modify the
> +containing function's lexical environment as described in Info
> +node `(elisp)Distinguish Interactive'.
> +
> +If KIND is \\='interactive, the function returns nil if either
> +`executing-kbd-macro' or `noninteractive' is true. The KIND
> +argument is deprecated in favor of checking those conditions
> +outside this function."
> + (let ((kind-exception (and (eq kind 'interactive)
> + (or noninteractive executing-kbd-macro))))
> + (unless kind-exception
> + ;; Call stack grows down with decreasing I.
> + ;; Walk up stack until containing function's frame reached.
> + (let* ((i 0)
> + (child (backtrace-frame i 'called-interactively-p))
> + (parent (backtrace-frame (1+ i) 'called-interactively-p))
> + (walk-up-stack
> + (lambda ()
> + (setq i (1+ i)
> + child parent
> + parent (backtrace-frame (1+ i) 'called-interactively-p)))))
> + (while (progn (funcall walk-up-stack)
> + (or
> + ;; Skip special forms from non-compiled code.
> + (and child (null (car child)))
> + ;; Skip package-specific stack-frames.
> + (let ((skip (run-hook-with-args-until-success
> + 'called-interactively-p-functions
> + (+ i 2) child parent)))
> + (pcase skip
> + ('nil nil)
> + (0 t)
> + (_ (setq i (1- (+ i skip)))
> + (funcall walk-up-stack)))))))
> + ;; CHILD should now be containing function.
> + (pcase (cons child parent)
> + ;; checks if CHILD is built-in primitive (never interactive).
> + (`((,_ ,(pred (lambda (f) (subr-primitive-p (indirect-function f)))) . ,_) . ,_)
> + nil)
> + ;; checks if PARENT is `funcall_interactively'.
> + (`(,_ . (t ,(pred (lambda (f)
> + (eq internal--funcall-interactively
> + (indirect-function f))))
> + . ,_))
> + t))))))
You describe this change as "Clarify". I don't immediately see whether
the code does the same as before nor in which sense it's more clear.
Can you describe a bit more precisely what changes you've made here?
I see you renamed the frames to `child` and `parent`, which doesn't
sound too bad.
[ One cosmetic thing I notice is that I was careful to have a single
copy of (backtrace-frame i 'called-interactively-p) whereas you now
have 3 of them. ]
> +(define-obsolete-function-alias 'interactive-p
> + #'called-interactively-p "23.2"
> + "Keep alias (https://lists.gnu.org/r/emacs-devel/2020-08/msg00564.html)")
I don't have an opinion on that.
> -/* BEWARE: Calling this directly from C would defeat the purpose! */
> DEFUN ("funcall-interactively", Ffuncall_interactively, Sfuncall_interactively,
> - 1, MANY, 0, doc: /* Like `funcall' but marks the call as interactive.
> -I.e. arrange that within the called function `called-interactively-p' will
> -return non-nil.
> + 1, MANY, 0, doc: /* Differentiate from `funcall' to indicate interactive call.
> +The function `called-interactively-p' looks for this very function token.
> +This primitive should not be called from C since its very purpose
> +is to appear as a literal token in the lisp call stack.
> usage: (funcall-interactively FUNCTION &rest ARGUMENTS) */)
> (ptrdiff_t nargs, Lisp_Object *args)
> {
> specpdl_ref speccount = SPECPDL_INDEX ();
> temporarily_switch_to_single_kboard (NULL);
> -
> - /* Nothing special to do here, all the work is inside
> - `called-interactively-p'. Which will look for us as a marker in the
> - backtrace. */
> return unbind_to (speccount, Ffuncall (nargs, args));
> }
No clear opinion on this either. I like when comments are replaced by
actual documentation, but we usually don't put into docstrings info
about whether a function can be called from C (docstrings are meant for
ELisp callers only).
> diff --git a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
> index b0211c915e6..b033fdddcd8 100644
> --- a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
> +++ b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
> @@ -33,6 +33,10 @@ edebug-test-code-fac
> (* n (edebug-test-code-fac (1- n)))!mult!
> 1))
>
> +(defun edebug-test-code-called-interactively-p ()
> + (interactive)
> + !start!(called-interactively-p))
> +
> (defun edebug-test-code-concat (a b flag)
> !start!(if flag!flag!
> !then-start!(concat a!then-a! b!then-b!)!then-concat!
> diff --git a/test/lisp/emacs-lisp/edebug-tests.el b/test/lisp/emacs-lisp/edebug-tests.el
> index de2fff5ef19..72ea5874cae 100644
> --- a/test/lisp/emacs-lisp/edebug-tests.el
> +++ b/test/lisp/emacs-lisp/edebug-tests.el
> @@ -56,6 +56,7 @@ edebug-tests-failure-in-post-command
> (defvar-keymap edebug-tests-keymap
> :doc "Keys used by the keyboard macros in Edebug's tests."
> "@" 'edebug-tests-call-instrumented-func
> + "#" 'edebug-tests-call-interactively-instrumented-func
> "C-u" 'universal-argument
> "C-p" 'previous-line
> "C-n" 'next-line
> @@ -268,6 +269,13 @@ edebug-tests-setup-@
> edebug-tests-args args)
> (setq edebug-tests-@-result 'no-result)))
>
> +(defun edebug-tests-call-interactively-instrumented-func ()
> + "Call interactively `edebug-tests-func' and save results."
> + (interactive)
> + (let ((result (call-interactively edebug-tests-func)))
> + (should (eq edebug-tests-@-result 'no-result))
> + (setq edebug-tests-@-result result)))
> +
> (defun edebug-tests-call-instrumented-func ()
> "Call `edebug-tests-func' with `edebug-tests-args' and save the results."
> (interactive)
> @@ -440,6 +448,14 @@ edebug-tests-stop-point-at-start-of-first-instrumented-function
> "SPC" (edebug-tests-should-be-at "fac" "step")
> "g" (should (equal edebug-tests-@-result 1)))))
>
> +(ert-deftest edebug-tests-called-interactively-p ()
> + "`called-interactively-p' still works under edebug."
> + (edebug-tests-with-normal-env
> + (edebug-tests-setup-@ "called-interactively-p" '() t)
> + (edebug-tests-run-kbd-macro
> + "#" (edebug-tests-should-be-at "called-interactively-p" "start")
> + "g" (should (equal edebug-tests-@-result t)))))
> +
> (ert-deftest edebug-tests-step-showing-evaluation-results ()
> "Edebug prints expression evaluation results to the echo area."
> (edebug-tests-with-normal-env
Are these new tests things that are fixed by your patch, or they are
just new tests "for the sake of it"?
[ In either case, I welcome new tests. ]
> diff --git a/test/lisp/emacs-lisp/nadvice-tests.el b/test/lisp/emacs-lisp/nadvice-tests.el
> index 748d42f2120..77df743a3e2 100644
> --- a/test/lisp/emacs-lisp/nadvice-tests.el
> +++ b/test/lisp/emacs-lisp/nadvice-tests.el
> @@ -145,9 +145,8 @@ advice-test-called-interactively-p-around
>
> (ert-deftest advice-test-called-interactively-p-filter-args ()
> "Check interaction between filter-args advice and called-interactively-p."
> - :expected-result :failed
> (defun sm-test7.3 () (interactive) (cons 1 (called-interactively-p)))
> - (advice-add 'sm-test7.3 :filter-args #'list)
> + (advice-add 'sm-test7.3 :filter-args #'identity)
> (should (equal (sm-test7.3) '(1 . nil)))
> (should (equal (call-interactively 'sm-test7.3) '(1 . t))))
Duh! Thanks.
> @@ -163,6 +162,18 @@ advice-test-call-interactively
> (advice-remove 'call-interactively #'ignore)
> (should (eq (symbol-function 'call-interactively) old)))))
>
> +(ert-deftest advice-test-called-interactively-p-around-careful ()
> + "Like sm-test7.2 but defensively preserve interactive context."
> + (defun sm-test7.5 () (interactive) (cons 1 (called-interactively-p)))
> + (advice-add 'sm-test7.5 :around
> + (lambda (f &rest args)
> + (list (cons 1 (called-interactively-p))
> + (if (called-interactively-p)
> + (call-interactively f)
> + (apply f args)))))
> + (should (equal (sm-test7.5) '((1 . nil) (1 . nil))))
> + (should (equal (call-interactively 'sm-test7.5) '((1 . t) (1 . t)))))
> +
> (ert-deftest advice-test-interactive ()
> "Check handling of interactive spec."
> (defun sm-test8 (a) (interactive "p") a)
I'd use `funcall-interactively` rather than `call-interactively` so as
to correctly preserve `args` rather than recreate them.
Stefan
Stefan Kangas <stefankangas <at> gmail.com>
to control <at> debbugs.gnu.org.
(Sun, 10 Sep 2023 17:19:02 GMT) Full text and rfc822 format available.bug-gnu-emacs <at> gnu.org:bug#61244; Package emacs.
(Wed, 12 Feb 2025 03:05:02 GMT) Full text and rfc822 format available.Message #16 received at 61244 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefankangas <at> gmail.com> To: Stefan Monnier <monnier <at> iro.umontreal.ca> Cc: 61244 <at> debbugs.gnu.org, dick.r.chiang <at> gmail.com Subject: Re: bug#61244: 30.0.50; [PATCH] Promote called-interactively-p Date: Tue, 11 Feb 2025 19:04:07 -0800
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> FUD surrounding `called-interactively-p` continues to saddle functions
>> with an incongruous "interactive-p" optional argument.
>
> If the arg is called `interactive-p` it's usually because the programmer
> was too lazy to think about a good name for that arg (one which actually
> describes the effect it has).
>
> Maybe in our recommendation for the use of an extra argument, we should
> make an effort to mention that (i.e. that we recommend that the arg's
> name reflect the effect, and that the docstring document it
> accordingly).
>
>> `called-interactively-p` is safe enough,
>
> I agree that `called-interactively-p` is safe: nothing bad will result
> if you call it. But its return value is unreliable (when it returns
> non-nil, I think it *is* reliable, but in many cases it will return nil
> even tho it "should" return non-nil).
>
>> +runtime links one to the other. When @code{called-interactively-p} is
>> +invoked, tracing the stack frame of the subject function is fraught
>> +given how many intervening function calls could result from arbitrary
>> +macro expansions, special forms, and advices including those for
>> +debugging instrumentation. The heuristics applied cannot guarantee a
>> +correct result under all conceivable conditions.
>
> Clearly, we agree on the technical aspect.
> We just disagree about what conclusion to draw from it.
>
> I personally can't recommend the use of a function for which we "cannot
> guarantee a correct result under all conceivable conditions" when there
> is a good alternative which does.
>
>> + "Return t if the containing function was called interactively.
>> +Be warned the function may yield an incorrect result when the
>> +containing function is advised or instrumented for debugging, or
>> +when the call to `called-interactively-p' is enclosed in
>> +macros or special forms which wrap it in a lambda closure.
>> +
>> +If knowing the calling context is critical, one must modify the
>> +containing function's lexical environment as described in Info
>> +node `(elisp)Distinguish Interactive'.
>> +
>> +If KIND is \\='interactive, the function returns nil if either
>> +`executing-kbd-macro' or `noninteractive' is true. The KIND
>> +argument is deprecated in favor of checking those conditions
>> +outside this function."
>> + (let ((kind-exception (and (eq kind 'interactive)
>> + (or noninteractive executing-kbd-macro))))
>> + (unless kind-exception
>> + ;; Call stack grows down with decreasing I.
>> + ;; Walk up stack until containing function's frame reached.
>> + (let* ((i 0)
>> + (child (backtrace-frame i 'called-interactively-p))
>> + (parent (backtrace-frame (1+ i) 'called-interactively-p))
>> + (walk-up-stack
>> + (lambda ()
>> + (setq i (1+ i)
>> + child parent
>> + parent (backtrace-frame (1+ i) 'called-interactively-p)))))
>> + (while (progn (funcall walk-up-stack)
>> + (or
>> + ;; Skip special forms from non-compiled code.
>> + (and child (null (car child)))
>> + ;; Skip package-specific stack-frames.
>> + (let ((skip (run-hook-with-args-until-success
>> + 'called-interactively-p-functions
>> + (+ i 2) child parent)))
>> + (pcase skip
>> + ('nil nil)
>> + (0 t)
>> + (_ (setq i (1- (+ i skip)))
>> + (funcall walk-up-stack)))))))
>> + ;; CHILD should now be containing function.
>> + (pcase (cons child parent)
>> + ;; checks if CHILD is built-in primitive (never interactive).
>> + (`((,_ ,(pred (lambda (f) (subr-primitive-p (indirect-function f)))) . ,_) . ,_)
>> + nil)
>> + ;; checks if PARENT is `funcall_interactively'.
>> + (`(,_ . (t ,(pred (lambda (f)
>> + (eq internal--funcall-interactively
>> + (indirect-function f))))
>> + . ,_))
>> + t))))))
>
> You describe this change as "Clarify". I don't immediately see whether
> the code does the same as before nor in which sense it's more clear.
> Can you describe a bit more precisely what changes you've made here?
> I see you renamed the frames to `child` and `parent`, which doesn't
> sound too bad.
>
> [ One cosmetic thing I notice is that I was careful to have a single
> copy of (backtrace-frame i 'called-interactively-p) whereas you now
> have 3 of them. ]
>
>> +(define-obsolete-function-alias 'interactive-p
>> + #'called-interactively-p "23.2"
>> + "Keep alias (https://lists.gnu.org/r/emacs-devel/2020-08/msg00564.html)")
>
> I don't have an opinion on that.
>
>> -/* BEWARE: Calling this directly from C would defeat the purpose! */
>> DEFUN ("funcall-interactively", Ffuncall_interactively, Sfuncall_interactively,
>> - 1, MANY, 0, doc: /* Like `funcall' but marks the call as interactive.
>> -I.e. arrange that within the called function `called-interactively-p' will
>> -return non-nil.
>> + 1, MANY, 0, doc: /* Differentiate from `funcall' to indicate interactive call.
>> +The function `called-interactively-p' looks for this very function token.
>> +This primitive should not be called from C since its very purpose
>> +is to appear as a literal token in the lisp call stack.
>> usage: (funcall-interactively FUNCTION &rest ARGUMENTS) */)
>> (ptrdiff_t nargs, Lisp_Object *args)
>> {
>> specpdl_ref speccount = SPECPDL_INDEX ();
>> temporarily_switch_to_single_kboard (NULL);
>> -
>> - /* Nothing special to do here, all the work is inside
>> - `called-interactively-p'. Which will look for us as a marker in the
>> - backtrace. */
>> return unbind_to (speccount, Ffuncall (nargs, args));
>> }
>
> No clear opinion on this either. I like when comments are replaced by
> actual documentation, but we usually don't put into docstrings info
> about whether a function can be called from C (docstrings are meant for
> ELisp callers only).
>
>> diff --git a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
>> index b0211c915e6..b033fdddcd8 100644
>> --- a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
>> +++ b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
>> @@ -33,6 +33,10 @@ edebug-test-code-fac
>> (* n (edebug-test-code-fac (1- n)))!mult!
>> 1))
>>
>> +(defun edebug-test-code-called-interactively-p ()
>> + (interactive)
>> + !start!(called-interactively-p))
>> +
>> (defun edebug-test-code-concat (a b flag)
>> !start!(if flag!flag!
>> !then-start!(concat a!then-a! b!then-b!)!then-concat!
>> diff --git a/test/lisp/emacs-lisp/edebug-tests.el b/test/lisp/emacs-lisp/edebug-tests.el
>> index de2fff5ef19..72ea5874cae 100644
>> --- a/test/lisp/emacs-lisp/edebug-tests.el
>> +++ b/test/lisp/emacs-lisp/edebug-tests.el
>> @@ -56,6 +56,7 @@ edebug-tests-failure-in-post-command
>> (defvar-keymap edebug-tests-keymap
>> :doc "Keys used by the keyboard macros in Edebug's tests."
>> "@" 'edebug-tests-call-instrumented-func
>> + "#" 'edebug-tests-call-interactively-instrumented-func
>> "C-u" 'universal-argument
>> "C-p" 'previous-line
>> "C-n" 'next-line
>> @@ -268,6 +269,13 @@ edebug-tests-setup-@
>> edebug-tests-args args)
>> (setq edebug-tests-@-result 'no-result)))
>>
>> +(defun edebug-tests-call-interactively-instrumented-func ()
>> + "Call interactively `edebug-tests-func' and save results."
>> + (interactive)
>> + (let ((result (call-interactively edebug-tests-func)))
>> + (should (eq edebug-tests-@-result 'no-result))
>> + (setq edebug-tests-@-result result)))
>> +
>> (defun edebug-tests-call-instrumented-func ()
>> "Call `edebug-tests-func' with `edebug-tests-args' and save the results."
>> (interactive)
>> @@ -440,6 +448,14 @@ edebug-tests-stop-point-at-start-of-first-instrumented-function
>> "SPC" (edebug-tests-should-be-at "fac" "step")
>> "g" (should (equal edebug-tests-@-result 1)))))
>>
>> +(ert-deftest edebug-tests-called-interactively-p ()
>> + "`called-interactively-p' still works under edebug."
>> + (edebug-tests-with-normal-env
>> + (edebug-tests-setup-@ "called-interactively-p" '() t)
>> + (edebug-tests-run-kbd-macro
>> + "#" (edebug-tests-should-be-at "called-interactively-p" "start")
>> + "g" (should (equal edebug-tests-@-result t)))))
>> +
>> (ert-deftest edebug-tests-step-showing-evaluation-results ()
>> "Edebug prints expression evaluation results to the echo area."
>> (edebug-tests-with-normal-env
>
> Are these new tests things that are fixed by your patch, or they are
> just new tests "for the sake of it"?
> [ In either case, I welcome new tests. ]
>
>> diff --git a/test/lisp/emacs-lisp/nadvice-tests.el b/test/lisp/emacs-lisp/nadvice-tests.el
>> index 748d42f2120..77df743a3e2 100644
>> --- a/test/lisp/emacs-lisp/nadvice-tests.el
>> +++ b/test/lisp/emacs-lisp/nadvice-tests.el
>> @@ -145,9 +145,8 @@ advice-test-called-interactively-p-around
>>
>> (ert-deftest advice-test-called-interactively-p-filter-args ()
>> "Check interaction between filter-args advice and called-interactively-p."
>> - :expected-result :failed
>> (defun sm-test7.3 () (interactive) (cons 1 (called-interactively-p)))
>> - (advice-add 'sm-test7.3 :filter-args #'list)
>> + (advice-add 'sm-test7.3 :filter-args #'identity)
>> (should (equal (sm-test7.3) '(1 . nil)))
>> (should (equal (call-interactively 'sm-test7.3) '(1 . t))))
>
> Duh! Thanks.
>
>> @@ -163,6 +162,18 @@ advice-test-call-interactively
>> (advice-remove 'call-interactively #'ignore)
>> (should (eq (symbol-function 'call-interactively) old)))))
>>
>> +(ert-deftest advice-test-called-interactively-p-around-careful ()
>> + "Like sm-test7.2 but defensively preserve interactive context."
>> + (defun sm-test7.5 () (interactive) (cons 1 (called-interactively-p)))
>> + (advice-add 'sm-test7.5 :around
>> + (lambda (f &rest args)
>> + (list (cons 1 (called-interactively-p))
>> + (if (called-interactively-p)
>> + (call-interactively f)
>> + (apply f args)))))
>> + (should (equal (sm-test7.5) '((1 . nil) (1 . nil))))
>> + (should (equal (call-interactively 'sm-test7.5) '((1 . t) (1 . t)))))
>> +
>> (ert-deftest advice-test-interactive ()
>> "Check handling of interactive spec."
>> (defun sm-test8 (a) (interactive "p") a)
>
> I'd use `funcall-interactively` rather than `call-interactively` so as
> to correctly preserve `args` rather than recreate them.
>
>
> Stefan
dick, could you please look into and reply to Stefan M's comments above?
Thanks in advance.
Stefan Kangas <stefankangas <at> gmail.com>:dick.r.chiang <at> gmail.com:Message #21 received at 61244-done <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefankangas <at> gmail.com> To: Stefan Monnier <monnier <at> iro.umontreal.ca> Cc: 61244-done <at> debbugs.gnu.org Subject: Re: bug#61244: 30.0.50; [PATCH] Promote called-interactively-p Date: Sun, 23 Feb 2025 01:31:33 +0000
Stefan Kangas <stefankangas <at> gmail.com> writes:
> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>>> FUD surrounding `called-interactively-p` continues to saddle functions
>>> with an incongruous "interactive-p" optional argument.
>>
>> If the arg is called `interactive-p` it's usually because the programmer
>> was too lazy to think about a good name for that arg (one which actually
>> describes the effect it has).
>>
>> Maybe in our recommendation for the use of an extra argument, we should
>> make an effort to mention that (i.e. that we recommend that the arg's
>> name reflect the effect, and that the docstring document it
>> accordingly).
>>
>>> `called-interactively-p` is safe enough,
>>
>> I agree that `called-interactively-p` is safe: nothing bad will result
>> if you call it. But its return value is unreliable (when it returns
>> non-nil, I think it *is* reliable, but in many cases it will return nil
>> even tho it "should" return non-nil).
>>
>>> +runtime links one to the other. When @code{called-interactively-p} is
>>> +invoked, tracing the stack frame of the subject function is fraught
>>> +given how many intervening function calls could result from arbitrary
>>> +macro expansions, special forms, and advices including those for
>>> +debugging instrumentation. The heuristics applied cannot guarantee a
>>> +correct result under all conceivable conditions.
>>
>> Clearly, we agree on the technical aspect.
>> We just disagree about what conclusion to draw from it.
>>
>> I personally can't recommend the use of a function for which we "cannot
>> guarantee a correct result under all conceivable conditions" when there
>> is a good alternative which does.
>>
>>> + "Return t if the containing function was called interactively.
>>> +Be warned the function may yield an incorrect result when the
>>> +containing function is advised or instrumented for debugging, or
>>> +when the call to `called-interactively-p' is enclosed in
>>> +macros or special forms which wrap it in a lambda closure.
>>> +
>>> +If knowing the calling context is critical, one must modify the
>>> +containing function's lexical environment as described in Info
>>> +node `(elisp)Distinguish Interactive'.
>>> +
>>> +If KIND is \\='interactive, the function returns nil if either
>>> +`executing-kbd-macro' or `noninteractive' is true. The KIND
>>> +argument is deprecated in favor of checking those conditions
>>> +outside this function."
>>> + (let ((kind-exception (and (eq kind 'interactive)
>>> + (or noninteractive executing-kbd-macro))))
>>> + (unless kind-exception
>>> + ;; Call stack grows down with decreasing I.
>>> + ;; Walk up stack until containing function's frame reached.
>>> + (let* ((i 0)
>>> + (child (backtrace-frame i 'called-interactively-p))
>>> + (parent (backtrace-frame (1+ i) 'called-interactively-p))
>>> + (walk-up-stack
>>> + (lambda ()
>>> + (setq i (1+ i)
>>> + child parent
>>> + parent (backtrace-frame (1+ i) 'called-interactively-p)))))
>>> + (while (progn (funcall walk-up-stack)
>>> + (or
>>> + ;; Skip special forms from non-compiled code.
>>> + (and child (null (car child)))
>>> + ;; Skip package-specific stack-frames.
>>> + (let ((skip (run-hook-with-args-until-success
>>> + 'called-interactively-p-functions
>>> + (+ i 2) child parent)))
>>> + (pcase skip
>>> + ('nil nil)
>>> + (0 t)
>>> + (_ (setq i (1- (+ i skip)))
>>> + (funcall walk-up-stack)))))))
>>> + ;; CHILD should now be containing function.
>>> + (pcase (cons child parent)
>>> + ;; checks if CHILD is built-in primitive (never interactive).
>>> + (`((,_ ,(pred (lambda (f) (subr-primitive-p (indirect-function f)))) . ,_) . ,_)
>>> + nil)
>>> + ;; checks if PARENT is `funcall_interactively'.
>>> + (`(,_ . (t ,(pred (lambda (f)
>>> + (eq internal--funcall-interactively
>>> + (indirect-function f))))
>>> + . ,_))
>>> + t))))))
>>
>> You describe this change as "Clarify". I don't immediately see whether
>> the code does the same as before nor in which sense it's more clear.
>> Can you describe a bit more precisely what changes you've made here?
>> I see you renamed the frames to `child` and `parent`, which doesn't
>> sound too bad.
>>
>> [ One cosmetic thing I notice is that I was careful to have a single
>> copy of (backtrace-frame i 'called-interactively-p) whereas you now
>> have 3 of them. ]
>>
>>> +(define-obsolete-function-alias 'interactive-p
>>> + #'called-interactively-p "23.2"
>>> + "Keep alias (https://lists.gnu.org/r/emacs-devel/2020-08/msg00564.html)")
>>
>> I don't have an opinion on that.
>>
>>> -/* BEWARE: Calling this directly from C would defeat the purpose! */
>>> DEFUN ("funcall-interactively", Ffuncall_interactively, Sfuncall_interactively,
>>> - 1, MANY, 0, doc: /* Like `funcall' but marks the call as interactive.
>>> -I.e. arrange that within the called function `called-interactively-p' will
>>> -return non-nil.
>>> + 1, MANY, 0, doc: /* Differentiate from `funcall' to indicate interactive call.
>>> +The function `called-interactively-p' looks for this very function token.
>>> +This primitive should not be called from C since its very purpose
>>> +is to appear as a literal token in the lisp call stack.
>>> usage: (funcall-interactively FUNCTION &rest ARGUMENTS) */)
>>> (ptrdiff_t nargs, Lisp_Object *args)
>>> {
>>> specpdl_ref speccount = SPECPDL_INDEX ();
>>> temporarily_switch_to_single_kboard (NULL);
>>> -
>>> - /* Nothing special to do here, all the work is inside
>>> - `called-interactively-p'. Which will look for us as a marker in the
>>> - backtrace. */
>>> return unbind_to (speccount, Ffuncall (nargs, args));
>>> }
>>
>> No clear opinion on this either. I like when comments are replaced by
>> actual documentation, but we usually don't put into docstrings info
>> about whether a function can be called from C (docstrings are meant for
>> ELisp callers only).
>>
>>> diff --git a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
>>> index b0211c915e6..b033fdddcd8 100644
>>> --- a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
>>> +++ b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
>>> @@ -33,6 +33,10 @@ edebug-test-code-fac
>>> (* n (edebug-test-code-fac (1- n)))!mult!
>>> 1))
>>>
>>> +(defun edebug-test-code-called-interactively-p ()
>>> + (interactive)
>>> + !start!(called-interactively-p))
>>> +
>>> (defun edebug-test-code-concat (a b flag)
>>> !start!(if flag!flag!
>>> !then-start!(concat a!then-a! b!then-b!)!then-concat!
>>> diff --git a/test/lisp/emacs-lisp/edebug-tests.el b/test/lisp/emacs-lisp/edebug-tests.el
>>> index de2fff5ef19..72ea5874cae 100644
>>> --- a/test/lisp/emacs-lisp/edebug-tests.el
>>> +++ b/test/lisp/emacs-lisp/edebug-tests.el
>>> @@ -56,6 +56,7 @@ edebug-tests-failure-in-post-command
>>> (defvar-keymap edebug-tests-keymap
>>> :doc "Keys used by the keyboard macros in Edebug's tests."
>>> "@" 'edebug-tests-call-instrumented-func
>>> + "#" 'edebug-tests-call-interactively-instrumented-func
>>> "C-u" 'universal-argument
>>> "C-p" 'previous-line
>>> "C-n" 'next-line
>>> @@ -268,6 +269,13 @@ edebug-tests-setup-@
>>> edebug-tests-args args)
>>> (setq edebug-tests-@-result 'no-result)))
>>>
>>> +(defun edebug-tests-call-interactively-instrumented-func ()
>>> + "Call interactively `edebug-tests-func' and save results."
>>> + (interactive)
>>> + (let ((result (call-interactively edebug-tests-func)))
>>> + (should (eq edebug-tests-@-result 'no-result))
>>> + (setq edebug-tests-@-result result)))
>>> +
>>> (defun edebug-tests-call-instrumented-func ()
>>> "Call `edebug-tests-func' with `edebug-tests-args' and save the results."
>>> (interactive)
>>> @@ -440,6 +448,14 @@ edebug-tests-stop-point-at-start-of-first-instrumented-function
>>> "SPC" (edebug-tests-should-be-at "fac" "step")
>>> "g" (should (equal edebug-tests-@-result 1)))))
>>>
>>> +(ert-deftest edebug-tests-called-interactively-p ()
>>> + "`called-interactively-p' still works under edebug."
>>> + (edebug-tests-with-normal-env
>>> + (edebug-tests-setup-@ "called-interactively-p" '() t)
>>> + (edebug-tests-run-kbd-macro
>>> + "#" (edebug-tests-should-be-at "called-interactively-p" "start")
>>> + "g" (should (equal edebug-tests-@-result t)))))
>>> +
>>> (ert-deftest edebug-tests-step-showing-evaluation-results ()
>>> "Edebug prints expression evaluation results to the echo area."
>>> (edebug-tests-with-normal-env
>>
>> Are these new tests things that are fixed by your patch, or they are
>> just new tests "for the sake of it"?
>> [ In either case, I welcome new tests. ]
>>
>>> diff --git a/test/lisp/emacs-lisp/nadvice-tests.el b/test/lisp/emacs-lisp/nadvice-tests.el
>>> index 748d42f2120..77df743a3e2 100644
>>> --- a/test/lisp/emacs-lisp/nadvice-tests.el
>>> +++ b/test/lisp/emacs-lisp/nadvice-tests.el
>>> @@ -145,9 +145,8 @@ advice-test-called-interactively-p-around
>>>
>>> (ert-deftest advice-test-called-interactively-p-filter-args ()
>>> "Check interaction between filter-args advice and called-interactively-p."
>>> - :expected-result :failed
>>> (defun sm-test7.3 () (interactive) (cons 1 (called-interactively-p)))
>>> - (advice-add 'sm-test7.3 :filter-args #'list)
>>> + (advice-add 'sm-test7.3 :filter-args #'identity)
>>> (should (equal (sm-test7.3) '(1 . nil)))
>>> (should (equal (call-interactively 'sm-test7.3) '(1 . t))))
>>
>> Duh! Thanks.
>>
>>> @@ -163,6 +162,18 @@ advice-test-call-interactively
>>> (advice-remove 'call-interactively #'ignore)
>>> (should (eq (symbol-function 'call-interactively) old)))))
>>>
>>> +(ert-deftest advice-test-called-interactively-p-around-careful ()
>>> + "Like sm-test7.2 but defensively preserve interactive context."
>>> + (defun sm-test7.5 () (interactive) (cons 1 (called-interactively-p)))
>>> + (advice-add 'sm-test7.5 :around
>>> + (lambda (f &rest args)
>>> + (list (cons 1 (called-interactively-p))
>>> + (if (called-interactively-p)
>>> + (call-interactively f)
>>> + (apply f args)))))
>>> + (should (equal (sm-test7.5) '((1 . nil) (1 . nil))))
>>> + (should (equal (call-interactively 'sm-test7.5) '((1 . t) (1 . t)))))
>>> +
>>> (ert-deftest advice-test-interactive ()
>>> "Check handling of interactive spec."
>>> (defun sm-test8 (a) (interactive "p") a)
>>
>> I'd use `funcall-interactively` rather than `call-interactively` so as
>> to correctly preserve `args` rather than recreate them.
>>
>>
>> Stefan
>
> dick, could you please look into and reply to Stefan M's comments above?
>
> Thanks in advance.
I guess we're not going to get any reply here.
I'm therefore closing this bug report.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Sun, 23 Mar 2025 11:24:34 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.