* bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults
@ 2012-08-31 22:38 Drew Adams
2012-09-01 12:04 ` Juri Linkov
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Drew Adams @ 2012-08-31 22:38 UTC (permalink / raw)
To: 12321
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@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'
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults
2012-08-31 22:38 bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults Drew Adams
@ 2012-09-01 12:04 ` Juri Linkov
2012-09-01 16:52 ` Drew Adams
2012-09-20 8:18 ` bug#12321: 24.2.50; `read-regexp' parameter PROMPT Juri Linkov
2012-09-20 8:43 ` bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults Juri Linkov
2 siblings, 1 reply; 7+ messages in thread
From: Juri Linkov @ 2012-09-01 12:04 UTC (permalink / raw)
To: Drew Adams; +Cc: 12321
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults
2012-09-01 12:04 ` Juri Linkov
@ 2012-09-01 16:52 ` Drew Adams
0 siblings, 0 replies; 7+ messages in thread
From: Drew Adams @ 2012-09-01 16:52 UTC (permalink / raw)
To: 'Juri Linkov'; +Cc: 12321
> 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).
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12321: 24.2.50; `read-regexp' parameter PROMPT
2012-08-31 22:38 bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults Drew Adams
2012-09-01 12:04 ` Juri Linkov
@ 2012-09-20 8:18 ` Juri Linkov
2012-09-20 12:46 ` Stefan Monnier
2012-09-20 8:43 ` bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults Juri Linkov
2 siblings, 1 reply; 7+ messages in thread
From: Juri Linkov @ 2012-09-20 8:18 UTC (permalink / raw)
To: Drew Adams; +Cc: 12321
> 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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults
2012-08-31 22:38 bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults Drew Adams
2012-09-01 12:04 ` Juri Linkov
2012-09-20 8:18 ` bug#12321: 24.2.50; `read-regexp' parameter PROMPT Juri Linkov
@ 2012-09-20 8:43 ` Juri Linkov
2 siblings, 0 replies; 7+ messages in thread
From: Juri Linkov @ 2012-09-20 8:43 UTC (permalink / raw)
To: Drew Adams; +Cc: 12321
> 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)))))
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12321: 24.2.50; `read-regexp' parameter PROMPT
2012-09-20 8:18 ` bug#12321: 24.2.50; `read-regexp' parameter PROMPT Juri Linkov
@ 2012-09-20 12:46 ` Stefan Monnier
2012-09-20 21:59 ` Juri Linkov
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2012-09-20 12:46 UTC (permalink / raw)
To: Juri Linkov; +Cc: 12321
> - (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
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12321: 24.2.50; `read-regexp' parameter PROMPT
2012-09-20 12:46 ` Stefan Monnier
@ 2012-09-20 21:59 ` Juri Linkov
0 siblings, 0 replies; 7+ messages in thread
From: Juri Linkov @ 2012-09-20 21:59 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 12321-done
> That looks OK, tho please use `cond' then.
Done.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-09-20 21:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-31 22:38 bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults Drew Adams
2012-09-01 12:04 ` Juri Linkov
2012-09-01 16:52 ` Drew Adams
2012-09-20 8:18 ` bug#12321: 24.2.50; `read-regexp' parameter PROMPT Juri Linkov
2012-09-20 12:46 ` Stefan Monnier
2012-09-20 21:59 ` Juri Linkov
2012-09-20 8:43 ` bug#12321: 24.2.50; `read-regexp' parameter DEFAULT-VALUE and the calculated defaults Juri Linkov
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).