GNU bug report logs - #12321
24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults

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 12321 in the body.
You can then email your comments to 12321 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: "Drew Adams" <drew.adams <at> oracle.com>
To: <bug-gnu-emacs <at> gnu.org>
Subject: 24.2.50;
	`read-regexp' parameter DEFAULT-VALUE and the calculated defaults
Date: Fri, 31 Aug 2012 15:38:45 -0700

A couple of things seem odd to me about `read-regexp' as currently
defined.
 
First, there is the business of automatically handling `: ' for the
PROMPT.  I thought we had finally gotten away from this kind of thing.
It just gives callers less control.  And it has sometimes resulted in
confusion and extra work.  I don't see this as a win.  But whatever.
 
More importantly -
 
It seems odd that optional parameter DEFAULT-VALUE is not simply
included among the `M-n' choices (hence the prompt fiddling and the
after-read fiddling if empty input).  And it seems odd that you cannot
pass a list of defaults as the optional parameter.  Why is the argument
handled separately from the calculated list of "standard" defaults?
 
The current version provides a list of defaults, but they are all
hard-coded.  A given calling context might well have its own set of
defaults that it would like to provide.  But it cannot do so (except for
one).
 
Here is a version I am using, which lets the arg be a list of defaults.
That gives callers more control.  Perhaps you might consider it.
(I have not changed the prompt behavior here.)
 
(defun read-regexp (prompt &optional defaults)
  "Read and return a regular expression as a string.
Prompt with PROMPT, which should not include a final `: '.
 
Non-nil optional arg DEFAULTS is a string or a list of strings that
are prepended to a list of standard default values, which include the
string at point, the last isearch regexp, the last isearch string, and
the last replacement regexp."
  (when (and defaults  (atom defaults)) (setq defaults  (list defaults)))
  (let* ((deflts (append
                  defaults
                  (list (regexp-quote
                         (or (funcall
                              (or find-tag-default-function
                                  (get major-mode 'find-tag-default-function)
                                  'find-tag-default))
                             ""))
                        (car regexp-search-ring)
                        (regexp-quote (or (car search-ring)  ""))
                        (car (symbol-value
                              query-replace-from-history-variable)))))
         (deflts (delete-dups (delq nil (delete "" deflts))))
         ;; Do not automatically add INPUT to the history, in case it is "".
         (history-add-new-input  nil)
         (input (read-from-minibuffer
                 (if defaults
                     (format "%s (default `%s'): " prompt
                             (mapconcat 'isearch-text-char-description
                                        (car deflts) ""))
                   (format "%s: " prompt))
                 nil nil nil 'regexp-history deflts t)))
    (if (equal input "")
        (or (car defaults)  input)
      (prog1 input (add-to-history 'regexp-history input)))))
 
Finally, I wonder if it is really appropriate to be filtering out empty
input ("") here.  Perhaps we should let `read-regexp' return "" under
some conditions?  For example, if DEFAULTS contains "", indicating an
explicit intention that the user be allowed to choose an empty regexp?
Seems like something like whether to allow "" should be up to the
caller, not to `read-regexp'.
 
It used to be the case, for instance, that you could enter empty input
for the `occur' regexp, and thus get all of the buffer lines in Occur
mode.  Whether or not that is something useful for `occur', you can
imagine that some caller of `read-regexp' might well want to allow for
reading an empty string as the user input.

In GNU Emacs 24.2.50.1 (i386-mingw-nt5.1.2600)
 of 2012-08-26 on MARVIN
Bzr revision: 109788 dmantipov <at> yandex.ru-20120827041533-3cy7pdjdqz14o90c
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.6) --no-opt --enable-checking --cflags
 -ID:/devel/emacs/libs/libXpm-3.5.8/include
 -ID:/devel/emacs/libs/libXpm-3.5.8/src
 -ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
 -ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
 -ID:/devel/emacs/libs/giflib-4.1.4-1/include
 -ID:/devel/emacs/libs/jpeg-6b-4/include
 -ID:/devel/emacs/libs/tiff-3.8.2-1/include
 -ID:/devel/emacs/libs/gnutls-3.0.9/include
 -ID:/devel/emacs/libs/libiconv-1.13.1-1-dev/include
 -ID:/devel/emacs/libs/libxml2-2.7.8/include/libxml2'
 

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

From: Juri Linkov <juri <at> jurta.org>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 12321 <at> debbugs.gnu.org
Subject: Re: bug#12321: 24.2.50;
	`read-regexp' parameter DEFAULT-VALUE and the calculated defaults
Date: Sat, 01 Sep 2012 15:04:49 +0300

Do you think your new report could be merged with bug#7567?

PS: See also http://thread.gmane.org/gmane.emacs.devel/134852
and http://thread.gmane.org/gmane.emacs.devel/104590

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

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Juri Linkov'" <juri <at> jurta.org>
Cc: 12321 <at> debbugs.gnu.org
Subject: RE: bug#12321: 24.2.50;
	`read-regexp' parameter DEFAULT-VALUE and the calculated defaults
Date: Sat, 1 Sep 2012 09:52:44 -0700

> Do you think your new report could be merged with bug#7567?
> 
> PS: See also http://thread.gmane.org/gmane.emacs.devel/134852
> and http://thread.gmane.org/gmane.emacs.devel/104590

Good question.
Wrt bug #7567 and the first URL you mention: I think not.

Bug #7567 has a long and winding road, that is not particularly coherent.  I
don't see that it touches on what I reported at all.  But if it does then the
touching is probably tangential.  Please keep this bug separate.

The only thing in common with the first URL you cite is the part about the
prompt string.  If you like, you can ignore that part of bug @12321, at least
for now.

The second URL you sent is germane, yes.  Eli's point/question is the same one I
raise in bug #12321.  But he did not file a bug report.

The detour in the second URL thread, which you introduced (changing the Subject
line - thank you for that), is not germane, AFAICT.  But the main thread, with
the original subject line, is pertinent.

AFAICT, the issue raised by Eli (and now by me) was never addressed.  The URL
thread ended with no real answer wrt this issue.

In sum, bug #12321 stands on its own.  It should not be merged with another bug.
You can ignore the prompt thing for now, if you like.  And we can deal later, if
there is interest, with the possibility of adding completion (discussed in bug
#7567).

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

From: Juri Linkov <juri <at> jurta.org>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 12321 <at> debbugs.gnu.org
Subject: Re: bug#12321: 24.2.50; `read-regexp' parameter PROMPT
Date: Thu, 20 Sep 2012 11:18:10 +0300

> First, there is the business of automatically handling `: ' for the
> PROMPT.  I thought we had finally gotten away from this kind of thing.
> It just gives callers less control.

There are three contradicting requirements:

1. Maintain backward-compatibility for the PROMPT arg
   in all existing calls to `read-regexp'.

2. Do not append automatically ": " to the end of PROMPT.
   As bug#7567 demonstrates, more control for callers is necessary
   to be able to use `read-regexp' in such specific contexts
   as in `query-replace-read-from' that constructs prompt
   according to the special format "%s (default %s -> %s): ".

3. The code should not be so "clever" in supporting
   two these options above at the same time, as you said in
   http://thread.gmane.org/gmane.emacs.devel/134852/focus=134854

But from these three options I see no better thing to do than
to allow `read-regexp' to be "clever" (like `read-buffer'):

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2012-09-09 22:15:24 +0000
+++ lisp/replace.el	2012-09-20 08:15:20 +0000
@@ -597,10 +597,12 @@ (defun read-regexp (prompt &optional def
 	 (history-add-new-input nil)
 	 (input
 	  (read-from-minibuffer
-	   (if default-value
-	       (format "%s (default %s): " prompt
-		       (query-replace-descr default-value))
-	     (format "%s: " prompt))
+	   (if (string-match-p ":[ \t]*\\'" prompt)
+	       prompt
+	     (if default-value
+		 (format "%s (default %s): " prompt
+			 (query-replace-descr default-value))
+	       (format "%s: " prompt)))
 	   nil nil nil 'regexp-history defaults t)))
     (if (equal input "")
 	(or default-value input)

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

From: Juri Linkov <juri <at> jurta.org>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 12321 <at> debbugs.gnu.org
Subject: Re: bug#12321: 24.2.50;
	`read-regexp' parameter DEFAULT-VALUE and the calculated defaults
Date: Thu, 20 Sep 2012 11:43:45 +0300

> It seems odd that optional parameter DEFAULT-VALUE is not simply
> included among the `M-n' choices (hence the prompt fiddling and the
> after-read fiddling if empty input).  And it seems odd that you cannot
> pass a list of defaults as the optional parameter.  Why is the argument
> handled separately from the calculated list of "standard" defaults?

This can be fixed by the following patch.  It is based on two
previously sent patches (one that fixes the PROMPT arg, and
another that adds the HISTORY arg to `read-regexp' in bug#7567),
so `read-regexp' has the `history' arg in this third patch.
These three patches could be committed separately.  No doc fix yet,
just confirmed that it works predictably in all my tests.

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2012-09-09 22:15:24 +0000
+++ lisp/replace.el	2012-09-20 08:42:58 +0000
@@ -575,7 +575,7 @@ (defvar regexp-history nil
 (defvar occur-collect-regexp-history '("\\1")
   "History of regexp for occur's collect operation")
 
-(defun read-regexp (prompt &optional default-value history)
+(defun read-regexp (prompt &optional defaults history)
   "Read regexp as a string using the regexp history and some useful defaults.
 Prompt for a regular expression with PROMPT (without a colon and
 space) in the minibuffer.  The optional argument DEFAULT-VALUE
@@ -585,7 +585,10 @@ (defun read-regexp (prompt &optional def
 If HISTORY is nil, `regexp-history' is used.
 Values available via M-n are the string at point, the last isearch
 regexp, the last isearch string, and the last replacement regexp."
-  (let* ((defaults
+  (let* ((default (if (consp defaults) (car defaults) defaults))
+	 (defaults
+	   (append
+	    (if (listp defaults) defaults (list defaults))
 	   (list (regexp-quote
 		  (or (funcall (or find-tag-default-function
 				   (get major-mode 'find-tag-default-function)
@@ -594,7 +597,7 @@ (defun read-regexp (prompt &optional def
 		 (car regexp-search-ring)
 		 (regexp-quote (or (car search-ring) ""))
 		 (car (symbol-value
-		       query-replace-from-history-variable))))
+			query-replace-from-history-variable)))))
 	 (defaults (delete-dups (delq nil (delete "" defaults))))
 	 ;; Don't add automatically the car of defaults for empty input
 	 (history-add-new-input nil)
@@ -602,13 +605,13 @@ (defun read-regexp (prompt &optional def
 	  (read-from-minibuffer
 	   (if (string-match-p ":[ \t]*\\'" prompt)
 	       prompt
-	     (if default-value
+	     (if default
 		 (format "%s (default %s): " prompt
-			 (query-replace-descr default-value))
+			 (query-replace-descr default))
 	       (format "%s: " prompt)))
 	   nil nil nil (or history 'regexp-history) defaults t)))
     (if (equal input "")
-	(or default-value input)
+	(or default input)
       (prog1 input
 	(add-to-history (or history 'regexp-history) input)))))
 

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: 12321 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#12321: 24.2.50; `read-regexp' parameter PROMPT
Date: Thu, 20 Sep 2012 08:46:10 -0400

> -	   (if default-value
> -	       (format "%s (default %s): " prompt
> -		       (query-replace-descr default-value))
> -	     (format "%s: " prompt))
> +	   (if (string-match-p ":[ \t]*\\'" prompt)
> +	       prompt
> +	     (if default-value
> +		 (format "%s (default %s): " prompt
> +			 (query-replace-descr default-value))
> +	       (format "%s: " prompt)))

That looks OK, tho please use `cond' then.


        Stefan

Message #25 received at 12321-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 12321-done <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#12321: 24.2.50; `read-regexp' parameter PROMPT
Date: Fri, 21 Sep 2012 00:59:08 +0300

> That looks OK, tho please use `cond' then.

Done.

Previous Next

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