bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el (read-regexp): Let-bind `default' to the first

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el (read-regexp): Let-bind `default' to the first

[Juri Linkov]

> emacs -Q
> M-s h r
>
> I find the following error
>
> ,----
> | Debugger entered--Lisp error: (error "Regexp cannot match an empty string")
> |   signal(error ("Regexp cannot match an empty string"))
> |   error("Regexp cannot match an empty string")
> |   hi-lock-regexp-okay("")
> |   byte-code("..." [regexp-history hi-lock-regexp-okay read-regexp "Regexp to highlight" hi-lock-read-face-name] 4)
> |   call-interactively(highlight-regexp nil nil)
> |   command-execute(highlight-regexp)
> `----
>
> So, if one does
>         (read-regexp something) ;; something is nil or evals to nil
>
> what should the interpretation be.
>
> With your change, a `nil' default will provide an empty string as input
> and force user to enter a regexp or rely on M-n.
>
> We seem to be bumping in to each other in this area.  Comments ...?
>
> ,---- Stefan @ http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13687#11
> | I disagree: read-regexp is a generic function which can be used in
> | various contexts, some of which might not care at all about the text
> | around point.  So the caller should have control over the first default
> | (of course, it's perfectly fine to always add the current tag in the
> | subsequent defaults).
> |
> | This said your patch seems to leave the caller's provided `defaults' at
> | the beginning of the minibuffer's `defaults', so I think your patch is
> | fine, feel free to install it.
> `----
>
> I am wondering how we can resolve the contex-free read-regexp and
> context-dependent regexp.  Any suggestions?

It's a responsibility of the caller to provide the default value.
`M-s h r' (`highlight-regexp') provides the default value as
`(car regexp-history)'.  When it is called the first time after
`emacs -Q', the history is empty, so its default value is nil
(this fact is indicated with missing default value in the prompt,
so the user is aware that RET with empty input will do nothing.)

When `highlight-regexp' is called the second time and more,
it gets the default value from `regexp-history', so you can't
provide the tag at point as the default for later invocations
of `highlight-regexp' anyway.

So the question is: should the default value in the caller
`highlight-regexp' be changed from `(car regexp-history)'
to code that gets the tag at point?  You could propose
such a change, but since it changes the long-standing behavior,
expect some disagreement (not from me :-)

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el (read-regexp): Let-bind `default' to the first

[Juri Linkov]

> So the question is: should the default value in the caller
> `highlight-regexp' be changed from `(car regexp-history)'
> to code that gets the tag at point?  You could propose
> such a change, but since it changes the long-standing behavior,
> expect some disagreement (not from me :-)

There are surprisingly many users who prefer the previous item from the
history instead of the tag at point as the default value of `occur',
`highlight-regexp', `rgrep'.  They got `(car regexp-history)' hard-coded
into core Emacs for `occur' and `highlight-regexp', but not for `rgrep',
so they raise the questions how to do the same for `rgrep', and get such
horribly ugly solutions as this one:

http://stackoverflow.com/questions/15161592/make-emacs-rgrep-default-to-last-search-term-rather-than-word-at-point

This situation suggests that the default values should be customizable.
Possible variants:

1. Put a special value on the command's symbol like:
   (put 'highlight-regexp 'default 'history)
   (put 'highlight-regexp 'default 'tag-at-point)
   checked on `this-command' in minibuffer-reading functions.
   Cons: Not easy to use.

2. Add a new defcustom like:
   (defcustom minibuffer-defaults '((highlight-regexp-default . tag-at-point)
                                    (rgrep . history)
                                    (occur . tag-at-point)
                                    (how-many . history)
                                    ...))
   Cons: Too large list of commands for one option.

3. In the DEFAULT arg of minibuffer-reading calls
   specify a function that returns default values:

   (defun highlight-regexp (regexp &optional face)
     (interactive
      (list
       (read-regexp "Regexp to highlight" 'highlight-regexp-default)
       ...

   (defun highlight-regexp-default ()
     (car regexp-history))

   where users can override it with another function:

   (defun highlight-regexp-default ()
     (let* ((tagf (or find-tag-default-function
			     (get major-mode 'find-tag-default-function)
			     'find-tag-default))
		   (tag (funcall tagf)))
	      (cond ((not tag) "")
		    ((eq tagf 'find-tag-default)
		     (format "\\_<%s\\_>" (regexp-quote tag)))
		    (t (regexp-quote tag)))))

   Pros: Flexible and easy to configure.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el (read-regexp): Let-bind `default' to the first

[Jambunathan K]

Juri Linkov <juri@jurta.org> writes:

>> So the question is: should the default value in the caller
>> `highlight-regexp' be changed from `(car regexp-history)'
>> to code that gets the tag at point?  You could propose
>> such a change, but since it changes the long-standing behavior,
>> expect some disagreement (not from me :-)
>
> There are surprisingly many users who prefer the previous item from the
> history instead of the tag at point as the default value of `occur',
> `highlight-regexp', `rgrep'.  They got `(car regexp-history)' hard-coded
> into core Emacs for `occur' and `highlight-regexp', but not for `rgrep',
> so they raise the questions how to do the same for `rgrep', and get such
> horribly ugly solutions as this one:

See bug#13892.  I have gone with (3).  For now, I have modified
hi-lock.el.  Modifications to occur will follow soon.


> http://stackoverflow.com/questions/15161592/make-emacs-rgrep-default-to-last-search-term-rather-than-word-at-point
>
> This situation suggests that the default values should be customizable.
> Possible variants:
>
> 1. Put a special value on the command's symbol like:
>    (put 'highlight-regexp 'default 'history)
>    (put 'highlight-regexp 'default 'tag-at-point)
>    checked on `this-command' in minibuffer-reading functions.
>    Cons: Not easy to use.
>
> 2. Add a new defcustom like:
>    (defcustom minibuffer-defaults '((highlight-regexp-default . tag-at-point)
>                                     (rgrep . history)
>                                     (occur . tag-at-point)
>                                     (how-many . history)
>                                     ...))
>    Cons: Too large list of commands for one option.
>
> 3. In the DEFAULT arg of minibuffer-reading calls
>    specify a function that returns default values:
>
>    (defun highlight-regexp (regexp &optional face)
>      (interactive
>       (list
>        (read-regexp "Regexp to highlight" 'highlight-regexp-default)
>        ...
>
>    (defun highlight-regexp-default ()
>      (car regexp-history))
>
>    where users can override it with another function:
>
>    (defun highlight-regexp-default ()
>      (let* ((tagf (or find-tag-default-function
> 			     (get major-mode 'find-tag-default-function)
> 			     'find-tag-default))
> 		   (tag (funcall tagf)))
> 	      (cond ((not tag) "")
> 		    ((eq tagf 'find-tag-default)
> 		     (format "\\_<%s\\_>" (regexp-quote tag)))
> 		    (t (regexp-quote tag)))))
>
>    Pros: Flexible and easy to configure.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el (read-regexp): Let-bind `default' to the first

[Jambunathan K]

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

Juri Linkov <juri@jurta.org> writes:

> 2. Add a new defcustom like:
>    (defcustom minibuffer-defaults '((highlight-regexp-default . tag-at-point)
>                                     (rgrep . history)
>                                     (occur . tag-at-point)
>                                     (how-many . history)
>                                     ...))
>    Cons: Too large list of commands for one option.

I am attaching a patch that handles `occur'.  I will commit this patch
tomorrow.

ps: I have never used how-many. The defaults for grep and rgrep never
bothered me.  So I leave these two commands out of my current scope.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: replace.el.diff --]
[-- Type: text/x-diff, Size: 2147 bytes --]

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2013-03-08 08:11:28 +0000
+++ lisp/ChangeLog	2013-03-08 12:52:16 +0000
@@ -1,5 +1,11 @@
 2013-03-08  Jambunathan K  <kjambunathan@gmail.com>
 
+	* replace.el (occur-read-regexp-defaults-function): New var.
+	(occur-read-regexp-defaults): New defun.
+	(occur-read-primary-args): Propagate above change (bug#13892).
+
+2013-03-08  Jambunathan K  <kjambunathan@gmail.com>
+
 	* hi-lock.el (hi-lock-read-regexp-defaults-function): New var.
 	(hi-lock-read-regexp-defaults):	 New defun.
 	(hi-lock-line-face-buffer, hi-lock-face-buffer)

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2013-03-08 04:18:16 +0000
+++ lisp/replace.el	2013-03-08 12:45:40 +0000
@@ -1135,12 +1135,33 @@ which means to discard all text properti
   :group 'matching
   :version "22.1")
 
+(defvar occur-read-regexp-defaults-function
+  'occur-read-regexp-defaults
+  "Function that provides default regexp(s) for occur commands.
+This function should take no arguments and return one of nil, a
+regexp or a list of regexps for use with occur commands -
+`occur', `multi-occur' and `multi-occur-in-matching-buffers'.
+The return value of this function is used as DEFAULTS param of
+`read-regexp' while executing the occur command.  This function
+is called only during interactive use.
+
+For example, to check for occurrence of a symbol at point by
+default use
+
+    \(setq occur-read-regexp-defaults-function
+	  'find-tag-default-as-regexp\).")
+
+(defun occur-read-regexp-defaults ()
+  "Return the latest regexp from `regexp-history'.
+See `occur-read-regexp-defaults-function' for details."
+  (car regexp-history))
+
 (defun occur-read-primary-args ()
   (let* ((perform-collect (consp current-prefix-arg))
          (regexp (read-regexp (if perform-collect
                                   "Collect strings matching regexp"
                                 "List lines matching regexp")
-                              (car regexp-history))))
+                              (funcall occur-read-regexp-defaults-function))))
     (list regexp
 	  (if perform-collect
 	      ;; Perform collect operation

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> > 2. Add a new defcustom like:
> >    (defcustom minibuffer-defaults 
> >      '((highlight-regexp-default . tag-at-point)
> >        (rgrep . history)
> >        (occur . tag-at-point)
> >        (how-many . history)
> >        ...))
> >    Cons: Too large list of commands for one option.
> 
> I am attaching a patch that handles `occur'.
> I will commit this patch tomorrow.

Sorry, I have not been following this thread at all - please excuse any jumping
to wrong conclusions.

If that defcustom is what it looks like to me, I'd say that such an approach is
quite misguided.  I.e., if you are defining a user option that globally controls
defaulting for commands reading input from the minibuffer, that's a bad idea,
IMHO.

Let the command decide its own defaulting.  Individual commands that read the
minibuffer should control such defaulting, at the point of call.

They can rely on user options if they want to.  But a single user option for
this kind of thing, and especially if it somehow takes control of defaulting
away from the caller (command), is a bad idea.

And doubly so if it does command lookup to determine the default value to use.
That's the real mistake, IMHO.

FWIW, in my code I do something that I'm guessing might be similar to the
_effect_ you want, but I stay completely away from command lookup/dispatching.

I have a user option, `search/replace-default-fn', whose value (a function)
provides the default value(s) for functions `query-replace', `occur',
`how-many', `flush-lines',...

This means that, a priori, the same option value is used for all such functions.
But a command can override this.  And the function that is the option value can
itself do the kind of dispatching that you are doing, if that is something the
user really wants.

code: http://www.emacswiki.org/emacs/download/replace%2b.el
doc:  http://www.emacswiki.org/cgi-bin/wiki/ReplacePlus

I also wonder whether you are perhaps changing the user experience here without
a proper proposal and discussion in emacs-devel first.  "I will
commit...tomorrow." does not inspire confidence.  How about proposing a change
and seeing what others in the development list think?

Again, if I have misunderstood, mille excuses.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

"Drew Adams" <drew.adams@oracle.com> writes:

>> > 2. Add a new defcustom like:
>> >    (defcustom minibuffer-defaults 
>> >      '((highlight-regexp-default . tag-at-point)
>> >        (rgrep . history)
>> >        (occur . tag-at-point)
>> >        (how-many . history)
>> >        ...))
>> >    Cons: Too large list of commands for one option.
>> 
>> I am attaching a patch that handles `occur'.
>> I will commit this patch tomorrow.
>
> Sorry, I have not been following this thread at all - please excuse
> any jumping to wrong conclusions.

Excused.  You didn't look at the inline patch that I enclosed.

> I also wonder whether you are perhaps changing the user experience
> here without a proper proposal and discussion in emacs-devel first.

You seem to think that emacs-bugs is not a right place to discuss a
proposal.

> "I will commit...tomorrow." does not inspire confidence.  How about
> proposing a change and seeing what others in the development list
> think?

It is happening as we type, don't you think.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> > I also wonder whether you are perhaps changing the user experience
> > here without a proper proposal and discussion in emacs-devel first.
> 
> You seem to think that emacs-bugs is not a right place to discuss a
> proposal.

You are correct - that it is what I think.

It is not the best place to discuss a user-visible behavior change.
emacs-devel@gnu.org is the list for discussing such changes.  Some interested in
Emacs development follow that list but do not follow the bugs list.

Yes, sometimes a bug report leads to further discussion that involves
potentially changing behavior for users.  And yes, sometimes that change is
minimal and it is OK to decide here and let the wider community know via NEWS.

And yes, sometimes it happens that discussion of even more substantial changes
does not get moved from here to emacs-devel, out of laziness or whatever.  But
that is a weakness to be overcome.  The wider Emacs development community
deserves to follow and weigh in on user-visible behavior changes.

> > "I will commit...tomorrow." does not inspire confidence.  How about
> > proposing a change and seeing what others in the development list
> > think?
> 
> It is happening as we type, don't you think.

No, it is not.  It is buried within a particular bug thread on a separate
mailing list from emacs-devel@gnu.org.

I would guess (but haven't checked) that many more people subscribe to
emacs-devel than subscribe to the bug mailing list.  Likewise would be my guess
wrt those who read the dev and bug threads in other ways.

Make a proposal for the behavior change and present it to emacs-devel.  Based on
the discussion (if any) and the decision reached there, you can then follow up
with the bug fix.  Reference the bug thread in the emacs-devel discussion via
the bug # or its URL (but don't cc both lists).

That's the right way, IMHO.  It is not always followed correctly, unfortunately.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

You are silent about the patch...

You may also be interested in 

        http://bzr.savannah.gnu.org/lh/emacs/trunk/revision/111971

I hear you.  I some how think that emacs-devel doesn't deserve to be
spammed by small time development efforts.

The way I have been working till now is to create a bug report (you may
call it a proposal) and provide a patch myself.  I want my
reports/proposals to be tracked.  If discussions happen on emacs-devel
then it may or may not be tracked.  For example, look at this thread

        http://lists.gnu.org/archive/html/emacs-devel/2013-03/msg00033.html

There are just 2 users talking.  This possibly because the discussion is
not critical for the current release cycle.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Juri Linkov]

> Let the command decide its own defaulting.  Individual commands that read the
> minibuffer should control such defaulting, at the point of call.

Since when the command commands the user, and not other way round?

> They can rely on user options if they want to.  But a single user option for
> this kind of thing, and especially if it somehow takes control of defaulting
> away from the caller (command), is a bad idea.

Perhaps you missed the whole thread, but see what the users are forced to use

http://stackoverflow.com/questions/15161592/make-emacs-rgrep-default-to-last-search-term-rather-than-word-at-point

in the absence of customizable options.

> FWIW, in my code I do something that I'm guessing might be similar to the
> _effect_ you want, but I stay completely away from command lookup/dispatching.

There is no command lookup/dispatching since Jambunathan implements
the third variant, and not the second cited above.

> I have a user option, `search/replace-default-fn', whose value (a function)
> provides the default value(s) for functions `query-replace', `occur',
> `how-many', `flush-lines',...

Your `search/replace-default-fn' is exactly the same thing as
`occur-read-regexp-defaults-function', so why do you object?

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> > Let the command decide its own defaulting.  Individual 
> > commands that read the minibuffer should control such defaulting,
> > at the point of call.
> 
> Since when the command commands the user, and not other way round?

Straw man.  No one proposed that the command command the user.

It is normal, typical, and traditional, for a command that calls a function to
read input, to provide whatever defaults are appropriate.  Appropriate for that
particular command.  And of course appropriate for a user who chooses to invoke
that command.

> Perhaps you missed the whole thread, but see what the users 
> are forced to use
>
http://stackoverflow.com/questions/15161592/make-emacs-rgrep-default-to-last-sea
rch-term-rather-than-word-at-point
> in the absence of customizable options.

Did you hear me propose to get rid of customizable options?  FWIW, I see nothing
wrong with the request of that user.

What I suggested would be a misguided approach is to have an option that tries
to configure defaulting for a whole slew of possibly unrelated commands.  That's
all.

I pointed to an alternative approach that provides an option specific to a
command or to a group of related commands, the option value being a
default-value-providing function.  All commands in the group use the same
defaulting function.

I even mentioned that if some user wanted to be more specific about defaulting
behavior within the group of related commands, s?he could always do so by
providing as the option value a function that dispatches among the commands -
just like option `minibuffer-defaults'.  The design would not encourage that,
but it would anyway allow for it.

E.g., in the code I cited, if a user does not want the same defaulting behavior
for commands `occur', `how-many', etc., she can set option
`search/replace-default-fn' to a function that distinguishes them (e.g., using
`this-command', as Jambunathan suggested).

But a priori (according to the developer of replace+.el), a user would want the
same defaulting behavior for such commands, and that's why they were grouped to
use a common option.  If it was thought that most users would want separate
defaulting behavior here, then the design would reflect that, providing separate
options.

The point is to keep things narrowed in focus, promoting modularity.

I would group some commands together for convenience in customizing their
defaulting behavior, but only if such grouping (common behavior) made sense.

You would apparently group any and all commands - across the entire Emacs
spectrum, providing a single option (`minibuffer-defaults') that dispatches
according to the given command.

IMO, that's not the best approach; that's all.

> > FWIW, in my code I do something that I'm guessing might be 
> > similar to the _effect_ you want, but I stay completely away
> > from command lookup/dispatching.
> 
> There is no command lookup/dispatching since Jambunathan implements
> the third variant, and not the second cited above.

Sorry, dunno what that means.  I was writing only in reaction to the (your?)
proposed `minibuffer-defaults' option that is an alist of (COMMAND . FUNCTION)
entries and that seemed to me to dispatch defaulting based on the COMMAND.

> > I have a user option, `search/replace-default-fn', whose 
> > value (a function) provides the default value(s) for functions 
> > `query-replace', `occur', `how-many', `flush-lines',...
> 
> Your `search/replace-default-fn' is exactly the same thing as
> `occur-read-regexp-defaults-function', so why do you object?

See above.

1. I admitted that I did not follow the thread.

2. I made clear that I was commenting only on the proposal of a
`minibuffer-defaults' option that dispatches defaulting according to the
command.

3. Wrt that proposal I suggested an alternative approach.
If such an alternative is what is really being used, wunderbar.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

"Drew Adams" <drew.adams@oracle.com> writes:

> E.g., in the code I cited, if a user does not want the same defaulting
> behavior for commands `occur', `how-many', etc., she can set option
> `search/replace-default-fn' to a function that distinguishes them
> (e.g., using `this-command', as Jambunathan suggested).

Interesting suggestion there.

This makes me think that there is no need for multiple
`hi-lock-read-regexp-defaults-function' and a separate
`occur-read-regexp-defaults-function' etc.  But a single
`read-regexp-defaults-function' that cases on `this-command'.

The function can return a symbol token like `t' for `this-command's
which it doesn't want to meddle with but return nil or a regexp or list
of regexps for commands it wants to insinuate.

Is there any problem with this `read-regexp-defaults-function' approach?

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> > E.g., in the code I cited, if a user does not want the same 
> > defaulting behavior for commands `occur', `how-many', etc.,
> > she can set option `search/replace-default-fn' to a function
> > that distinguishes them (e.g., using `this-command', as
> > Jambunathan suggested).
> 
> Interesting suggestion there.
> 
> This makes me think that there is no need for multiple
> `hi-lock-read-regexp-defaults-function' and a separate
> `occur-read-regexp-defaults-function' etc.  But a single
> `read-regexp-defaults-function' that cases on `this-command'.

The question, as I said, is whether it makes sense, for the particular commands
that we group to use the same option, to provide the default regexp (or other
string) in the same way.

I can't speak to whether that is the case for hi-lock, occur, etc.  But if it is
true, then yes, a single option for such a group of commands makes sense.

> The function can return a symbol token like `t' for
> `this-command's which it doesn't want to meddle with but
> return nil or a regexp or list of regexps for commands it
> wants to insinuate.

That is not what I suggested.  I suggested that the option value be a function
that returns a string to use as the default value when reading user input.

What I said in the passage you cite is that that function (the value of the
option) could, if the user so wants, itself test `this-command' and provide a
different string depending on the current command.

> Is there any problem with this 
> `read-regexp-defaults-function' approach?

I think you're suggesting that the option value be a function that returns t or
nil, instead of returning a default-value string.  It's not clear to me how a
given command such as `occur' would make use of that Boolean return value.

As I noted before, I would not _encourage_ users to use a dispatching function
as the option value, but that would not (could not) be prevented.  They can do
anything they want using any function they want.

The out-of-the-box design should make a reasonable assumption about which
commands to group (i.e., which should use the same option).

If it is expected that some command that reads a regexp would generally be
better off with a different defaulting behavior, then that command should not
use the group option.  It could use its own, similar option, with a different
default option value (a different default-value-providing function).  Or it
could hard-code its defaulting, or whatever.

The use of a function to dispatch according to the current command should be
exceptional, IMO - only a fallback possibility and not something to be
encouraged.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

Suspend your judgement.  I don't like to discuss (or rather I cannot
discuss) in abstract.  It is so error prone.  I will show my wares
tomorrow.


"Drew Adams" <drew.adams@oracle.com> writes:

>> > E.g., in the code I cited, if a user does not want the same 
>> > defaulting behavior for commands `occur', `how-many', etc.,
>> > she can set option `search/replace-default-fn' to a function
>> > that distinguishes them (e.g., using `this-command', as
>> > Jambunathan suggested).
>> 
>> Interesting suggestion there.
>> 
>> This makes me think that there is no need for multiple
>> `hi-lock-read-regexp-defaults-function' and a separate
>> `occur-read-regexp-defaults-function' etc.  But a single
>> `read-regexp-defaults-function' that cases on `this-command'.
>
> The question, as I said, is whether it makes sense, for the particular commands
> that we group to use the same option, to provide the default regexp (or other
> string) in the same way.
>
> I can't speak to whether that is the case for hi-lock, occur, etc.  But if it is
> true, then yes, a single option for such a group of commands makes sense.
>
>> The function can return a symbol token like `t' for
>> `this-command's which it doesn't want to meddle with but
>> return nil or a regexp or list of regexps for commands it
>> wants to insinuate.
>
> That is not what I suggested.  I suggested that the option value be a function
> that returns a string to use as the default value when reading user input.
>
> What I said in the passage you cite is that that function (the value of the
> option) could, if the user so wants, itself test `this-command' and provide a
> different string depending on the current command.
>
>> Is there any problem with this 
>> `read-regexp-defaults-function' approach?
>
> I think you're suggesting that the option value be a function that returns t or
> nil, instead of returning a default-value string.  It's not clear to me how a
> given command such as `occur' would make use of that Boolean return value.
>
> As I noted before, I would not _encourage_ users to use a dispatching function
> as the option value, but that would not (could not) be prevented.  They can do
> anything they want using any function they want.
>
> The out-of-the-box design should make a reasonable assumption about which
> commands to group (i.e., which should use the same option).
>
> If it is expected that some command that reads a regexp would generally be
> better off with a different defaulting behavior, then that command should not
> use the group option.  It could use its own, similar option, with a different
> default option value (a different default-value-providing function).  Or it
> could hard-code its defaulting, or whatever.
>
> The use of a function to dispatch according to the current command should be
> exceptional, IMO - only a fallback possibility and not something to be
> encouraged.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> Suspend your judgement.  I don't like to discuss (or rather I cannot
> discuss) in abstract.  It is so error prone.  I will show my wares
> tomorrow.

Please take your time.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

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

Jambunathan K <kjambunathan@gmail.com> writes:

> "Drew Adams" <drew.adams@oracle.com> writes:
>
>> E.g., in the code I cited, if a user does not want the same defaulting
>> behavior for commands `occur', `how-many', etc., she can set option
>> `search/replace-default-fn' to a function that distinguishes them
>> (e.g., using `this-command', as Jambunathan suggested).
>
> Interesting suggestion there.
>
> This makes me think that there is no need for multiple
> `hi-lock-read-regexp-defaults-function' and a separate
> `occur-read-regexp-defaults-function' etc.  But a single
> `read-regexp-defaults-function' that cases on `this-command'.
>
> The function can return a symbol token like `t' for `this-command's
> which it doesn't want to meddle with but return nil or a regexp or list
> of regexps for commands it wants to insinuate.
>
> Is there any problem with this `read-regexp-defaults-function'
> approach?

EXPERIMENTAL and ABANDONED PATCH

Use of `this-command' is very fragile and flaky.  Consider
`multi-occur-in-matching-buffers' which does multiple `read-regexp' -
one for the buffers and one for the actual regexp.  It is not possible
to return a two different regexps for the same `this-command'.

Interestingly, I am attaching a long from *Messages* buffer and it looks
like `this-command' is not reliable (Do you see `exit-minibuffer' in the
logs.)

So `this-command' could work for simple commands like highlighting
commands but will be flaky to be applied in general.

Anyways good to experiment and see where an idea takes us...

----------------------------------------------------------------
Customization

    (custom-set-variables
     '(read-regexp-user-defaults 
       (quote ((highlight-regexp find-tag-default-as-regexp)
               (highlight-phrase find-tag-default)
               (multi-occur-in-matching-buffers find-tag-default)))))

----------------------------------------------------------------



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: replace.el.patch --]
[-- Type: text/x-diff, Size: 2023 bytes --]

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2013-03-08 04:18:16 +0000
+++ lisp/replace.el	2013-03-09 08:23:57 +0000
@@ -580,6 +580,39 @@ of `history-length', which see.")
 (defvar occur-collect-regexp-history '("\\1")
   "History of regexp for occur's collect operation")
 
+(defcustom read-regexp-user-defaults nil
+  ""
+  :type '(choice
+	  (const :tag "Use system defaults" nil)
+	  (repeat :tag "Per-command defaults"
+		  (list (radio :tag "Command"
+			       (function-item highlight-regexp)
+			       (function-item highlight-phrase)
+			       (function-item highlight-lines-matching-regexp)
+			       (function :tag "Command"))
+			(choice :tag "Function to retrieve the regexp"
+				(const :tag "Use no defaults" nil)
+				(const :tag "Use system defaults" t)
+				(radio
+				 (function-item find-tag-default-as-regexp)
+				 (function-item find-tag-default)
+				 (function-item :tag "Regexp history"
+						(lambda nil
+						  "Use regexp history."
+						  (car regexp-history)))
+				 function)))))
+  :group 'matching
+  :version "24.4")
+
+(defun read-regexp-defaults ()
+  (if (not read-regexp-user-defaults) t
+    (let ((user-default (assoc this-command read-regexp-user-defaults)))
+      (pcase user-default
+	(`(,cmd ,(and (pred functionp) getter))
+	 (funcall getter))
+	(`nil nil)
+	(_ t)))))
+
 (defun read-regexp (prompt &optional defaults history)
   "Read and return a regular expression as a string.
 When PROMPT doesn't end with a colon and space, it adds a final \": \".
@@ -597,6 +630,11 @@ and the last replacement regexp.
 
 Optional arg HISTORY is a symbol to use for the history list.
 If HISTORY is nil, `regexp-history' is used."
+  (let ((user-defaults (read-regexp-defaults)))
+    (unless (eq user-defaults t)
+      (setq defaults user-defaults)
+      (message "cmd: %s defaults: %S" this-command defaults)))
+
   (let* ((default     (if (consp defaults) (car defaults) defaults))
 	 (suggestions (if (listp defaults) defaults (list defaults)))
 	 (suggestions


[-- Attachment #3: Type: text/plain, Size: 431 bytes --]




----------------------------------------------------------------

*Messages*


Global-Hi-Lock mode enabled
Mark saved where search started
cmd: highlight-regexp defaults: "\\_<hi-yellow\\_>"
cmd: highlight-regexp defaults: "\\_<defface\\_>"
cmd: highlight-phrase defaults: "min-colors"
cmd: multi-occur-in-matching-buffers defaults: ":background"
cmd: exit-minibuffer defaults: nil
Searched 1 buffer; 10 matches for `yellow'

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> EXPERIMENTAL and ABANDONED PATCH
> 
> Use of `this-command' is very fragile and flaky.

No, but it is somewhat fragile.

> Consider `multi-occur-in-matching-buffers' which does multiple
> `read-regexp' - one for the buffers and one for the actual
> regexp.  It is not possible to return a two different regexps
> for the same `this-command'.

You don't need to.  If a user needs to test for
`multi-occur-in-matching-buffers' via `this-command' then s?he can do that and
act accordingly.  No need for general code that tries to second-guess things. 

> Interestingly, I am attaching a long from *Messages* buffer 
> and it looks like `this-command' is not reliable (Do you see 
> `exit-minibuffer' in the logs.)

See below.

> So `this-command' could work for simple commands like highlighting
> commands but will be flaky to be applied in general.
> 
> Anyways good to experiment and see where an idea takes us...

As I said, we should not encourage this.  And yes, any use of `last-command' or
`this-command' is somewhat fragile, because some functions intentionally change
their values.

And yes, comparing functions is also problematic in general.  No eta reduction,
for one thing: (equal (lambda (x) (car x)) 'car).

See what I wrote earlier.  Let users choose any string-returning function they
want to use for defaulting.  If a user wants to use a function that conditions
its return value on `this-command', s?he can always do so.

But there is no reason to encourage that.  Any set of commands that we design to
use the same defaulting choice (via the same user option) should be a cohesive
group: the same choice should make sense across that set.

If you have one or more commands that do not fit that, then give them their own
defaulting options (grouping again, where appropriate).

There is nothing new here - just common sense.  The solution is simple, IMO.

> Interestingly, I am attaching a long from *Messages* buffer 
> and it looks like `this-command' is not reliable (Do you see 
> `exit-minibuffer' in the logs.)
>
> cmd: exit-minibuffer defaults: nil

Your code checks only (eq user-defaults t).  When `user-defaults' is nil, this
returns nil.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

Drew

You are beating a dead horse and the first line clearly says it is a
DEAD HORSE.  The mail only stated why I KILLED the horse.

It is my way of saying my earlier patch to hi-lock and the previous
patch to occur stands and is useful.  I will definitely look in to
comments that you may have to offer with the patch that introduces
`occur-read-regexp-defaults-function'.

Grouping - I don't know why you are beating on it - will now be user's
responsibility not that of Emacs.

Jambunathan K.



"Drew Adams" <drew.adams@oracle.com> writes:

>> EXPERIMENTAL and ABANDONED PATCH
>
>> Use of `this-command' is very fragile and flaky.
>
> No, but it is somewhat fragile.
>
>> Consider `multi-occur-in-matching-buffers' which does multiple
>> `read-regexp' - one for the buffers and one for the actual
>> regexp.  It is not possible to return a two different regexps
>> for the same `this-command'.
>
> You don't need to.  If a user needs to test for
> `multi-occur-in-matching-buffers' via `this-command' then s?he can do that and
> act accordingly.  No need for general code that tries to second-guess things. 
>
>> Interestingly, I am attaching a long from *Messages* buffer 
>> and it looks like `this-command' is not reliable (Do you see 
>> `exit-minibuffer' in the logs.)
>
> See below.
>
>> So `this-command' could work for simple commands like highlighting
>> commands but will be flaky to be applied in general.
>> 
>> Anyways good to experiment and see where an idea takes us...
>
> As I said, we should not encourage this.  And yes, any use of `last-command' or
> `this-command' is somewhat fragile, because some functions intentionally change
> their values.
>
> And yes, comparing functions is also problematic in general.  No eta reduction,
> for one thing: (equal (lambda (x) (car x)) 'car).
>
> See what I wrote earlier.  Let users choose any string-returning function they
> want to use for defaulting.  If a user wants to use a function that conditions
> its return value on `this-command', s?he can always do so.
>
> But there is no reason to encourage that.  Any set of commands that we design to
> use the same defaulting choice (via the same user option) should be a cohesive
> group: the same choice should make sense across that set.
>
> If you have one or more commands that do not fit that, then give them their own
> defaulting options (grouping again, where appropriate).
>
> There is nothing new here - just common sense.  The solution is simple, IMO.
>
>> Interestingly, I am attaching a long from *Messages* buffer 
>> and it looks like `this-command' is not reliable (Do you see 
>> `exit-minibuffer' in the logs.)
>>
>> cmd: exit-minibuffer defaults: nil
>
> Your code checks only (eq user-defaults t).  When `user-defaults' is nil, this
> returns nil.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> Drew  You are beating a dead horse and the first line clearly
> says it is a DEAD HORSE.  The mail only stated why I KILLED
> the horse.

I'm not beating anything.

> It is my way of saying my earlier patch to hi-lock and the previous
> patch to occur stands and is useful.  I will definitely look in to
> comments that you may have to offer with the patch that introduces
> `occur-read-regexp-defaults-function'.
> 
> Grouping - I don't know why you are beating on it - will now be
> user's responsibility not that of Emacs.

I'm not beating on anything.

Do you at least see why your *Messages* logged `exit-minibuffer'?

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

"Drew Adams" <drew.adams@oracle.com> writes:

> Do you at least see why your *Messages* logged `exit-minibuffer'?


,---- In `read-regexp'
| +  (let ((user-defaults (read-regexp-defaults)))
| +    (unless (eq user-defaults t)
| +      (setq defaults user-defaults)
| +      (message "cmd: %s defaults: %S" this-command defaults)))
| +
`----

,----
| Global-Hi-Lock mode enabled
| Mark saved where search started
| cmd: highlight-regexp defaults: "\\_<hi-yellow\\_>"
| cmd: highlight-regexp defaults: "\\_<defface\\_>"
| cmd: highlight-phrase defaults: "min-colors"
| cmd: multi-occur-in-matching-buffers defaults: ":background"
| cmd: exit-minibuffer defaults: nil
| Searched 1 buffer; 10 matches for `yellow'
`----


>> Interestingly, I am attaching a long from *Messages* buffer 
>> and it looks like `this-command' is not reliable (Do you see 
>> `exit-minibuffer' in the logs.)
>>
>> cmd: exit-minibuffer defaults: nil
>
> Your code checks only (eq user-defaults t).  When `user-defaults' is nil, this
> returns nil.

The cmd is `exit-minibuffer'.  That corresponds to RET in minibuffer
map.  I have no other explanation.

Btw, your explanation is *totally* off the mark.  It talks about
defaults in my snippet and not the cmd.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> > Do you at least see why your *Messages* logged `exit-minibuffer'?
> 
> ,---- In `read-regexp'
> | +  (let ((user-defaults (read-regexp-defaults)))
> | +    (unless (eq user-defaults t)
> | +      (setq defaults user-defaults)
> | +      (message "cmd: %s defaults: %S" this-command defaults)))
> | +
> `----
> 
> cmd: exit-minibuffer defaults: nil
> 
> > Your code checks only (eq user-defaults t).  When 
> > `user-defaults' is nil, this returns nil.
> 
> The cmd is `exit-minibuffer'.  That corresponds to RET in minibuffer
> map.  I have no other explanation.
> 
> Btw, your explanation is *totally* off the mark.  It talks about
> defaults in my snippet and not the cmd.

NOW I suppose I AM dealing with a dead horse.  But in hopes of helping and at
the risk of being told once more to f___ off, let me try once more:

+(defun read-regexp-defaults ()
+  (if (not read-regexp-user-defaults) t
+    (let ((user-default (assoc this-command read-regexp-user-defaults)))
+      (pcase user-default
+	  (`(,cmd ,(and (pred functionp) getter))
+	   (funcall getter))
+	  (`nil nil)
+	  (_ t)))))

Don't you think that that will return nil when `this-command' =
`exit-minibuffer', since `exit-minibuffer' is not in your value of alist
`read-regexp-user-defaults'?

And if it returns nil, don't you think that the following will then print `cmd:
exit-minibuffer defaults: nil'?

+  (let ((user-defaults (read-regexp-defaults)))
+    (unless (eq user-defaults t)
+      (setq defaults user-defaults)
+      (message "cmd: %s defaults: %S" this-command defaults)))

If this doesn't help, I give up and lie down next to your dead horse.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

"Drew Adams" <drew.adams@oracle.com> writes:

>> > Do you at least see why your *Messages* logged `exit-minibuffer'?
>> 
>> ,---- In `read-regexp'
>> | +  (let ((user-defaults (read-regexp-defaults)))
>> | +    (unless (eq user-defaults t)
>> | +      (setq defaults user-defaults)
>> | +      (message "cmd: %s defaults: %S" this-command defaults)))
>> | +
>> `----
>> 
>> cmd: exit-minibuffer defaults: nil
>> 
>> > Your code checks only (eq user-defaults t).  When 
>> > `user-defaults' is nil, this returns nil.
>> 
>> The cmd is `exit-minibuffer'.  That corresponds to RET in minibuffer
>> map.  I have no other explanation.
>> 
>> Btw, your explanation is *totally* off the mark.  It talks about
>> defaults in my snippet and not the cmd.
>
> NOW I suppose I AM dealing with a dead horse.  But in hopes of helping and at
> the risk of being told once more to f___ off, let me try once more:
>
> +(defun read-regexp-defaults ()
> +  (if (not read-regexp-user-defaults) t
> +    (let ((user-default (assoc this-command read-regexp-user-defaults)))
> +      (pcase user-default
> +	  (`(,cmd ,(and (pred functionp) getter))
> +	   (funcall getter))
> +	  (`nil nil)
> +	  (_ t)))))
>
> Don't you think that that will return nil when `this-command' =
> `exit-minibuffer', since `exit-minibuffer' is not in your value of alist
> `read-regexp-user-defaults'?

Right.

> And if it returns nil, don't you think that the following will then print `cmd:
> exit-minibuffer defaults: nil'?
>
> +  (let ((user-defaults (read-regexp-defaults)))
> +    (unless (eq user-defaults t)
> +      (setq defaults user-defaults)
> +      (message "cmd: %s defaults: %S" this-command defaults)))
>
> If this doesn't help, I give up and lie down next to your dead horse.

Right.

The question is why is this-command `exit-minibuffer' when it should
have been `multi-occur-in-matching-buffers'.  Where does
`exit-minibuffer' come from.

I am not concerned about the defaults, I am concerned about how the cmd.
Do you have an explanation.  There should be an explanation... and the
closest I have come to is to map RET to `exit-minibuffer'.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Drew Adams]

> The question is why is this-command `exit-minibuffer' when it should
> have been `multi-occur-in-matching-buffers'.  Where does
> `exit-minibuffer' come from.
> 
> I am not concerned about the defaults, I am concerned about 
> how the cmd.  Do you have an explanation.  There should be an
> explanation... and the closest I have come to is to map RET to
> `exit-minibuffer'.

That IS the explanation, which you mentioned earlier as well.

`read-regexp' calls `read-from-minibuffer'.  When you hit `RET' during that
read, `exit-minibuffer' is the value of `this-command'.

You did not provide your recipe of testing interactions.  But if you enter
something in the minibuffer using `RET', then that explains why the current
command when `read-regexp-default's is eval'd is `exit-minibuffer'.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Juri Linkov]

> EXPERIMENTAL and ABANDONED PATCH

I think your patch is useful, please don't abandon it
except its `this-command' part.  As you already noted
`this-command' is very fragile and flaky.  Removing everything
related to `this-command' would leave other useful parts of your patch
that adds a new defcustom `read-regexp-defaults' and especially this part:

+               (choice :tag "Function to retrieve the regexp"
+                       (const :tag "Use no defaults" nil)
+                       (radio
+                        (function-item find-tag-default-as-regexp)
+                        (function-item find-tag-default)
+                        (function-item :tag "Regexp history"
+                                       (lambda nil
+                                         "Use regexp history."
+                                         (car regexp-history)))
+                        function)))))

and a new function `read-regexp-defaults'.

Instead of using `this-command', look for ideas to other similar features.
For example, many invocations of minibuffer functions specify their
HISTORY argument as a symbol that divides history variables into groups.
The DEFAULTS argument could use a similar grouping, i.e. when
`read-regexp' uses the symbol `regexp-history' in a call like:

  (read-regexp "Regexp to highlight" (car regexp-history) 'regexp-history)

This could be changed to specify DEFAULTS as the symbol `read-regexp-defaults':

  (read-regexp "Regexp to highlight" 'read-regexp-defaults 'regexp-history)

where `read-regexp-defaults' is a symbol name of the function that uses
the value of the defcustom `read-regexp-defaults' the get the default value
or returns nil.

We could add as many additional default-providing functions
as the number of places that call `read-regexp'.  But I think
it should be enough to have just two functions:

(defun read-regexp-defaults-or-history ()
  (or (read-regexp-defaults)
      (car regexp-history)))

(defun read-regexp-defaults-or-tag ()
  (or (read-regexp-defaults)
      (find-tag-default-as-regexp)))

These two functions are necessary to keep the current status quo
where some commands traditionally provide the last history element
as the default (`highlight-regexp', `occur-read-primary-args', `how-many',
`flush-lines', `keep-lines'), and other commands provide the tag at point
(`rgrep', `query-replace', `multi-occur-in-matching-buffers').
This is an artificial division existing solely for historical reasons.
These functions could be used e.g. in `occur-read-primary-args' as:

  (read-regexp "List lines matching regexp" 'read-regexp-defaults-or-history 'regexp-history)

and e.g. in `grep-read-regexp':

  (read-regexp "Search for" 'read-regexp-defaults-or-tag 'regexp-history)

This implementation will satisfy three goals:

1. Backward compatibility for the current traditional defaults,
2. Allow easy customization in one place (defcustom `read-regexp-defaults'),
3. Allow fine-tuning with function redefinitions, i.e. when
   the users will ask for more specific functions they could be added as:

(defun occur-read-regexp-defaults ()
  (read-regexp-defaults-or-history))

(defun grep-read-regexp-defaults ()
  (read-regexp-defaults-or-tag))

and can be used in `occur-read-primary-args':

  (read-regexp "List lines matching regexp" 'occur-read-regexp-defaults 'regexp-history)

and in `grep-read-regexp':

  (read-regexp "Search for" 'grep-read-regexp-defaults 'regexp-history)

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el(read-regexp): Let-bind `default' to the first

[Jambunathan K]

Juri

(For the sake of record)

Feel free to take over this bug.

In light of recent developments on the mailing list, I am a bit hesitant
to take these discussions any further.

Jambunathan K.


Juri Linkov <juri@jurta.org> writes:

>> EXPERIMENTAL and ABANDONED PATCH
>
> I think your patch is useful, please don't abandon it
> except its `this-command' part.  As you already noted
> `this-command' is very fragile and flaky.  Removing everything
> related to `this-command' would leave other useful parts of your patch
> that adds a new defcustom `read-regexp-defaults' and especially this part:
>
> +               (choice :tag "Function to retrieve the regexp"
> +                       (const :tag "Use no defaults" nil)
> +                       (radio
> +                        (function-item find-tag-default-as-regexp)
> +                        (function-item find-tag-default)
> +                        (function-item :tag "Regexp history"
> +                                       (lambda nil
> +                                         "Use regexp history."
> +                                         (car regexp-history)))
> +                        function)))))
>
> and a new function `read-regexp-defaults'.
>
> Instead of using `this-command', look for ideas to other similar features.
> For example, many invocations of minibuffer functions specify their
> HISTORY argument as a symbol that divides history variables into groups.
> The DEFAULTS argument could use a similar grouping, i.e. when
> `read-regexp' uses the symbol `regexp-history' in a call like:
>
>   (read-regexp "Regexp to highlight" (car regexp-history) 'regexp-history)
>
> This could be changed to specify DEFAULTS as the symbol `read-regexp-defaults':
>
>   (read-regexp "Regexp to highlight" 'read-regexp-defaults 'regexp-history)
>
> where `read-regexp-defaults' is a symbol name of the function that uses
> the value of the defcustom `read-regexp-defaults' the get the default value
> or returns nil.
>
> We could add as many additional default-providing functions
> as the number of places that call `read-regexp'.  But I think
> it should be enough to have just two functions:
>
> (defun read-regexp-defaults-or-history ()
>   (or (read-regexp-defaults)
>       (car regexp-history)))
>
> (defun read-regexp-defaults-or-tag ()
>   (or (read-regexp-defaults)
>       (find-tag-default-as-regexp)))
>
> These two functions are necessary to keep the current status quo
> where some commands traditionally provide the last history element
> as the default (`highlight-regexp', `occur-read-primary-args', `how-many',
> `flush-lines', `keep-lines'), and other commands provide the tag at point
> (`rgrep', `query-replace', `multi-occur-in-matching-buffers').
> This is an artificial division existing solely for historical reasons.
> These functions could be used e.g. in `occur-read-primary-args' as:
>
>   (read-regexp "List lines matching regexp" 'read-regexp-defaults-or-history 'regexp-history)
>
> and e.g. in `grep-read-regexp':
>
>   (read-regexp "Search for" 'read-regexp-defaults-or-tag 'regexp-history)
>
> This implementation will satisfy three goals:
>
> 1. Backward compatibility for the current traditional defaults,
> 2. Allow easy customization in one place (defcustom `read-regexp-defaults'),
> 3. Allow fine-tuning with function redefinitions, i.e. when
>    the users will ask for more specific functions they could be added as:
>
> (defun occur-read-regexp-defaults ()
>   (read-regexp-defaults-or-history))
>
> (defun grep-read-regexp-defaults ()
>   (read-regexp-defaults-or-tag))
>
> and can be used in `occur-read-primary-args':
>
>   (read-regexp "List lines matching regexp" 'occur-read-regexp-defaults 'regexp-history)
>
> and in `grep-read-regexp':
>
>   (read-regexp "Search for" 'grep-read-regexp-defaults 'regexp-history)

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el (read-regexp): Let-bind `default' to the first

[Jambunathan K]

Juri Linkov <juri@jurta.org> writes:

>> emacs -Q
>> M-s h r
>>
>> I find the following error
>>
>> ,----
>> | Debugger entered--Lisp error: (error "Regexp cannot match an empty string")
>> |   signal(error ("Regexp cannot match an empty string"))
>> |   error("Regexp cannot match an empty string")
>> |   hi-lock-regexp-okay("")
>> |   byte-code("..." [regexp-history hi-lock-regexp-okay read-regexp
>> | "Regexp to highlight" hi-lock-read-face-name] 4)
>> |   call-interactively(highlight-regexp nil nil)
>> |   command-execute(highlight-regexp)
>> `----
>>
>> So, if one does
>>         (read-regexp something) ;; something is nil or evals to nil
>>
>> what should the interpretation be.
>>
>> With your change, a `nil' default will provide an empty string as input
>> and force user to enter a regexp or rely on M-n.
>>
>> We seem to be bumping in to each other in this area.  Comments ...?
>>
>> ,---- Stefan @ http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13687#11
>> | I disagree: read-regexp is a generic function which can be used in
>> | various contexts, some of which might not care at all about the text
>> | around point.  So the caller should have control over the first default
>> | (of course, it's perfectly fine to always add the current tag in the
>> | subsequent defaults).
>> |
>> | This said your patch seems to leave the caller's provided `defaults' at
>> | the beginning of the minibuffer's `defaults', so I think your patch is
>> | fine, feel free to install it.
>> `----
>>
>> I am wondering how we can resolve the contex-free read-regexp and
>> context-dependent regexp.  Any suggestions?
>
> It's a responsibility of the caller to provide the default value.
> `M-s h r' (`highlight-regexp') provides the default value as
> `(car regexp-history)'.  When it is called the first time after
> `emacs -Q', the history is empty, so its default value is nil
> (this fact is indicated with missing default value in the prompt,
> so the user is aware that RET with empty input will do nothing.)
>
> When `highlight-regexp' is called the second time and more,
> it gets the default value from `regexp-history', so you can't
> provide the tag at point as the default for later invocations
> of `highlight-regexp' anyway.

The question was one of how `read-regexp' should behave?

If (car DEFAULTS) is nil should it offer (car SUGGESTIONS) as a default
or offer an empty string for default.

>
> So the question is: should the default value in the caller
> `highlight-regexp' be changed from `(car regexp-history)'
> to code that gets the tag at point?  You could propose
> such a change, but since it changes the long-standing behavior,
> expect some disagreement (not from me :-)

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el (read-regexp): Let-bind `default' to the first

[Juri Linkov]

> The question was one of how `read-regexp' should behave?
>
> If (car DEFAULTS) is nil should it offer (car SUGGESTIONS) as a default
> or offer an empty string for default.

As bug#13805 indicates, `read-regexp' should return an empty string
when the caller specifies that (car DEFAULTS) is nil.

This is like what `read-from-minibuffer' does.  But note that
`read-from-minibuffer' is even more strict, and doesn't return
the default value even if it's specified, i.e. typing RET
after evaluating

(read-from-minibuffer "Prompt: " nil nil nil nil "default")

returns an empty string instead of the default value.
This is because its arg DEFAULT-VALUE acts more like
a suggestion available for M-n whose values should not be
returned for empty input.

bug#13687: /srv/bzr/emacs/trunk r111878: * lisp/replace.el (read-regexp): Let-bind `default' to the first

[Jambunathan K]

Juri Linkov <juri@jurta.org> writes:

>> The question was one of how `read-regexp' should behave?
>>
>> If (car DEFAULTS) is nil should it offer (car SUGGESTIONS) as a default
>> or offer an empty string for default.
>
> As bug#13805 indicates, `read-regexp' should return an empty string
> when the caller specifies that (car DEFAULTS) is nil.

I think I overlooked the bug number in your commit.  Otherwise, I
wouldn't have triggered this discussion in first place.  Sorry.

>
> This is like what `read-from-minibuffer' does.  But note that
> `read-from-minibuffer' is even more strict, and doesn't return
> the default value even if it's specified, i.e. typing RET
> after evaluating
>
> (read-from-minibuffer "Prompt: " nil nil nil nil "default")
>
> returns an empty string instead of the default value.
> This is because its arg DEFAULT-VALUE acts more like
> a suggestion available for M-n whose values should not be
> returned for empty input.