all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Ryan Thompson <rct@thompsonclan.org>
To: Drew Adams <drew.adams@oracle.com>, 27158@debbugs.gnu.org
Subject: bug#27158: 25.2; Eliminating old usage of completing-read from built-in files
Date: Wed, 31 May 2017 11:45:33 +0000	[thread overview]
Message-ID: <CAHCt_aZSuuuZ8EY5ETtNiWN69P_3xygGgRsT+D1GAVgkF56RDw@mail.gmail.com> (raw)
In-Reply-To: <0276a0cd-874b-47ee-a2dc-fe8ca08ece9d@default>

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

>
> The default arg has been available since the beginning, AFAIK.
> The behavior you describe is no more legacy than is providing a
> default-value arg.  Nothing to do with backward compatibility,
> I think.


I suppose I inferred that it was a backward-compatibility issue from the
fact that the feature is used intentionally in very few places (all fairly
old code) and is triggered unintentionally in more places, i.e. more
recently-written functions that set REQUIRE-MATCH to t and then assume that
completing-read will always return an element from the collection. The
latter kind will typically produce an error or do something nonsensical for
the empty string.

And it returns the default, even if REQUIRE-MATCH is non-nil
> and the default is not in the collection.  Do you dislike
> that behavior too?
>
> "" is just the default default, if you like (or not).
>

Thanks, that's actually a good way to think about it.


> Would you make a default arg be mandatory instead of optional?
> Is that it?  If not, what default default would you propose?
> What should be returned if no explicit default is provided and
> the user hits RET with no input?
>

Thinking some more about it, I guess one solution would be to give
REQUIRE-MATCH could gain a special 3rd value, such as `force', which would
mandate that the returned value is a valid completion. (Not sure what
should happen in this case if DEF is provided and is not a valid
completion.)

"" seems like a great choice for the default default, to me.
> It's pretty much guaranteed never to coincide with any usable
> completion candidate (which is not the case for a non-empty
> default value).  For one thing, that lets you easily check
> whether the user chose one of the candidates, even in the
> case where the collection is complex (e.g. a function).
>
> > This quirk
>
> Why is it a quirk?
>
> IN any case, nothing stops someone from defining their own
> `my-completing-read', which does not have this feature, er,
> quirk.  I don't see the problem.
>

The problem comes when implementing a completion system that returns the
first available completion when pressiong RET (e.g. ido and ivy). In the
case of an empty input, RET causes these completion systems to return the
first element of COLLECTION if no default was specified. Sometimes this is
correct, but sometimes this is wrong, as in the following usage pattern:

(defun pick-a-fruit ()
  (interactive)
  (let* ((default "banana")
         (prompt (format "Pick a fruit (default %s): " default))
         (collection '("apple" "banana" "cherry"))
         (selection (completing-read prompt collection nil t)))
    (when (string= selection "")
      (setq selection default))
    (message "You selected %s" selection)))

This pattern is used in e.g. Info-follow-reference. The problem is that
there's no way for the completion function to know whether it's being used
in this way or not, so if the user presses RET on an empty input and the
default wasn't provied to completing-read, it doesn't know whether it
should return the empty string or the first match. For an example where
returning the first match seems like the more correct behavior, see
read-char-by-name, which throws an error on the empty string. This
ambiguity makes it difficult to reconcile the desired convenience feature
of "return the first match on RET" with the documented completing-read
behavior of "return the empty string on empty input + RET" without breaking
some functions. And it's not even possible to implement an automatic
fallback to completing-read-default, because there's no way for the
completion function to know whether the caller is expecting that behavior
(Info-follow-reference) or is expecting it not to happen
(read-char-by-name). Ultimately, that's the issue: if completing-read is
called with a DEF being nil, the calling code may or may not be expecting
it to return the empty string, and while not expecting the empty string
might represent a bug in the caller, the existence of such functions makes
it difficult to implement ido-style completion that does the right thing in
all cases.

Going back to your statement that "empty string is just the default
default", it's possible that one might get reasonable behavior for ido and
ivy by taking this statement literally and prepending "" to the list of
completions when DEF is nil. By "reasonable" I mean correct behavior in all
of the cases that completing-read-default is correct, and bug-for-bug
compatibility in functions that don't expect completing-read to return "".
I'll try that out and see how it works.

Thanks for the insights. I guess you can close this.

[-- Attachment #2: Type: text/html, Size: 6803 bytes --]

  reply	other threads:[~2017-05-31 11:45 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-31  4:41 bug#27158: 25.2; Eliminating old usage of completing-read from built-in files Ryan
2017-05-31  5:52 ` Drew Adams
2017-05-31 11:45   ` Ryan Thompson [this message]
2017-05-31 14:51     ` Drew Adams
2017-05-31 12:23   ` Dmitry Gutov
2017-05-31 14:51     ` Drew Adams
2017-05-31 14:59       ` Dmitry Gutov
2017-05-31 15:19         ` Drew Adams
2017-05-31 15:44           ` Ryan Thompson
2017-05-31 22:41             ` Dmitry Gutov
2017-05-31 23:16               ` Drew Adams
2017-05-31 23:54                 ` Dmitry Gutov
2017-06-01  2:23                   ` Drew Adams
2017-06-01  9:27                     ` Dmitry Gutov
2017-06-01 14:57                       ` Drew Adams
2017-06-01 20:53                         ` Dmitry Gutov
2017-06-01 21:04                           ` Ryan Thompson
2017-06-05 23:01                             ` Dmitry Gutov
2017-06-06  0:06                               ` Ryan Thompson
2017-06-06  0:09                                 ` Dmitry Gutov
2017-05-31 21:20           ` Dmitry Gutov
2020-08-24 14:58 ` Lars Ingebrigtsen

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=CAHCt_aZSuuuZ8EY5ETtNiWN69P_3xygGgRsT+D1GAVgkF56RDw@mail.gmail.com \
    --to=rct@thompsonclan.org \
    --cc=27158@debbugs.gnu.org \
    --cc=drew.adams@oracle.com \
    /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.