all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Drew Adams <drew.adams@oracle.com>
To: Lars Ingebrigtsen <larsi@gnus.org>
Cc: 35231@debbugs.gnu.org
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)	[thread overview]
Message-ID: <cadcfd01-0cee-4fc5-b443-381076b90138@default> (raw)
In-Reply-To: <87ef2ztdx3.fsf@mouse.gnus.org>

> > 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	other threads:[~2019-07-09 16:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11 15:10 bug#35231: 26.1; Redefine `read-command' behavior for empty input and DEFAULT not a command name Drew Adams
2019-07-09 15:39 ` Lars Ingebrigtsen
2019-07-09 16:17   ` Drew Adams [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cadcfd01-0cee-4fc5-b443-381076b90138@default \
    --to=drew.adams@oracle.com \
    --cc=35231@debbugs.gnu.org \
    --cc=larsi@gnus.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.