From ddde988f37c5b8a0ca688b62ab6bc66f075dbcf0 Mon Sep 17 00:00:00 2001 From: dickmao Date: Fri, 3 Feb 2023 12:12:35 -0500 Subject: [PATCH] Deprecate called-interactively-p * doc/lispref/commands.texi (Distinguish Interactive): Deprecate. * lisp/subr.el (called-interactively-p): Deprecate. (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, edebug-tests-call-interactively-instrumented-func, 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 | 91 ++++--------------- lisp/subr.el | 55 +---------- src/callint.c | 15 ++- .../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, 61 insertions(+), 135 deletions(-) diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi index dc78adc4520..446d44bf8b0 100644 --- a/doc/lispref/commands.texi +++ b/doc/lispref/commands.texi @@ -906,89 +906,30 @@ 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: +The function @code{called-interactively-p} is deprecated. -@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}: +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 -@group -(defun foo () - (interactive) - (when (called-interactively-p 'any) - (message "Interactive!") - 'foo-called-interactively)) -@end group - -@group -;; @r{Type @kbd{M-x foo}.} - @print{} Interactive! -@end group - -@group -(foo) - @result{} nil -@end group +(defun foo (bar) + (interactive (list "bar")) + (when (called-interactively-p) + (message "bar is %s" bar))) @end example @noindent -Here is another example that contrasts direct and indirect calls to -@code{called-interactively-p}. +should be rewritten @example -@group -(defun bar () - (interactive) - (message "%s" (list (foo) (called-interactively-p 'any)))) -@end group - -@group -;; @r{Type @kbd{M-x bar}.} - @print{} (nil t) -@end group +(defun foo (bar &optional verbose-p) + (interactive "i\np") + (when verbose-p + (setq bar "bar") + (message "bar is %s" bar))) @end example @node Command Loop Info diff --git a/lisp/subr.el b/lisp/subr.el index f909b63aabe..f5add8c63ae 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -6061,32 +6061,8 @@ 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")) + "Deprecated. +Refer to Info node `(elisp)Distinguish Interactive'." (when (not (and (eq kind 'interactive) (or executing-kbd-macro noninteractive))) (let* ((i 1) ;; 0 is the called-interactively-p frame. @@ -6134,30 +6110,9 @@ called-interactively-p . ,_)) 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)) +(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..6be96273f95 100644 --- a/src/callint.c +++ b/src/callint.c @@ -233,20 +233,19 @@ read_file_name (Lisp_Object default_filename, Lisp_Object mustmatch, mustmatch, initial, predicate); } -/* BEWARE: Calling this directly from C would defeat the purpose! */ +/* This primitive should only be called from lisp and not C since its + very purpose is to appear as a literal token in the lisp call + stack. +*/ + 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. 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..478de8177f7 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) + (apply #'funcall-interactively f args) + (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