unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#47599: 28.0.50; Feature request improve/update isearch
       [not found] <20210405020725.ob7bewlin7cid4pa.ref@Ergus>
@ 2021-04-05  2:07 ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-04-06 19:16   ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-04-05  2:07 UTC (permalink / raw)
  To: 47599

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






^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-05  2:07 ` bug#47599: 28.0.50; Feature request improve/update isearch Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-04-06 19:16   ` Juri Linkov
  2021-04-06 20:38     ` Gregory Heytings
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2021-04-06 19:16 UTC (permalink / raw)
  To: Ergus; +Cc: 47599-done

> 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.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 19:16   ` Juri Linkov
@ 2021-04-06 20:38     ` Gregory Heytings
  2021-04-06 21:01       ` Gregory Heytings
  2021-04-07 10:44       ` Gregory Heytings
  0 siblings, 2 replies; 15+ messages in thread
From: Gregory Heytings @ 2021-04-06 20:38 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Ergus, 47599-done

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


>> 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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-Add-options-when-exiting-isearch.patch, Size: 1537 bytes --]

From e7df31567e9175392a677c225825a4f2784376eb Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 6 Apr 2021 20:33:09 +0000
Subject: [PATCH] Add options when exiting isearch

* lisp/isearch.el (isearch-exit): Add two options when exiting isearch.
---
 lisp/isearch.el | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 943e24aa56..a2ac4964d2 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1523,12 +1523,20 @@ REGEXP if non-nil says use the regexp search ring."
   "Exit search normally.
 However, if this is the first command after starting incremental
 search and `search-nonincremental-instead' is non-nil, do a
-nonincremental search instead via `isearch-edit-string'."
+nonincremental search instead via `isearch-edit-string'.
+With a single prefix argument, move point to the beginning of the
+search string, unless the region is active.
+With two prefix arguments, activate the region around the match,
+unless the region is already active."
   (interactive)
   (if (and search-nonincremental-instead
 	   (= 0 (length isearch-string)))
       (let ((isearch-nonincremental t))
 	(isearch-edit-string)) ;; this calls isearch-done as well
+    (if (and (> arg 1) isearch-other-end (not (use-region-p)))
+        (if (> arg 4)
+            (push-mark isearch-other-end t 'activate)
+          (goto-char isearch-other-end)))
     (isearch-done))
   (isearch-clean-overlays))
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 20:38     ` Gregory Heytings
@ 2021-04-06 21:01       ` Gregory Heytings
  2021-04-06 21:32         ` Gregory Heytings
  2021-04-07 10:44       ` Gregory Heytings
  1 sibling, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-04-06 21:01 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Ergus, 47599-done

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


>>> 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 ;-)

[-- Attachment #2: Type: text/x-diff, Size: 1563 bytes --]

From 754f8953d24734a8a71bc6705f156f9981ed067d Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 6 Apr 2021 20:59:46 +0000
Subject: [PATCH] Add options when exiting isearch

* lisp/isearch.el (isearch-exit): Add two options when exiting isearch.
---
 lisp/isearch.el | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 943e24aa56..0f66110493 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1523,12 +1523,20 @@ REGEXP if non-nil says use the regexp search ring."
   "Exit search normally.
 However, if this is the first command after starting incremental
 search and `search-nonincremental-instead' is non-nil, do a
-nonincremental search instead via `isearch-edit-string'."
-  (interactive)
+nonincremental search instead via `isearch-edit-string'.
+With a single prefix argument, move point to the beginning of the
+search string, unless the region is active.
+With two prefix arguments, activate the region around the match,
+unless the region is already active."
+  (interactive "p")
   (if (and search-nonincremental-instead
 	   (= 0 (length isearch-string)))
       (let ((isearch-nonincremental t))
 	(isearch-edit-string)) ;; this calls isearch-done as well
+    (if (and (> arg 1) isearch-other-end (not (use-region-p)))
+        (if (> arg 4)
+            (push-mark isearch-other-end t 'activate)
+          (goto-char isearch-other-end)))
     (isearch-done))
   (isearch-clean-overlays))
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 21:01       ` Gregory Heytings
@ 2021-04-06 21:32         ` Gregory Heytings
  2021-04-06 22:39           ` bug#47599: [External] : " Drew Adams
  2021-04-07  2:29           ` Eli Zaretskii
  0 siblings, 2 replies; 15+ messages in thread
From: Gregory Heytings @ 2021-04-06 21:32 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Ergus, 47599-done

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


>>>> 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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-Find-another-match-when-changing-direction-in-isearc.patch, Size: 932 bytes --]

From 63e74e7a06df1ebf05061328de7edb1b5f9f8200 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 6 Apr 2021 21:24:47 +0000
Subject: [PATCH] Find another match when changing direction in isearch.

* lisp/isearch.el (isearch-repeat): Move point to the other end when
changing direction, to find another match.
---
 lisp/isearch.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index a828c569aa..7241b6b319 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1847,6 +1847,7 @@ Use `isearch-exit' to quit without signaling."
 	      (funcall isearch-wrap-function)
 	    (goto-char (if isearch-forward (point-min) (point-max))))))
     ;; C-s in reverse or C-r in forward, change direction.
+    (if isearch-other-end (goto-char isearch-other-end))
     (setq isearch-forward (not isearch-forward)
 	  isearch-success t))
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#47599: [External] : bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 21:32         ` Gregory Heytings
@ 2021-04-06 22:39           ` Drew Adams
  2021-04-06 22:43             ` Gregory Heytings
  2021-04-07  2:29           ` Eli Zaretskii
  1 sibling, 1 reply; 15+ messages in thread
From: Drew Adams @ 2021-04-06 22:39 UTC (permalink / raw)
  To: Gregory Heytings, Juri Linkov; +Cc: Ergus, 47599-done@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.

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).






^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#47599: [External] : bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 22:39           ` bug#47599: [External] : " Drew Adams
@ 2021-04-06 22:43             ` Gregory Heytings
  2021-04-06 23:26               ` Gregory Heytings
  0 siblings, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-04-06 22:43 UTC (permalink / raw)
  To: Drew Adams; +Cc: Ergus, 47599-done, Juri Linkov


>> 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"???





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#47599: [External] : bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 22:43             ` Gregory Heytings
@ 2021-04-06 23:26               ` Gregory Heytings
  2021-04-07 16:20                 ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-04-06 23:26 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Ergus, 47599-done

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


>>> 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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-User-option-to-move-to-another-match-when-changing-d.patch, Size: 3328 bytes --]

From 8276a0808a978eb124d1a71615a4995e3bc27bd2 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 6 Apr 2021 23:19:19 +0000
Subject: [PATCH] User option to move to another match when changing direction
 in isearch.

* lisp/isearch.el (isearch-direction-change-changes-match): New
user option.
(isearch-repeat): Use the new user option.

* etc/NEWS: Mention the new user option.

* doc/emacs/search.texi: Document the new user option.
---
 doc/emacs/search.texi |  8 ++++++++
 etc/NEWS              |  6 ++++++
 lisp/isearch.el       | 12 ++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/doc/emacs/search.texi b/doc/emacs/search.texi
index f3c42bcea7..222e5b6c96 100644
--- a/doc/emacs/search.texi
+++ b/doc/emacs/search.texi
@@ -201,6 +201,14 @@ something before the starting point, type @kbd{C-r} to switch to a
 backward search, leaving the search string unchanged.  Similarly,
 @kbd{C-s} in a backward search switches to a forward search.
 
+@cindex search, changing direction
+@vindex isearch-direction-change-changes-match
+  When you change the direction of a search, the first command you
+type will, by default, remain on the same match, and the cursor will
+move to the other end of the match.  To move to another match
+immediately, customize the variable
+@code{isearch-direction-change-changes-match} to @code{t}.
+
 @cindex search, wrapping around
 @cindex search, overwrapped
 @cindex wrapped search
diff --git a/etc/NEWS b/etc/NEWS
index d3a8748ded..73ceb96f1f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -367,6 +367,12 @@ trying to be non-destructive.
 This command opens a new buffer called "*Memory Report*" and gives a
 summary of where Emacs is using memory currently.
 
++++
+** New user option 'isearch-direction-change-changes-match'.
+When this option is set, direction changes in Isearch move to another
+search match, if there is one, instead of moving point to the other
+end of the current match.
+
 ** Outline
 
 +++
diff --git a/lisp/isearch.el b/lisp/isearch.el
index a828c569aa..bec202516b 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -185,6 +185,16 @@ When `nil', never wrap, just stop at the last match."
                  (const :tag "Disable wrapping" nil))
   :version "28.1")
 
+(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))
+  :version "28.1")
+
 (defvar isearch-mode-hook nil
   "Function(s) to call after starting up an incremental search.")
 
@@ -1847,6 +1857,8 @@ Use `isearch-exit' to quit without signaling."
 	      (funcall isearch-wrap-function)
 	    (goto-char (if isearch-forward (point-min) (point-max))))))
     ;; 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))
     (setq isearch-forward (not isearch-forward)
 	  isearch-success t))
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 21:32         ` Gregory Heytings
  2021-04-06 22:39           ` bug#47599: [External] : " Drew Adams
@ 2021-04-07  2:29           ` Eli Zaretskii
  1 sibling, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2021-04-07  2:29 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: 47599, spacibba, juri

> Date: Tue, 06 Apr 2021 21:32:51 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: Ergus <spacibba@aol.com>, 47599-done@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.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 20:38     ` Gregory Heytings
  2021-04-06 21:01       ` Gregory Heytings
@ 2021-04-07 10:44       ` Gregory Heytings
  2021-04-07 11:20         ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-04-07 10:44 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Ergus, 47599-done

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


>>> 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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-Add-options-when-exiting-isearch.patch, Size: 2001 bytes --]

From 8922c42ad93c5c1bfeb0ed29c598941e9141227c Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 7 Apr 2021 10:39:19 +0000
Subject: [PATCH] Add options when exiting isearch

* lisp/isearch.el (isearch-exit): Add three options when exiting
isearch, that can be used with prefix arguments.
---
 lisp/isearch.el | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index a828c569aa..e4679f78a8 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1532,16 +1532,29 @@ REGEXP if non-nil says use the regexp search ring."
 \f
 ;; Commands active while inside of the isearch minor mode.
 
-(defun isearch-exit ()
+(defun isearch-exit (&optional arg)
   "Exit search normally.
 However, if this is the first command after starting incremental
 search and `search-nonincremental-instead' is non-nil, do a
-nonincremental search instead via `isearch-edit-string'."
-  (interactive)
+nonincremental search instead via `isearch-edit-string'.
+With a single prefix argument, move point to the other end of the
+search match, unless the region is active.
+With two prefix arguments, activate the region around the search
+match, unless the region is already active.
+With three prefix arguments, activate the region around the search
+match and move point to the other end of the search match, unless
+the region is already active."
+  (interactive "p")
   (if (and search-nonincremental-instead
 	   (= 0 (length isearch-string)))
       (let ((isearch-nonincremental t))
 	(isearch-edit-string)) ;; this calls isearch-done as well
+    (if (and (> arg 1) isearch-other-end (not (use-region-p)))
+        (if (> arg 4)
+            (progn
+              (push-mark isearch-other-end t 'activate)
+              (if (> arg 16) (exchange-point-and-mark)))
+          (goto-char isearch-other-end)))
     (isearch-done))
   (isearch-clean-overlays))
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-07 10:44       ` Gregory Heytings
@ 2021-04-07 11:20         ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-04-07 11:33           ` Gregory Heytings
  0 siblings, 1 reply; 15+ messages in thread
From: Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-04-07 11:20 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: 47599-done, Juri Linkov

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?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-07 11:20         ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-04-07 11:33           ` Gregory Heytings
  0 siblings, 0 replies; 15+ messages in thread
From: Gregory Heytings @ 2021-04-07 11:33 UTC (permalink / raw)
  To: Ergus; +Cc: 47599-done, Juri Linkov


>> 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.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#47599: [External] : bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-06 23:26               ` Gregory Heytings
@ 2021-04-07 16:20                 ` Juri Linkov
  2021-04-07 17:58                   ` Gregory Heytings
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2021-04-07 16:20 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: 47599, Ergus

>>>> 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.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-07 16:20                 ` Juri Linkov
@ 2021-04-07 17:58                   ` Gregory Heytings
  2021-04-08 19:05                     ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-04-07 17:58 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 47599, Ergus

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


>
> 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!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-User-option-to-move-to-another-match-when-changing-d.patch, Size: 4942 bytes --]

From a90d2447b448831f3f4ab8200769ef749612c02a Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 7 Apr 2021 17:51:30 +0000
Subject: [PATCH] User option to move to another match when changing direction
 in isearch.

* lisp/isearch.el (isearch-direction-change-changes-match): New
user option.
(isearch-repeat): Use the new option.
(isearch-repeat-forward, isearch-repeat-backward): Adapt to the
new option.

* etc/NEWS: Mention the new user option.

* doc/emacs/search.texi: Document the new user option.
---
 doc/emacs/search.texi |  8 ++++++++
 etc/NEWS              |  6 ++++++
 lisp/isearch.el       | 24 ++++++++++++++++++++----
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/doc/emacs/search.texi b/doc/emacs/search.texi
index f3c42bcea7..38430a2ab1 100644
--- a/doc/emacs/search.texi
+++ b/doc/emacs/search.texi
@@ -201,6 +201,14 @@ something before the starting point, type @kbd{C-r} to switch to a
 backward search, leaving the search string unchanged.  Similarly,
 @kbd{C-s} in a backward search switches to a forward search.
 
+@cindex search, changing direction
+@vindex isearch-repeat-on-direction-change
+  When you change the direction of a search, the first command you
+type will, by default, remain on the same match, and the cursor will
+move to the other end of the match.  To move to another match
+immediately, customize the variable
+@code{isearch-repeat-on-direction-change} to @code{t}.
+
 @cindex search, wrapping around
 @cindex search, overwrapped
 @cindex wrapped search
diff --git a/etc/NEWS b/etc/NEWS
index d3a8748ded..8d7b3a6c46 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -367,6 +367,12 @@ trying to be non-destructive.
 This command opens a new buffer called "*Memory Report*" and gives a
 summary of where Emacs is using memory currently.
 
++++
+** New user option 'isearch-repeat-on-direction-change'.
+When this option is set, direction changes in Isearch move to another
+search match, if there is one, instead of moving point to the other
+end of the current match.
+
 ** Outline
 
 +++
diff --git a/lisp/isearch.el b/lisp/isearch.el
index a828c569aa..a41827f9cd 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -185,6 +185,16 @@ When `nil', never wrap, just stop at the last match."
                  (const :tag "Disable wrapping" nil))
   :version "28.1")
 
+(defcustom isearch-repeat-on-direction-change 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))
+  :version "28.1")
+
 (defvar isearch-mode-hook nil
   "Function(s) to call after starting up an incremental search.")
 
@@ -1847,6 +1857,8 @@ Use `isearch-exit' to quit without signaling."
 	      (funcall isearch-wrap-function)
 	    (goto-char (if isearch-forward (point-min) (point-max))))))
     ;; C-s in reverse or C-r in forward, change direction.
+    (if (and isearch-other-end isearch-repeat-on-direction-change)
+        (goto-char isearch-other-end))
     (setq isearch-forward (not isearch-forward)
 	  isearch-success t))
 
@@ -1912,10 +1924,12 @@ of the buffer, type \\[isearch-beginning-of-buffer] with a numeric argument."
         (cond ((< count 0)
                (isearch-repeat-backward (abs count))
                ;; Reverse the direction back
-               (isearch-repeat 'forward))
+               (let ((isearch-repeat-on-direction-change nil))
+                 (isearch-repeat 'forward)))
               (t
                ;; Take into account one iteration to reverse direction
-               (when (not isearch-forward) (setq count (1+ count)))
+               (unless isearch-repeat-on-direction-change
+                 (when (not isearch-forward) (setq count (1+ count))))
                (isearch-repeat 'forward count))))
     (isearch-repeat 'forward)))
 
@@ -1933,10 +1947,12 @@ of the buffer, type \\[isearch-end-of-buffer] with a numeric argument."
         (cond ((< count 0)
                (isearch-repeat-forward (abs count))
                ;; Reverse the direction back
-               (isearch-repeat 'backward))
+               (let ((isearch-repeat-on-direction-change nil))
+                 (isearch-repeat 'backward)))
               (t
                ;; Take into account one iteration to reverse direction
-               (when isearch-forward (setq count (1+ count)))
+               (unless isearch-repeat-on-direction-change
+                 (when isearch-forward (setq count (1+ count))))
                (isearch-repeat 'backward count))))
     (isearch-repeat 'backward)))
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#47599: 28.0.50; Feature request improve/update isearch
  2021-04-07 17:58                   ` Gregory Heytings
@ 2021-04-08 19:05                     ` Juri Linkov
  0 siblings, 0 replies; 15+ messages in thread
From: Juri Linkov @ 2021-04-08 19:05 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: 47599, Ergus

>> 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!





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-04-08 19:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210405020725.ob7bewlin7cid4pa.ref@Ergus>
2021-04-05  2:07 ` bug#47599: 28.0.50; Feature request improve/update isearch Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-04-06 19:16   ` Juri Linkov
2021-04-06 20:38     ` Gregory Heytings
2021-04-06 21:01       ` Gregory Heytings
2021-04-06 21:32         ` Gregory Heytings
2021-04-06 22:39           ` bug#47599: [External] : " Drew Adams
2021-04-06 22:43             ` Gregory Heytings
2021-04-06 23:26               ` Gregory Heytings
2021-04-07 16:20                 ` Juri Linkov
2021-04-07 17:58                   ` Gregory Heytings
2021-04-08 19:05                     ` Juri Linkov
2021-04-07  2:29           ` Eli Zaretskii
2021-04-07 10:44       ` Gregory Heytings
2021-04-07 11:20         ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-04-07 11:33           ` Gregory Heytings

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).