all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Drew Adams" <drew.adams@oracle.com>
To: "'MON KEY'" <monkey@sandpframing.com>, <emacs-devel@gnu.org>
Subject: RE: functions which read by prompting should not add additional chars to prompt string
Date: Sat, 22 Jan 2011 19:06:03 -0800	[thread overview]
Message-ID: <04AC080BE7B240DC88EB86EF60623900@us.oracle.com> (raw)
In-Reply-To: <AANLkTi=0nGio6WWN0gTFdh6=xfgg85edr0Yk9HSndCme@mail.gmail.com>

> Lisp functions which read user input and accept a PROMPT arg should
> not append additional characters to the supplied PROMPT string
> esp. when no additional arguments are supplied.

Not sure we need an explicit rule to handle the problems you point to.  But I
also don't have any special use case for such a function tacking on extra chars,
and I agree about the principle as well as the cases that need to be fixed.

It would be one thing to express such a rule wrt Emacs's own sources (makes
sense); it would be another thing to proclaim it wider (e.g. as a convention,
say, in the doc).

I would say let's just take care of the concrete problems you point to, and then
adhere to the same approach generally (in Emacs sources), without promulgating
another rule to users.

>  (if default-value
>      (format "%s (default %s): " prompt
>              (query-replace-descr default-value))
>    (format "%s: " prompt))
> This should be:
>   (format "%s" prompt)

1+.

IMO there is no need to add the default value, whatever it might be, to the
prompt string.  As always, it is available via `M-n'.  There's nothing special
about `read-regexp' (or the other functions mentioned below) in this regard.

However, this function is weird wrt the default value.  DEFAULT-VALUE is not
included in the list of defaults passed to `read-from-minibuffer', so it is not
even accessible using `M-n'.  It is accessible _only_ by hitting RET with empty
input.  Why the weird design here?

If there is no good reason for it then let's get rid of this confusing
exception.  Just put DEFAULT-VALUE at the head of the defaults list, and let it
go at that.

[Another point about `read-regexp': It is a completely general function, and as
such should be moved out of `replace.el'.  There is already enough stuff in that
file that is not specific to replacement (e.g. `occur').  Presumably it is there
only out of laziness: someone added it for use in `replace.el' code, and it was
never moved to a better home.]

> Each of the above rely on the value of (car regexp-history) for
> DEFAULT-VALUE.  IOW, even where callers do supply DEFAULT-VALUE there
> is no guarantee that the car of `regexp-history' isn't null.

Null (car regexp-history) is not a problem, that I can see.

> A good fix might be to adjust the existing semantics of `read-regexp's
> DEFAULT-VALUE to accept a boolean which when t defaults to whatever is
> at the head `regexp-history' e.g.:
>  (and (eq default-value t) (setq default-value (car regexp-history)))

Not a good idea, IMO.  No gain, and some loss of clarity (more doc lookup, for
little info).

`read-regexp' puts a bunch of defaults ahead of the history, for `M-n'.  If a
caller wants to privilege (car regexp-history), then it's better for everyone if
that is explicit in the call.  We gain nothing by identifying an apparently
common case ((car regexp-history)) and adding a layer of source
indirection/obfuscation by having `t' stand for it.

> Following two functions, `read-face-name' and `read-quoted-char' both
> step on the supplied PROMPT and should be fixed:
> (read-face-name   "face ")
> (read-quoted-char "char ")

1+.  Let the caller supply the prompt, including any `: '.  Always.  Unless
there is some special countervailing reason.

And let users get the default values with `M-n' - forget about adding defaults
to prompts (that's not done consistently anyway).

> `read-buffer' may step on the PROMPT string only when DEF is non-nil:
> (read-buffer "buffer ")

1+.  Oddly, here the C source apparently tries to compensate for the presence or
absence of a space and a colon in the supplied PROMPT.  These all produce the
same prompt:

(read-buffer "foo:" "toto")
(read-buffer "foo: " "toto")
(read-buffer "foo "  "toto")
(read-buffer "foo"   "toto")

The code should not be so "clever".  Again, let the caller be responsible for
the prompt.  Likewise for `read-buffer-to-switch' (which just calls
`read-buffer').

In sum, I agree with you: 1+.

All such attempts at "clever prompting" or easing the caller's burden (just some
guesses at the motivations) are misguided.  Keep it simple.




  reply	other threads:[~2011-01-23  3:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-22 20:38 functions which read by prompting should not add additional chars to prompt string MON KEY
2011-01-23  3:06 ` Drew Adams [this message]
2011-01-24  2:57   ` Stefan Monnier

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=04AC080BE7B240DC88EB86EF60623900@us.oracle.com \
    --to=drew.adams@oracle.com \
    --cc=emacs-devel@gnu.org \
    --cc=monkey@sandpframing.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.