GNU bug report logs - #47599
28.0.50; Feature request improve/update isearch

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 47599 in the body.
You can then email your comments to 47599 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: Ergus <spacibba <at> aol.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Feature request improve/update isearch
Date: Mon, 5 Apr 2021 04:07:25 +0200

Hi:

Just to follow this:
https://lists.gnu.org/archive/html/emacs-devel/2021-04/msg00080.html

I open a feature request to suggest some updates in the isearch code and
the addition of some simple functionalities like:

1) Option or command to automatically go to the other end on exit.

2) Convert isearch-wrap-function, isearch-push-state-function,
isearch-filter-predicate into customs instead of use defvar, improve a
bit their documentation and provide some handy options.

3) Make some general refactor of isearch code to simplify and remove
some redundant or useless checks, vars and code. Update the code to use
some useful modern api like define-minor-mode or easy-mmode-defmap. 

There are some external packages that reimplement the isearch
functionalities or hack it to produce some of these
functionalities/behavior and maybe (as an extra) we could consider add
some customs to make isearch behave like a bit like that; because most
of them are not very different to what isearch already does now.

Example: https://github.com/raxod502/ctrlf 

Some of the differences where mentioned already here:

https://lists.gnu.org/archive/html/emacs-devel/2021-04/msg00108.html

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

From: Juri Linkov <juri <at> linkov.net>
To: Ergus <spacibba <at> aol.com>
Cc: 47599-done <at> debbugs.gnu.org
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Tue, 06 Apr 2021 22:16:24 +0300

> Just to follow this:
> https://lists.gnu.org/archive/html/emacs-devel/2021-04/msg00080.html
>
> I open a feature request to suggest some updates in the isearch code and
> the addition of some simple functionalities like:
>
> 1) Option or command to automatically go to the other end on exit.

It seems the conclusion was that it should not be an option.
As for a command, there were objections against binding it to a key,
but without a keybinding such command has little sense.

> 2) Convert isearch-wrap-function,

Thanks for the suggestion.  Customization of wrapping was implemented now
by the new option 'isearch-wrap-pause'.  It also handles numeric arguments
of 'isearch-repeat'.

> isearch-push-state-function,
> isearch-filter-predicate into customs instead of use defvar, improve a
> bit their documentation and provide some handy options.

These functions are intended to be modified by modes, not by users.

> 3) Make some general refactor of isearch code to simplify and remove
> some redundant or useless checks, vars and code. Update the code to use
> some useful modern api like define-minor-mode or easy-mmode-defmap. There
> are some external packages that reimplement the isearch
> functionalities or hack it to produce some of these
> functionalities/behavior and maybe (as an extra) we could consider add
> some customs to make isearch behave like a bit like that; because most
> of them are not very different to what isearch already does now.

Rewriting isearch is a huge and unnecessary task.

More patches were posted directly to emacs-devel now,
so more changes are expected from that discussion.

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Ergus <spacibba <at> aol.com>, 47599-done <at> debbugs.gnu.org
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Tue, 06 Apr 2021 20:38:49 +0000

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

>> 1) Option or command to automatically go to the other end on exit.
>
> It seems the conclusion was that it should not be an option. As for a 
> command, there were objections against binding it to a key, but without 
> a keybinding such command has little sense.
>

I attach a patch which implements these two "options" in isearch-exit.

[0001-Add-options-when-exiting-isearch.patch (text/x-diff, attachment)]

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Ergus <spacibba <at> aol.com>, 47599-done <at> debbugs.gnu.org
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Tue, 06 Apr 2021 21:01:48 +0000

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

>>> 1) Option or command to automatically go to the other end on exit.
>> 
>> It seems the conclusion was that it should not be an option. As for a 
>> command, there were objections against binding it to a key, but without 
>> a keybinding such command has little sense.
>
> I attach a patch which implements these two "options" in isearch-exit.
>

And of course my patch had an error ;-)

[0001-Add-options-when-exiting-isearch.patch (text/x-diff, attachment)]

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Ergus <spacibba <at> aol.com>, 47599-done <at> debbugs.gnu.org
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Tue, 06 Apr 2021 21:32:51 +0000

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

>>>> 1) Option or command to automatically go to the other end on exit.
>>> 
>>> It seems the conclusion was that it should not be an option. As for a 
>>> command, there were objections against binding it to a key, but 
>>> without a keybinding such command has little sense.
>> 
>> I attach a patch which implements these two "options" in isearch-exit.
>
> And of course my patch had an error ;-)
>

While we're improving isearch, there is another minor change that I think 
would improve its behavior, namely to go to the next/previous match when 
changing the search direction, instead of hitting the same match before 
moving to the next/previous one with the next C-s/C-r.  This change is so 
small that I'm not sure it's worth creating a defcustom for it, but you 
might have a different opinion.

[0001-Find-another-match-when-changing-direction-in-isearc.patch (text/x-diff, attachment)]

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Gregory Heytings <gregory <at> heytings.org>, Juri Linkov <juri <at> linkov.net>
Cc: Ergus <spacibba <at> aol.com>,
 "47599-done <at> debbugs.gnu.org" <47599-done <at> debbugs.gnu.org>
Subject: RE: [External] : bug#47599: 28.0.50; Feature request improve/update
 isearch
Date: Tue, 6 Apr 2021 22:39:29 +0000

> While we're improving isearch, there is another minor change that I think
> would improve its behavior, namely to go to the next/previous match when
> changing the search direction, instead of hitting the same match before
> moving to the next/previous one with the next C-s/C-r.  This change is so
> small that I'm not sure it's worth creating a defcustom for it, but you
> might have a different opinion.

1. Please file a separate bug report when you move the goal post.

2. Please do NOT do this.  There are reasons we want to
   stay at the same search hit.  One of them has already
   surfaced in these (lamentable) discussions of isearch
   "improvements": namely, use `C-r RET' to move to the
   beginning of the search hit (when searching forward).

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Ergus <spacibba <at> aol.com>, 47599-done <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: RE: [External] : bug#47599: 28.0.50; Feature request improve/update
 isearch
Date: Tue, 06 Apr 2021 22:43:47 +0000

>> While we're improving isearch, there is another minor change that I 
>> think would improve its behavior, namely to go to the next/previous 
>> match when changing the search direction, instead of hitting the same 
>> match before moving to the next/previous one with the next C-s/C-r. 
>> This change is so small that I'm not sure it's worth creating a 
>> defcustom for it, but you might have a different opinion.
>
> Please do NOT do this.  There are reasons we want to stay at the same 
> search hit.
>

Okay, so I have the answer to my own question: this should become yet 
another user option.

>
> One of them has already surfaced in these (lamentable) discussions of 
> isearch "improvements":
>

Why "lamentable"???

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Ergus <spacibba <at> aol.com>, 47599-done <at> debbugs.gnu.org,
 Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#47599: [External] : bug#47599: 28.0.50; Feature request
 improve/update isearch
Date: Tue, 06 Apr 2021 23:26:30 +0000

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

>>> While we're improving isearch, there is another minor change that I 
>>> think would improve its behavior, namely to go to the next/previous 
>>> match when changing the search direction, instead of hitting the same 
>>> match before moving to the next/previous one with the next C-s/C-r. 
>>> This change is so small that I'm not sure it's worth creating a 
>>> defcustom for it, but you might have a different opinion.
>> 
>> Please do NOT do this.  There are reasons we want to stay at the same 
>> search hit.
>
> Okay, so I have the answer to my own question: this should become yet 
> another user option.
>

And here is the corresponding patch.

[0001-User-option-to-move-to-another-match-when-changing-d.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 47599 <at> debbugs.gnu.org, spacibba <at> aol.com, juri <at> linkov.net
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Wed, 07 Apr 2021 05:29:25 +0300

> Date: Tue, 06 Apr 2021 21:32:51 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> Cc: Ergus <spacibba <at> aol.com>, 47599-done <at> debbugs.gnu.org
> 
> While we're improving isearch, there is another minor change that I think 
> would improve its behavior, namely to go to the next/previous match when 
> changing the search direction, instead of hitting the same match before 
> moving to the next/previous one with the next C-s/C-r.  This change is so 
> small that I'm not sure it's worth creating a defcustom for it, but you 
> might have a different opinion.

The current behavior is the easy way to "go to the other end of the
match", so I'm quite sure some people will want to keep the old
behavior if we make this change.

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Ergus <spacibba <at> aol.com>, 47599-done <at> debbugs.gnu.org
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Wed, 07 Apr 2021 10:44:23 +0000

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

>>> 1) Option or command to automatically go to the other end on exit.
>> 
>> It seems the conclusion was that it should not be an option. As for a 
>> command, there were objections against binding it to a key, but without 
>> a keybinding such command has little sense.
>
> I attach a patch which implements these two "options" in isearch-exit.
>

And here is an updated patch which adds three options to isearch-exit: C-u 
RET moves point to the other end, C-u C-u RET activates region around 
match, C-u C-u C-u RET moves point to the other end and activate region 
around match.

[0001-Add-options-when-exiting-isearch.patch (text/x-diff, attachment)]

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

From: Ergus <spacibba <at> aol.com>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 47599-done <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Wed, 7 Apr 2021 13:20:50 +0200

On Wed, Apr 07, 2021 at 10:44:23AM +0000, Gregory Heytings wrote:
>
>>>>1) Option or command to automatically go to the other end on exit.
>>>
>>>It seems the conclusion was that it should not be an option. As 
>>>for a command, there were objections against binding it to a key, 
>>>but without a keybinding such command has little sense.
>>
>>I attach a patch which implements these two "options" in isearch-exit.
>>
>
>And here is an updated patch which adds three options to isearch-exit: 
>C-u RET moves point to the other end, C-u C-u RET activates region 
>around match, C-u C-u C-u RET moves point to the other end and 
>activate region around match.

Hi Gregory:

Maybe I am wrong but I think it makes no sense to add C-u RET for this;
because it is not better than C-r RET. Same applies to the other two
commands.

I am only interested in the first one really; and for that case I would
prefer something shorter like M-RET and for the others maybe C-M-@ (just
to mention random examples that are shorter, free or "similar" somehow
to what happens outside isearch). We can consider things like M-s RET;
M-@ and so on too... But IMO the C-u C-u C-u is not the way we should
go..

Does it makes sense?

Ergus?

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Ergus <spacibba <at> aol.com>
Cc: 47599-done <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Wed, 07 Apr 2021 11:33:53 +0000

>> And here is an updated patch which adds three options to isearch-exit: 
>> C-u RET moves point to the other end, C-u C-u RET activates region 
>> around match, C-u C-u C-u RET moves point to the other end and activate 
>> region around match.
>
> Hi Gregory:
>
> Maybe I am wrong but I think it makes no sense to add C-u RET for this; 
> because it is not better than C-r RET. Same applies to the other two 
> commands.
>

I have no strong opinion, I proposed this patch because of your request. 
But C-u RET is better I think, it's a single command instead of two (C-r 
RET and C-s RET).  For the two other commands, I don't know an easy way to 
mark the match.

>
> I am only interested in the first one really;
>

Yes, I was proposing something more general, with what I understood from 
the discussion.

>
> But IMO the C-u C-u C-u is not the way we should go..
>

I think it's easy to type, much easier than M-s RET for example, but as I 
sait I have no strong opinion on this, it's fine if my patch isn't 
applied.

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

From: Juri Linkov <juri <at> linkov.net>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 47599 <at> debbugs.gnu.org, Ergus <spacibba <at> aol.com>,
 Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#47599: [External] : bug#47599: 28.0.50; Feature request
 improve/update isearch
Date: Wed, 07 Apr 2021 19:20:13 +0300

>>>> While we're improving isearch, there is another minor change that
>>>> I think would improve its behavior, namely to go to the next/previous
>>>> match when changing the search direction, instead of hitting the same
>>>> match before moving to the next/previous one with the next
>>>> C-s/C-r. This change is so small that I'm not sure it's worth creating
>>>> a defcustom for it, but you might have a different opinion.
>>> Please do NOT do this.  There are reasons we want to stay at the same
>>> search hit.
>>
>> Okay, so I have the answer to my own question: this should become yet
>> another user option.
>
> And here is the corresponding patch.

Thanks, finally there is an option to avoid typing extra C-r.

> +(defcustom isearch-direction-change-changes-match nil
> +  "Whether a direction change should move to another match.
> +When `nil', the default, a direction change moves point to the other
> +end of the current search match.
> +When `t', a direction change moves to another search match, if there
> +is one."
> +  :type '(choice (const :tag "Remain on the same match" nil)
> +                 (const :tag "Move to another match" t))

Is it possible to find a clearer name?
Maybe isearch-repeat-on-direction-change would be better
with the prefix 'isearch-repeat-' to hint that it applies
to the commands 'isearch-repeat-*'?

>      ;; C-s in reverse or C-r in forward, change direction.
> +    (if (and isearch-other-end isearch-direction-change-changes-match)
> +        (goto-char isearch-other-end))

This breaks the following feature:

When isearch-forward is t:
- C-1 C-r moves to the previous match (like your patch does without 'C-1')
- C-2 C-r moves to the second previous match
- C-u -1 C-r moves to the next match
- C-u -2 C-r moves to the second next match

This is due to these lines in isearch-repeat-backward:

               ;; Reverse the direction back
               (isearch-repeat 'backward))
              (t
               ;; Take into account one iteration to reverse direction
               (when isearch-forward (setq count (1+ count)))

When the new option is non-nil, there is no need to increment 'count'.
Also the new option should be let-bound to nil around the call to
'(isearch-repeat 'backward)' above to just change the direction back
without moving to the next match.

The same applies to isearch-repeat-forward and when isearch-forward is nil.

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47599 <at> debbugs.gnu.org, Ergus <spacibba <at> aol.com>
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Wed, 07 Apr 2021 17:58:13 +0000

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

>
> Thanks, finally there is an option to avoid typing extra C-r.
>

:-)  And thanks for your feedback!

>
> Is it possible to find a clearer name?
> Maybe isearch-repeat-on-direction-change would be better with the prefix 
> 'isearch-repeat-' to hint that it applies to the commands 
> 'isearch-repeat-*'?
>

Done.

>
> This breaks the following feature:
>
> When isearch-forward is t:
> - C-1 C-r moves to the previous match (like your patch does without 'C-1')
> - C-2 C-r moves to the second previous match
> - C-u -1 C-r moves to the next match
> - C-u -2 C-r moves to the second next match
>
> This is due to these lines in isearch-repeat-backward:
>
>               ;; Reverse the direction back
>               (isearch-repeat 'backward))
>              (t
>               ;; Take into account one iteration to reverse direction
>               (when isearch-forward (setq count (1+ count)))
>
> When the new option is non-nil, there is no need to increment 'count'.
> Also the new option should be let-bound to nil around the call to
> '(isearch-repeat 'backward)' above to just change the direction back
> without moving to the next match.
>
> The same applies to isearch-repeat-forward and when isearch-forward is nil.
>

Fixed, thank you!

[0001-User-option-to-move-to-another-match-when-changing-d.patch (text/x-diff, attachment)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 47599 <at> debbugs.gnu.org, Ergus <spacibba <at> aol.com>
Subject: Re: bug#47599: 28.0.50; Feature request improve/update isearch
Date: Thu, 08 Apr 2021 22:05:34 +0300

>> Is it possible to find a clearer name?
>> Maybe isearch-repeat-on-direction-change would be better with the prefix
>> 'isearch-repeat-' to hint that it applies to the commands
>> 'isearch-repeat-*'?
>
> Done.

Now your patch is pushed in 972bab0981.  Thanks for such useful feature!

Previous Next

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