GNU bug report logs - #47286
28.0.50; [PATCH] Replace "(default %s)" with 'format-prompt'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47286 in the body.
You can then email your comments to 47286 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Gabriel <gabriel376 <at> hotmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; [PATCH] Replace "(default %s)" with 'format-prompt'
Date: Sat, 20 Mar 2021 19:22:52 -0300

[Message part 1 (text/plain, inline)]

This patches replaces several occurences of the hardcoded minibuffer
default prompt "(default %s)" with a call to 'format-prompt'.  It also
removes the suffix ": ", when applicable, that is automatically handled
by 'format-prompt'.

[0001-Replace-minibuffer-default-prompt-default-s-with-for.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

Thread on emacs-devel:
https://lists.gnu.org/archive/html/emacs-devel/2021-03/msg00817.html

Let me know if something is missing, wrong or can be improved in this
patch.

Regards,
Gabriel

Message #8 received at 47286 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gabriel <gabriel376 <at> hotmail.com>
Cc: 47286 <at> debbugs.gnu.org
Subject: Re: bug#47286: 28.0.50;
 [PATCH] Replace "(default %s)" with 'format-prompt'
Date: Sun, 21 Mar 2021 08:07:44 +0200

> From: Gabriel <gabriel376 <at> hotmail.com>
> Date: Sat, 20 Mar 2021 19:22:52 -0300
> 
> This patches replaces several occurences of the hardcoded minibuffer
> default prompt "(default %s)" with a call to 'format-prompt'.  It also
> removes the suffix ": ", when applicable, that is automatically handled
> by 'format-prompt'.

Thanks.  A minor nit about formatting the commit log messages in such
cases, where the same change is made in many places.

> * lisp/cmuscheme.el (scheme-load-file, scheme-compile-file): Remove
> prompt suffix ": ".
> 
> * lisp/comint.el (comint-get-source): Use 'format-prompt'.
> 
> * lisp/emulation/viper-cmd.el (viper-quote-region, viper-kill-buffer)
> (viper-query-replace): Remove prompt suffix ": ".
> (viper-read-string-with-history): Use 'format-prompt'.
> 
> * lisp/eshell/esh-mode.el (eshell-find-tag): Remove prompt suffix ": ".
> 
> * lisp/gnus/gnus-sum.el (gnus-articles-to-read)
> (gnus-summary-search-article-forward)
> (gnus-summary-search-article-backward): Use 'format-prompt'.

Instead of having a long series of basically identical text, it is
better to format this as follows:

* lisp/cmuscheme.el (scheme-load-file, scheme-compile-file):
* lisp/comint.el (comint-get-source):
* lisp/emulation/viper-cmd.el (viper-quote-region, viper-kill-buffer)
(viper-query-replace, viper-read-string-with-history):
* lisp/eshell/esh-mode.el (eshell-find-tag):
* lisp/gnus/gnus-sum.el (gnus-articles-to-read)
(gnus-summary-search-article-forward)
(gnus-summary-search-article-backward):
* lisp/international/mule-cmds.el (set-input-method, toggle-input-method)
(describe-input-method, set-language-environment)
(describe-language-environment):
* lisp/mh-e/mh-gnus.el (mh-mml-minibuffer-read-disposition)
(mh-insert-letter)
* lisp/mh-e/mh-mime.el (mh-display-with-external-viewer)
(mh-mime-save-parts, mh-mh-forward-message)
(mh-mml-query-cryptographic-method): Remove prompt suffix and
use 'format-prompt'.

That is, have the list of files and functions, and put the description
of the change only in the last one.  IME, this makes the long list
easier to read and understand.

Message #11 received at 47286 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Gabriel <gabriel376 <at> hotmail.com>
Cc: 47286 <at> debbugs.gnu.org
Subject: Re: 28.0.50; [PATCH] Replace "(default %s)" with 'format-prompt'
Date: Sun, 21 Mar 2021 16:53:21 +0100

Gabriel <gabriel376 <at> hotmail.com> writes:

> * lisp/progmodes/project.el (project--completing-read-strict)
> (project--completing-read-strict): Use 'format-prompt'.

Mmm, project.el is released on ELPA as a :core package; IIUC it strives
for backward-compatibility as far back as Emacs 26.1
(cf. Package-Requires field at the top of the file), so maybe it's a bit
early to use format-prompt there?

Apologies if I'm misunderstanding something.

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Gabriel <gabriel376 <at> hotmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#47286: 28.0.50; [PATCH] Replace "(default %s)" with
 'format-prompt'
Date: Sun, 21 Mar 2021 15:22:53 -0300

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:
>
> That is, have the list of files and functions, and put the description
> of the change only in the last one.  IME, this makes the long list
> easier to read and understand.

Hi Eli, thank you for your suggestion, looks much better! I will make
sure my next patches follow this format, if applicable.

Patch with commit message updated:

[0001-Replace-default-s-with-format-prompt.patch (text/x-diff, attachment)]

Message #17 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Gabriel <gabriel376 <at> hotmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#47286: 28.0.50; [PATCH] Replace "(default %s)" with
 'format-prompt'
Date: Sun, 21 Mar 2021 15:31:29 -0300

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>
> Mmm, project.el is released on ELPA as a :core package; IIUC it strives
> for backward-compatibility as far back as Emacs 26.1
> (cf. Package-Requires field at the top of the file), so maybe it's a bit
> early to use format-prompt there?
>
> Apologies if I'm misunderstanding something.

Hi Kevin,

Thanks for pointing it out. I just searched for the current source tree
of master branch for ocurrences of "(default %s)", so I am not sure if I
changed some package that shouldn't be update. If so, I will promptly
update the patch.

Perhaps project.el could use a similar fallback that exists on
cperl-mode.el (commit 3a95f4fc12df0a570ad413e099a22bf901b7346d):

;; Available in Emacs 28: format-prompt
(defalias 'cperl--format-prompt
  (if (fboundp 'format-prompt) 'format-prompt
    (lambda (msg default)
      (if default (format "%s (default %s): " msg default)
        (concat msg ": ")))))

Message #20 received at 47286 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Gabriel <gabriel376 <at> hotmail.com>, 47286 <at> debbugs.gnu.org
Subject: Re: bug#47286: 28.0.50; [PATCH] Replace "(default %s)" with
 'format-prompt'
Date: Sun, 21 Mar 2021 20:57:04 +0200

On 21.03.2021 20:31, Gabriel wrote:
> Thanks for pointing it out. I just searched for the current source tree
> of master branch for ocurrences of "(default %s)", so I am not sure if I
> changed some package that shouldn't be update. If so, I will promptly
> update the patch.
> 
> Perhaps project.el could use a similar fallback that exists on
> cperl-mode.el (commit 3a95f4fc12df0a570ad413e099a22bf901b7346d):
> 
> ;; Available in Emacs 28: format-prompt
> (defalias 'cperl--format-prompt
>    (if (fboundp 'format-prompt) 'format-prompt
>      (lambda (msg default)
>        (if default (format "%s (default %s): " msg default)
>          (concat msg ": ")))))

Sounds reasonable to me.

Also note that project--completing-read-strict includes as (string-equal 
default "") check added in 2be537e64.

Perhaps format-prompt could use it as well.

Message #23 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Gabriel <gabriel376 <at> hotmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#47286: 28.0.50; [PATCH] Replace "(default %s)" with
 'format-prompt'
Date: Sun, 21 Mar 2021 21:03:15 -0300

Dmitry Gutov <dgutov <at> yandex.ru> writes:
> Also note that project--completing-read-strict includes as (string-equal default
> "") check added in 2be537e64.
>
> Perhaps format-prompt could use it as well.

Do you mean to add a new check to 'format-prompt' to consider an empty
DEFAULT as nil, e.g., to not show it on minibuffer prompt in case the
value of the DEFAULT argument is an empty string ?

Also, maybe another useful check for 'format-prompt' would remove the
": " suffix from the PROMPT argument, to avoid displaying something like
"prompt: : ", which could avoid regressions when migrating existing
code.

Example of the new 'format-prompt':

(defun format-prompt (prompt default &rest format-args)
  "Format PROMPT with DEFAULT according to `minibuffer-default-prompt-format'.
If FORMAT-ARGS is nil, PROMPT is used as a plain string.  If
FORMAT-ARGS is non-nil, PROMPT is used as a format control
string, and FORMAT-ARGS are the arguments to be substituted into
it.  See `format' for details.

If DEFAULT is a list, the first element is used as the default.
If not, the element is used as is.

If DEFAULT is nil or an empty string, no \"default value\" string
is included in the return value."
  (let* ((suffix ": ")
         (prompt (if (string-suffix-p suffix prompt)
                     (substring prompt 0 (* -1 (length suffix)))
                   prompt))
         (default (if (equal "" default)
                      nil
                    default))
         (formatted-prompt (if (null format-args)
                               prompt
                             (apply #'format prompt format-args)))
         (formatted-default (and default
                                 (format minibuffer-default-prompt-format
                                         (if (consp default)
                                             (car default)
                                           default)))))
    (concat formatted-prompt
            formatted-default
            suffix)))

Message #26 received at 47286 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Gabriel <gabriel376 <at> hotmail.com>, 47286 <at> debbugs.gnu.org
Subject: Re: bug#47286: 28.0.50; [PATCH] Replace "(default %s)" with
 'format-prompt'
Date: Mon, 22 Mar 2021 02:48:56 +0200

On 22.03.2021 02:03, Gabriel wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>> Also note that project--completing-read-strict includes as (string-equal default
>> "") check added in 2be537e64.
>>
>> Perhaps format-prompt could use it as well.
> 
> Do you mean to add a new check to 'format-prompt' to consider an empty
> DEFAULT as nil, e.g., to not show it on minibuffer prompt in case the
> value of the DEFAULT argument is an empty string ?

Yup.

> Also, maybe another useful check for 'format-prompt' would remove the
> ": " suffix from the PROMPT argument, to avoid displaying something like
> "prompt: : ", which could avoid regressions when migrating existing
> code.

My (weak) preference would be on "no": whether the prompt is showing an 
extra ":" should be apparent from very brief testing of the code. It's 
not like it will show up only if DEFAULT is non-nil, for instance.

So no need to complicate the implementation.

Message #29 received at 47286 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Gabriel <gabriel376 <at> hotmail.com>
Cc: 47286 <at> debbugs.gnu.org
Subject: Re: bug#47286: 28.0.50; [PATCH] Replace "(default %s)" with
 'format-prompt'
Date: Mon, 22 Mar 2021 20:06:37 +0100

Gabriel <gabriel376 <at> hotmail.com> writes:

> Thanks for pointing it out. I just searched for the current source tree
> of master branch for ocurrences of "(default %s)", so I am not sure if I
> changed some package that shouldn't be update. If so, I will promptly
> update the patch.

The files that have a line like

;; Package-Requires: ((emacs "26.1") (xref "1.0.2"))

or similar should be handled with more care, yes -- they have to be
backwards-compatible, and `format-prompt' is new in Emacs 28.  So if you
could re-spin the patch without changes to files like that, I'll get it
applied (it looks good otherwise).

There are some packages that are maintained for older Emacs versions,
too, but we unfortunately have no markup to say that explicitly.
Looking over the list, I think this affects only these two files, and
should also be removed from the patch:

 lisp/progmodes/cc-styles.el
 lisp/progmodes/verilog-mode.el 


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #32 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Gabriel <gabriel376 <at> hotmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#47286: 28.0.50; [PATCH] Replace "(default %s)" with
 'format-prompt'
Date: Tue, 23 Mar 2021 00:47:55 -0300

[Message part 1 (text/plain, inline)]

>
> The files that have a line like
>
> ;; Package-Requires: ((emacs "26.1") (xref "1.0.2"))
>
> or similar should be handled with more care, yes -- they have to be
> backwards-compatible, and `format-prompt' is new in Emacs 28.  So if you
> could re-spin the patch without changes to files like that, I'll get it
> applied (it looks good otherwise).
>
> There are some packages that are maintained for older Emacs versions,
> too, but we unfortunately have no markup to say that explicitly.
> Looking over the list, I think this affects only these two files, and
> should also be removed from the patch:
>
>  lisp/progmodes/cc-styles.el
>  lisp/progmodes/verilog-mode.el

Hi Lars,

Thanks so much for your review. Here is an updated patch that removes
the mentioned files (project.el, cc-styles.el and verilog-mode.el) and
updates the 'format-prompt' function with the suggestion from Dmitry
Gutov to consider an empty DEFAULT as nil. Let me know if something
could be improved.

[0001-Replace-default-s-with-format-prompt.patch (text/x-diff, attachment)]

Message #35 received at 47286 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Gabriel <gabriel376 <at> hotmail.com>
Cc: 47286 <at> debbugs.gnu.org
Subject: Re: bug#47286: 28.0.50; [PATCH] Replace "(default %s)" with
 'format-prompt'
Date: Wed, 24 Mar 2021 10:32:31 +0100

Gabriel <gabriel376 <at> hotmail.com> writes:

> Thanks so much for your review. Here is an updated patch that removes
> the mentioned files (project.el, cc-styles.el and verilog-mode.el) and
> updates the 'format-prompt' function with the suggestion from Dmitry
> Gutov to consider an empty DEFAULT as nil. Let me know if something
> could be improved.

Looks good to me (although that latter change could probably have been a
separate patch for clarity).  Pushed now to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.