>From 0e638368eeef35136b2c8126e2a2e0e8fd0aa47d Mon Sep 17 00:00:00 2001 From: dickmao Date: Thu, 2 Feb 2023 16:20:04 -0500 Subject: [PATCH] Promote called-interactively-p * doc/lispref/commands.texi (Distinguish Interactive): Promote. * lisp/subr.el (called-interactively-p): Clarify. (interactive-p): Alias. * src/callint.c (Ffuncall_interactively): Clarify. * test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el (edebug-test-code-called-interactively-p): Test. * test/lisp/emacs-lisp/edebug-tests.el (edebug-tests-keymap): Test. (edebug-tests-call-interactively-instrumented-func): Test. (edebug-tests-called-interactively-p): Test. * test/lisp/emacs-lisp/nadvice-tests.el (advice-test-called-interactively-p-filter-args): Fix. (advice-test-called-interactively-p-around-careful): Test. --- doc/lispref/commands.texi | 96 +++++------ lisp/subr.el | 152 +++++++----------- src/callint.c | 12 +- .../edebug-resources/edebug-test-code.el | 4 + test/lisp/emacs-lisp/edebug-tests.el | 16 ++ test/lisp/emacs-lisp/nadvice-tests.el | 15 +- 6 files changed, 135 insertions(+), 160 deletions(-) diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi index dc78adc4520..b02b6ba470a 100644 --- a/doc/lispref/commands.texi +++ b/doc/lispref/commands.texi @@ -906,61 +906,15 @@ Distinguish Interactive @cindex distinguish interactive calls @cindex is this call interactive - Sometimes a command should display additional visual feedback (such -as an informative message in the echo area) for interactive calls -only. There are three ways to do this. The recommended way to test -whether the function was called using @code{call-interactively} is to -give it an optional argument @code{print-message} and use the -@code{interactive} spec to make it non-@code{nil} in interactive -calls. Here's an example: - -@example -(defun foo (&optional print-message) - (interactive "p") - (when print-message - (message "foo"))) -@end example - -@noindent -We use @code{"p"} because the numeric prefix argument is never -@code{nil}. Defined in this way, the function does display the -message when called from a keyboard macro. - - The above method with the additional argument is usually best, -because it allows callers to say ``treat this call as interactive''. -But you can also do the job by testing @code{called-interactively-p}. - -@defun called-interactively-p kind -This function returns @code{t} when the calling function was called -using @code{call-interactively}. - -The argument @var{kind} should be either the symbol @code{interactive} -or the symbol @code{any}. If it is @code{interactive}, then -@code{called-interactively-p} returns @code{t} only if the call was -made directly by the user---e.g., if the user typed a key sequence -bound to the calling function, but @emph{not} if the user ran a -keyboard macro that called the function (@pxref{Keyboard Macros}). If -@var{kind} is @code{any}, @code{called-interactively-p} returns -@code{t} for any kind of interactive call, including keyboard macros. - -If in doubt, use @code{any}; the only known proper use of -@code{interactive} is if you need to decide whether to display a -helpful message while a function is running. - -A function is never considered to be called interactively if it was -called via Lisp evaluation (or with @code{apply} or @code{funcall}). -@end defun - -@noindent -Here is an example of using @code{called-interactively-p}: + Generally, use @code{called-interactively-p} to determine whether +the current function's context is interactive. @example @group (defun foo () (interactive) - (when (called-interactively-p 'any) - (message "Interactive!") - 'foo-called-interactively)) + (when (called-interactively-p) + (message "Interactive!"))) @end group @group @@ -975,14 +929,13 @@ Distinguish Interactive @end example @noindent -Here is another example that contrasts direct and indirect calls to -@code{called-interactively-p}. +Contrast direct and indirect calls to @code{called-interactively-p}. @example @group (defun bar () (interactive) - (message "%s" (list (foo) (called-interactively-p 'any)))) + (message "%s" (list (foo) (called-interactively-p)))) @end group @group @@ -991,6 +944,43 @@ Distinguish Interactive @end group @end example + While the call site of @code{called-interactively-p} may lexically +reside within the body of the subject function, nothing in the Lisp +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. + + What is guaranteed is a function's lexical environment. If +distinguishing interactive is critical, instead add to the subject +function an optional argument with interactive spec @code{p} (or +another code that cannot take on a @code{nil} value). This argument +is thus assured to be non-@code{nil} when the subject function assigns +arguments via the @code{interactive} form. For example the following +function: + +@example +(defun foo (bar) + (interactive (list "bar")) + (called-interactively-p)) +@end example + +@noindent +could be rewritten + +@example +(defun foo (bar &optional interactive-p) + (interactive "i\np") + (when interactive-p + (setq bar "bar")) + interactive-p) +@end example + +@noindent +at the cost of obfuscation. + @node Command Loop Info @section Information from the Command Loop @cindex command loop variables diff --git a/lisp/subr.el b/lisp/subr.el index f909b63aabe..3a213fc56c8 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -6061,103 +6061,61 @@ internal--funcall-interactively (symbol-function 'funcall-interactively)) (defun called-interactively-p (&optional kind) - "Return t if the containing function was called by `call-interactively'. -If KIND is `interactive', then return t only if the call was made -interactively by the user, i.e. not in `noninteractive' mode nor -when `executing-kbd-macro'. -If KIND is `any', on the other hand, it will return t for any kind of -interactive call, including being called as the binding of a key or -from a keyboard macro, even in `noninteractive' mode. - -This function is very brittle, it may fail to return the intended result when -the code is debugged, advised, or instrumented in some form. Some macros and -special forms (such as `condition-case') may also sometimes wrap their bodies -in a `lambda', so any call to `called-interactively-p' from those bodies will -indicate whether that lambda (rather than the surrounding function) was called -interactively. - -Instead of using this function, it is cleaner and more reliable to give your -function an extra optional argument whose `interactive' spec specifies -non-nil unconditionally (\"p\" is a good way to do this), or via -\(not (or executing-kbd-macro noninteractive)). - -The only known proper use of `interactive' for KIND is in deciding -whether to display a helpful message, or how to display it. If you're -thinking of using it for any other purpose, it is quite likely that -you're making a mistake. Think: what do you want to do when the -command is called from a keyboard macro?" - (declare (advertised-calling-convention (kind) "23.1")) - (when (not (and (eq kind 'interactive) - (or executing-kbd-macro noninteractive))) - (let* ((i 1) ;; 0 is the called-interactively-p frame. - frame nextframe - (get-next-frame - (lambda () - (setq frame nextframe) - (setq nextframe (backtrace-frame i 'called-interactively-p)) - ;; (message "Frame %d = %S" i nextframe) - (setq i (1+ i))))) - (funcall get-next-frame) ;; Get the first frame. - (while - ;; FIXME: The edebug and advice handling should be made modular and - ;; provided directly by edebug.el and nadvice.el. - (progn - ;; frame =(backtrace-frame i-2) - ;; nextframe=(backtrace-frame i-1) - (funcall get-next-frame) - ;; `pcase' would be a fairly good fit here, but it sometimes moves - ;; branches within local functions, which then messes up the - ;; `backtrace-frame' data we get, - (or - ;; Skip special forms (from non-compiled code). - (and frame (null (car frame))) - ;; Skip also `interactive-p' (because we don't want to know if - ;; interactive-p was called interactively but if it's caller was). - (eq (nth 1 frame) 'interactive-p) - ;; Skip package-specific stack-frames. - (let ((skip (run-hook-with-args-until-success - 'called-interactively-p-functions - i frame nextframe))) - (pcase skip - ('nil nil) - (0 t) - (_ (setq i (+ i skip -1)) (funcall get-next-frame))))))) - ;; Now `frame' should be "the function from which we were called". - (pcase (cons frame nextframe) - ;; No subr calls `interactive-p', so we can rule that out. - (`((,_ ,(pred (lambda (f) (subr-primitive-p (indirect-function f)))) . ,_) . ,_) nil) - ;; In case # without going through the - ;; `funcall-interactively' symbol (bug#3984). - (`(,_ . (t ,(pred (lambda (f) - (eq internal--funcall-interactively - (indirect-function f)))) - . ,_)) - t))))) - -(defun interactive-p () - "Return t if the containing function was run directly by user input. -This means that the function was called with `call-interactively' -\(which includes being called as the binding of a key) -and input is currently coming from the keyboard (not a keyboard macro), -and Emacs is not running in batch mode (`noninteractive' is nil). - -The only known proper use of `interactive-p' is in deciding whether to -display a helpful message, or how to display it. If you're thinking -of using it for any other purpose, it is quite likely that you're -making a mistake. Think: what do you want to do when the command is -called from a keyboard macro or in batch mode? - -To test whether your function was called with `call-interactively', -either (i) add an extra optional argument and give it an `interactive' -spec that specifies non-nil unconditionally (such as \"p\"); or (ii) -use `called-interactively-p'. - -To test whether a function can be called interactively, use -`commandp'." - ;; Kept around for now. See discussion at: - ;; https://lists.gnu.org/r/emacs-devel/2020-08/msg00564.html - (declare (obsolete called-interactively-p "23.2")) - (called-interactively-p 'interactive)) + "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)))))) + +(define-obsolete-function-alias 'interactive-p + #'called-interactively-p "23.2" + "Keep alias (https://lists.gnu.org/r/emacs-devel/2020-08/msg00564.html)") (defun internal-push-keymap (keymap symbol) (let ((map (symbol-value symbol))) diff --git a/src/callint.c b/src/callint.c index d8d2b278458..6259f7d0caa 100644 --- a/src/callint.c +++ b/src/callint.c @@ -233,20 +233,16 @@ read_file_name (Lisp_Object default_filename, Lisp_Object mustmatch, mustmatch, initial, predicate); } -/* 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)); } 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 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)))) @@ -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) -- 2.38.1