Re: bug: "C-x z" ("repeat") no longer works correctly with M-x

[Aaron S. Hawley]

On 6/2/12, Chong Yidong <address@hidden> wrote:
> "Aaron S. Hawley" <address@hidden> writes:
>
>> I had given a more literal translation of the C command to the
>> gnu-emacs-sources mailing list back in March.  Stefan and I discussed
>> it but hadn't run across this issue with repeat.  He later made some
>> modifications to my submission and committed it in May.  I have only
>> now run it and can confirm this bug with repeat.  I'm not sure what's
>> causing it though, but found the following fixes it.  Again, I'm not
>> sure why.  Nor am I sure if it's a wholly appropriate fix.
>
> No, tweaking the interactive spec does not really fix the problem.
>
> The problem is that the old execute-extended-command sets the
> real_this_command internal variable, which causes the Emacs command loop
> to record the command that was actually executed into real-last-command
> and last-repeatable-command.
>
> In other words, it's not just the fact that `C-x z' doesn't work
> properly.  Moving execute-extended-command to Lisp produces a
> backward-incompatible change in the values of the real-last-command and
> last-repeatable-command variables for M-x.  I suspect this may break
> things other than `C-x z'.  I guess we could fix this by exposing
> real_this_command to Lisp too, but that kinda defeats the point of that
> variable...
>
> Is there a strong rationale for moving execute-extended-command to Lisp,
> other than the general principle that we want as much functionality
> implemented Lisp as possible?  If not, it may not be worth making this
> change, at least in Emacs 24.2, due to the risk of gratuitous
> incompatibilities.

I agree, it's not worth it if things break.  As I wrote to
gnu-emacs-sources, I've been using a Lisp version of M-x for several
years.  Obviously, I ran into issues when I first tried writing the
code, but haven't run into any issues since.  I am a big user of
repeat and repeat-complex-command and keyboard macros.  I think it's
possible to get this Lisp version to work as well as the C version.

I understand your concern about real_this_command in command_loop and
call-interactively.  However, it's not clear if there's any reason for
it for M-x.  I'd be interested to know if there other issues.  Perhaps
the timeline of 24.2 won't allow for it?

I've made sure repeat works if M-x is a mouse event by adding it to
the tool-bar or the menu-bar.

(define-key menu-bar-tools-menu [m-x]
  `(menu-item ,(purecopy "M-x") execute-extended-command
              :enable (menu-bar-non-minibuffer-window-p)
              :help ,(purecopy "Execute extended command")))

(tool-bar-add-item-from-menu 'execute-extended-command "mpc/add"
                             nil :label "Execute extended command"
                             :vert-only t)

I do suggest the following fixes to rework Stefan's revision,
including the change to the interactive spec.  Some of it is white
space changes and a comment that no longer applies.

2012-05-29  Aaron S. Hawley  <address@hidden>

        * simple.el (execute-extended-command): Reading command-name in
        interactive form breaks the repeat command.  Improve error message
        for empty command-name.

--- simple.el   2012-05-29 08:21:45.000000000 -0400
+++ simple.el   2012-05-29 18:46:04.836717200 -0400
@@ -1373,17 +1373,20 @@

 Noninteractively, the argument PREFIXARG is the prefix argument to
 give to the command you invoke, if it asks for an argument."
-  (interactive (list current-prefix-arg (read-extended-command)))
+  (interactive "P")
   ;; Emacs<24 calling-convention was with a single `prefixarg' argument.
-  (if (null command-name) (setq command-name (read-extended-command)))
+  (when (null command-name)
+    ;; Could be in `interactive' as well, but it breaks `repeat'.
+    (setq command-name (read-extended-command)))
   (let* ((function (and (stringp command-name) (intern-soft command-name)))
          (binding (and suggest-key-bindings
-                        (not executing-kbd-macro)
-                        (where-is-internal function overriding-local-map t))))
+                       (not executing-kbd-macro)
+                       (where-is-internal function overriding-local-map t))))
+    (when (and (stringp command-name)
+               (= 0 (length command-name)))
+      (error "No command name given"))
     (unless (commandp function)
       (error "`%s' is not a valid command name" command-name))
-    ;; Set this_command_keys to the concatenation of saved-keys and
-    ;; function, followed by a RET.
     (setq this-command function)
     (let ((prefix-arg prefixarg))
       (command-execute function 'record))

-- 
In general, we reserve the right to have a poor
memory--the computer, however, is supposed to
remember!  Poor computer.  -- Guy Lewis Steele Jr.