bug#17151: 24.3.50; Disabled commands are executed anyway

bug#17151: 24.3.50; Disabled commands are executed anyway

[Nicolas Richard]

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

I report two problems, but they really are the same bug.

1st problem:
From emacs -Q :
select e.g. the first line of the scratch buffer
hit C-x n n
(that is a disabled command, so you get a prompt)
answer 'n' to the prompt
the buffer is narrowed anyway.

I expected nothing to happen wrt narrowing.

2nd problem:
From emacs -Q, eval :

(defun my-command ()
  (interactive)
  (message "Got called !"))
(put 'my-command 'disabled t)

then:
M-x my-command RET
(you get a prompt)
hit SPC to execute just once and leave disabled
hit C-h e to show the *Messages* buffer
You can see:
Got called ! [2 times]

I expected to see "Got called !" (only once).

Both of these seem to be fixed with the attached patch.

However, with this patch, when `disabled-command-function' decides to
run the command, it does so by invoking call-interactively, thereby not
executing what command-execute would have done if the command had not
been disabled. It doesn't seem right but I'm not sure (and have no idea
what to change anyway).

Here's the patch :

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-simple.el-command-execute-Do-not-execute-disabl.patch --]
[-- Type: text/x-diff, Size: 4161 bytes --]

From d5259941c61fb9de3967135925d1649a3aaa9381 Mon Sep 17 00:00:00 2001
From: Nicolas Richard <theonewiththeevillook@yahoo.fr>
Date: Mon, 31 Mar 2014 19:41:22 +0200
Subject: [PATCH] * lisp/simple.el (command-execute): Do not execute disabled
 commands.

---
 lisp/ChangeLog |  4 ++++
 lisp/simple.el | 60 +++++++++++++++++++++++++++++-----------------------------
 2 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 747bbcf..6ad449f 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,7 @@
+2014-03-31  Nicolas Richard  <theonewiththeevillook@yahoo.fr>
+
+	* simple.el (command-execute): Do not execute the command when it
+	is disabled.
 2014-03-29  Juri Linkov  <juri@jurta.org>
 
 	* dired-aux.el (dired-compress-file): Don't use string-match-p
diff --git a/lisp/simple.el b/lisp/simple.el
index 98604a4..b469fc1 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1628,36 +1628,36 @@ a special event, so ignore the prefix argument and don't clear it."
                      (prog1 prefix-arg
                        (setq current-prefix-arg prefix-arg)
                        (setq prefix-arg nil)))))
-    (and (symbolp cmd)
-         (get cmd 'disabled)
-         ;; FIXME: Weird calling convention!
-         (run-hooks 'disabled-command-function))
-    (let ((final cmd))
-      (while
-          (progn
-            (setq final (indirect-function final))
-            (if (autoloadp final)
-                (setq final (autoload-do-load final cmd)))))
-      (cond
-       ((arrayp final)
-        ;; If requested, place the macro in the command history.  For
-        ;; other sorts of commands, call-interactively takes care of this.
-        (when record-flag
-          (push `(execute-kbd-macro ,final ,prefixarg) command-history)
-          ;; Don't keep command history around forever.
-          (when (and (numberp history-length) (> history-length 0))
-            (let ((cell (nthcdr history-length command-history)))
-              (if (consp cell) (setcdr cell nil)))))
-        (execute-kbd-macro final prefixarg))
-       (t
-        ;; Pass `cmd' rather than `final', for the backtrace's sake.
-        (prog1 (call-interactively cmd record-flag keys)
-          (when (and (symbolp cmd)
-                     (get cmd 'byte-obsolete-info)
-                     (not (get cmd 'command-execute-obsolete-warned)))
-            (put cmd 'command-execute-obsolete-warned t)
-            (message "%s" (macroexp--obsolete-warning
-                           cmd (get cmd 'byte-obsolete-info) "command")))))))))
+    (if  (and (symbolp cmd)
+              (get cmd 'disabled))
+        ;; FIXME: Weird calling convention!
+        (run-hooks 'disabled-command-function)
+      (let ((final cmd))
+        (while
+            (progn
+              (setq final (indirect-function final))
+              (if (autoloadp final)
+                  (setq final (autoload-do-load final cmd)))))
+        (cond
+         ((arrayp final)
+          ;; If requested, place the macro in the command history.  For
+          ;; other sorts of commands, call-interactively takes care of this.
+          (when record-flag
+            (push `(execute-kbd-macro ,final ,prefixarg) command-history)
+            ;; Don't keep command history around forever.
+            (when (and (numberp history-length) (> history-length 0))
+              (let ((cell (nthcdr history-length command-history)))
+                (if (consp cell) (setcdr cell nil)))))
+          (execute-kbd-macro final prefixarg))
+         (t
+          ;; Pass `cmd' rather than `final', for the backtrace's sake.
+          (prog1 (call-interactively cmd record-flag keys)
+            (when (and (symbolp cmd)
+                       (get cmd 'byte-obsolete-info)
+                       (not (get cmd 'command-execute-obsolete-warned)))
+              (put cmd 'command-execute-obsolete-warned t)
+              (message "%s" (macroexp--obsolete-warning
+                             cmd (get cmd 'byte-obsolete-info) "command"))))))))))
 \f
 (defvar minibuffer-history nil
   "Default minibuffer history list.
-- 
1.9.1


[-- Attachment #3: Type: text/plain, Size: 10 bytes --]

-- 
Nico.

bug#17151: 24.3.50; Disabled commands are executed anyway

[Glenn Morris]

Thanks; applied.

It would be good if someone could write a test-case for this.
(Maybe use a custom disabled-command-function.)