bug#22118: 23.2; Hitting ^W in a search selects the wrong word.

[Stefan Kangas <stefan@marxist.se>]

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

Juri Linkov <juri@linkov.net> writes:

>> Is the below patch still relevant?
>
> I don't know.  My patch tried to make the behavior in case of error
> slightly more useful, but now I see nothing useful in it because
> after a failure it makes no sense to type C-w C-w C-w ...
>
> I don't recommend making code in isearch.el more complicated to handle
> useless cases.  So I won't complain if you'll close this report.  :-)

Jan-Mark <jms@codersco.com> writes:

>> OK, thanks.  Any objections to closing this?
> I don't remember even what this was about. But ^W should not add words at the cursor if the
> search failed to match. Which is different from the search failing because it is at the last
> match (about to warp to the first match again).
>
> If the word 'aapX' is not in my text, 'aap noot' is, and I search for 'aapX' it brings me to
> 'aap noot' with a failure notice. If I than type ^W it should not start looking for 'aapX noot'.
> Just take the ^W out of the game in cases like this. I mean if 'aapX' cannot be found (even
> once) why should you allow people to look for 'aapX noot'?
>
> I still feel it is a bug, but I agree that it will not hit many people often. I have this once
> every 1000 hours (estimate). So it's not a big bug. But a bug nonetheless.
>
> If there is bigger bugs to fry, close it, if it is an easy fix (should be?) I'd say fix it first.

Thanks.

Juri feels that we should close this bug as a wontfix because it is not
worth complicating the isearch code over this.

Testing this again, I'm leaning more towards fixing the bug.  I think
there is definitely a bug here (the recipe in OP is still reproducible
on master), but it is very minor.  We also already have a proposed fix.

I considered also the use case when you have 'isearch-wrapped' set to a
non-nil value, which would make yank after last occurrence more useful.
I do remember having seen stuff similar to this in the past.
(Unfortunately I no longer use isearch and can't remember the exact
details, or if it was this exact bug.)

I therefore propose to push the proposed changes to master.  The
complexity is localized to only one function, which is already in itself
not too complicated.  I've attached a patch with proper ChangeLog for
review.

Juri, if you are still not convinced, we can go ahead and close this as
wontfix.  I'm okay with either option.

Best regards,
Stefan Kangas

[-- Attachment #2: 0001-Fix-isearch-yank-on-last-occurrence-of-search-string.patch --]
[-- Type: text/x-diff, Size: 1578 bytes --]

From c5ebd3ee6c2c14727d926aef5d8e36682a00bb59 Mon Sep 17 00:00:00 2001
From: Juri Linkov <juri@linkov.net>
Date: Sat, 29 Aug 2020 18:33:05 +0200
Subject: [PATCH] Fix isearch yank on last occurrence of search string

* lisp/isearch.el (isearch-yank-internal):
(isearch-yank-prev-point): Fix yanking when we are at the last
occurrence of a search string.  (Bug#22118)
---
 lisp/isearch.el | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 81e83d7950..ac8b48e43a 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2518,6 +2518,8 @@ isearch-xterm-paste
     (let ((pasted-text (nth 1 event)))
       (isearch-yank-string pasted-text))))
 
+(defvar isearch-yank-prev-point nil)
+
 (defun isearch-yank-internal (jumpform)
   "Pull the text from point to the point reached by JUMPFORM.
 JUMPFORM is a lambda expression that takes no arguments and returns
@@ -2528,7 +2530,14 @@ isearch-yank-internal
    (save-excursion
      (and (not isearch-forward) isearch-other-end
 	  (goto-char isearch-other-end))
-     (buffer-substring-no-properties (point) (funcall jumpform)))))
+     (and (not isearch-success) isearch-yank-prev-point
+	  (goto-char isearch-yank-prev-point))
+     (buffer-substring-no-properties
+      (point)
+      (prog1
+	  (setq isearch-yank-prev-point (funcall jumpform))
+	(when isearch-success
+	  (setq isearch-yank-prev-point nil)))))))
 
 (defun isearch-yank-char-in-minibuffer (&optional arg)
   "Pull next character from buffer into end of search string in minibuffer."
-- 
2.28.0