[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#35231: 26.1; Redefine `read-command' behavior for empty input and DE

From: Drew Adams
Subject: bug#35231: 26.1; Redefine `read-command' behavior for empty input and DEFAULT not a command name
Date: Tue, 9 Jul 2019 09:17:55 -0700 (PDT)

> > This is a followup to bug #35222 (and thanks for fixing that doc bug).
> >
> > As discussed in #35222, the current C-code definition of `read-command'
> > seems not so desirable, when it comes to DEFAULT = nil or "".
> >
> > The behavior in that case is to return an uninterned symbol whose name
> > is empty, which has the read/print syntax `##'.
> >
> > This is _not_ a symbol whose function definition is `commandp'.  There
> > is nothing about this symbol that qualifies for consideration as a
> > command.  It seems wrong (a bug) for `read-command' to ever return a
> > symbol whose function definition is `commandp'.
> That is, indeed, pretty odd.  The function isn't used a lot in Emacs:
> ./lisp/subr.el1088:           (read-command (format "Set key %s to command:
> "
> ./lisp/edmacro.el118:      (setq cmd (read-command "Name of keyboard
> macro to edit: "))
> ./lisp/emulation/edt.el1130:  (setq edt-function (read-command "Enter
> command name: "))
> ./lisp/info.el4526:       (read-command "Find documentation for command:
> ")))
> ./lisp/strokes.el448:    (read-command "Command to map stroke to: ")))
> But some of them expect this behaviour:
>            (setq cmd (read-command "Name of keyboard macro to edit: "))
>            (if (string-equal cmd "")
>                (error "No command name given"))
>       (setq edt-function (read-command "Enter command name: "))
>       (if (string-equal "" edt-function)
>           (message "Key not defined")
> So changing the behaviour is probably not a good idea -- there may be
> out-of-tree usages.  So I think we'll have to live with it, and I'm
> closing this bug report.

Just for the record:

I don't think this is the best way to handle this.
It seems clear from those rare examples you show
that they do what they do only to _work around_
the odd (bugged) behavior.  It would be easy to
fix them.  Likewise, any 3rd-party code.

And in fact, with a fixed `read-command' no change
would really be needed to any such caller.  The
test for equality with "" would simply never
succeed.  The test would be superfluous, but if
it were not removed there would be no loss.

I made good suggestions for how this function can
be better defined, and how to handle the (minor)
incompatible change (fix) for returning a non
command name.

The function can be trivially defined in Lisp, in
such a way that users can benefit from completion
and no caller will need to test for a non-command
name (whether odd, such as "", or not).

That's the way to improve Emacs, here.  Closing
the bug because there may be callers that "depend"
on the bugged behavior in the way you indicated
(i.e., as a bug workaround, for the odd possibility
that a non-command name would be read) doesn't
make sense to me.

This bug is simple to fix properly, and I see no
downside to that being done.

It's also possible (but it shouldn't be necessary)
to define a new function, with a different name,
and use that.  And declare the old one, with the
bug, defined in C, as obsolete.

FWIW, I use this (in Dired+):

(defun diredp-read-command (&optional prompt default)
  "Read the name of a command and return a symbol with that name.
\(A command is anything that satisfies predicate `commandp'.)
Prompt with PROMPT, which defaults to \"Command: \".
By default, return the command named DEFAULT (or, with Emacs 23+, its
first element if DEFAULT is a list).  (If DEFAULT does not name a
command then it is ignored.)"
  (setq prompt  (or prompt  "Command: "))
  (let ((name  (completing-read prompt obarray #'commandp t nil
                                'extended-command-history default)))
    (while (string= "" name)
      (setq name  (completing-read prompt obarray #'commandp t nil
                                   'extended-command-history default)))
    (intern name)))

Any good reason why such a definition (or
similar) should not be used as a replacement
for the current, bugged, `read-command'?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]