isearch-repeat-forward: wrong overlay after failure

isearch-repeat-forward: wrong overlay after failure

[David Reitter]

isearch wrongly highlights a part of a buffer when `isearch-forward'  
fails.

To reproduce, create buffer A, containing just the following:

123456

Also, create buffer B, containing

12346

Now switch to buffer A, move point to the beginning, and do C-s 5.   
The `5' will be highlighted.
Then switch to buffer B and move point to the beginning.  (You can  
show the buffers in two windows, or just switch between them: it  
doesn't matter.)
Then do M-x isearch-repeat-forward.  (Do NOT do C-s C-s.)

You will find that `6' is highlighted, and a (correct) message is  
displayed that the search failed.  (It appears that the text at a  
position equivalent to where the last match was in the previous buffer  
is highlighted.)

The overlay in buffer B shouldn't be there.

This issue has been around for a long time, and I suspect it was never  
noticed because many people just do C-s C-s rather than binding a key  
to `isearch-repeat-forward'.






In GNU Emacs 22.2.50.2 (i386-apple-darwin9.2.2, Carbon Version 1.6.0)
  of 2008-04-26 on plume.sr.unh.edu
Windowing system distributor `Apple Inc.', version 10.5.2
configured using `configure  '--without-x' '--prefix=/usr/local''

Important settings:
   value of $LC_ALL: nil
   value of $LC_COLLATE: nil
   value of $LC_CTYPE: nil
   value of $LC_MESSAGES: nil
   value of $LC_MONETARY: nil
   value of $LC_NUMERIC: nil
   value of $LC_TIME: nil
   value of $LANG: nil
   locale-coding-system: iso-8859-1
   default-enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
   tooltip-mode: t
   tool-bar-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   unify-8859-on-encoding-mode: t
   utf-translate-cjk-mode: t
   auto-compression-mode: t
   line-number-mode: t

Recent input:
C-x b <return> <up> <up> <up> <up> <up> C-k C-k C-k
C-k C-k C-k 1 <return> 2 <return> 3 <return> 4 <return>
5 <return> 6 5 <return> <backspace> <backspace> <return>
<escape> x v i s i t - n e <tab> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> c r e a t e - b <tab> u <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> v i s i <tab> C-x
C-f T e m p SPC t 1 <backspace> <backspace> <backspace>
/ t 2 <return> 1 <return> 2 <return> 3 <return> 4 <return>
6 <return> <up> <up> <up> <up> <up> <up> <up> <up>
<up> <up> C-x b <return> <up> <up> <up> <up> <up> <up>
<up> <up> <up> <up> <up> <up> <up> C-s 5 C-x b <return>
<escape> x i s e a r c h - r e <tab> <tab> p e <tab>
f o r <tab> <return> <menu-bar> <help-menu> <send-
emacs-bug-report>

Recent messages:
Making completion list...
Loading help-mode...done
Quit
(New file)
Mark saved where search started
Making completion list...
Loading emacsbug...
Loading regexp-opt...done
Loading emacsbug...done
byte-code: Text is read-only [2 times]

Re: isearch-repeat-forward: wrong overlay after failure

[Juri Linkov]

> This issue has been around for a long time, and I suspect it was never
> noticed because many people just do C-s C-s rather than binding a key  to
> isearch-repeat-forward'.

`isearch-repeat-forward' is not an entry point to the incremental search:
it doesn't activate isearch mode neither deactivates it.  IIUC, it was
never intended for non-incremental search, so it doesn't guarantee to
work when isearch mode is not active.

If you want to find the next occurrence without highlighting it
and without activating isearch mode, you can use the command
`search-forward'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: isearch-repeat-forward: wrong overlay after failure

[David Reitter]

On 11 May 2008, at 17:59, Juri Linkov wrote:
>
> `isearch-repeat-forward' is not an entry point to the incremental  
> search:
> it doesn't activate isearch mode neither deactivates it.  IIUC, it was
> never intended for non-incremental search, so it doesn't guarantee to
> work when isearch mode is not active.
>
> If you want to find the next occurrence without highlighting it
> and without activating isearch mode, you can use the command
> `search-forward'.

isearch-repeat-forward is interactive, which is why I thought it would  
be a decent entry point.
It works beautifully, except when you switch buffers and when  
nonsensical text is highlighted.

Something like  (search-forward isearch-string) works too, as long as  
the match is visualized.  That's what I'm doing now; thanks for  
pointing out this (better) variant.

Re: isearch-repeat-forward: wrong overlay after failure

[David Kastrup]

David Reitter <david.reitter@gmail.com> writes:

> On 11 May 2008, at 17:59, Juri Linkov wrote:
>>
>> `isearch-repeat-forward' is not an entry point to the incremental
>> search:
>> it doesn't activate isearch mode neither deactivates it.  IIUC, it was
>> never intended for non-incremental search, so it doesn't guarantee to
>> work when isearch mode is not active.
>>
>> If you want to find the next occurrence without highlighting it
>> and without activating isearch mode, you can use the command
>> `search-forward'.
>
> isearch-repeat-forward is interactive, which is why I thought it would
> be a decent entry point.

Which I would call a reasonable expectation.  However, keybindings even
in specialized keymaps need to be bound to interactive functions.  So if
we have functions that are only intended for use in specialized keymaps,
there is no way to keep people from binding them to normal keymaps.

So it is sort of a combined documentation/expectation problem.  I have
no good idea how to approach this.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: isearch-repeat-forward: wrong overlay after failure

[Lennart Borgman (gmail)]

David Kastrup wrote:
> David Reitter <david.reitter@gmail.com> writes:
> 
>> On 11 May 2008, at 17:59, Juri Linkov wrote:
>>> `isearch-repeat-forward' is not an entry point to the incremental
>>> search:
>>> it doesn't activate isearch mode neither deactivates it.  IIUC, it was
>>> never intended for non-incremental search, so it doesn't guarantee to
>>> work when isearch mode is not active.
>>>
>>> If you want to find the next occurrence without highlighting it
>>> and without activating isearch mode, you can use the command
>>> `search-forward'.
>> isearch-repeat-forward is interactive, which is why I thought it would
>> be a decent entry point.
> 
> Which I would call a reasonable expectation.  However, keybindings even
> in specialized keymaps need to be bound to interactive functions.  So if
> we have functions that are only intended for use in specialized keymaps,
> there is no way to keep people from binding them to normal keymaps.
> 
> So it is sort of a combined documentation/expectation problem.  I have
> no good idea how to approach this.

Maybe an error message that explains/points to an explanation?

Re: isearch-repeat-forward: wrong overlay after failure

[David Kastrup]

"Lennart Borgman (gmail)" <lennart.borgman@gmail.com> writes:

> David Kastrup wrote:
>> David Reitter <david.reitter@gmail.com> writes:
>>
>>> On 11 May 2008, at 17:59, Juri Linkov wrote:
>>>> `isearch-repeat-forward' is not an entry point to the incremental
>>>> search:
>>>> it doesn't activate isearch mode neither deactivates it.  IIUC, it was
>>>> never intended for non-incremental search, so it doesn't guarantee to
>>>> work when isearch mode is not active.
>>>>
>>>> If you want to find the next occurrence without highlighting it
>>>> and without activating isearch mode, you can use the command
>>>> `search-forward'.
>>> isearch-repeat-forward is interactive, which is why I thought it would
>>> be a decent entry point.
>>
>> Which I would call a reasonable expectation.  However, keybindings even
>> in specialized keymaps need to be bound to interactive functions.  So if
>> we have functions that are only intended for use in specialized keymaps,
>> there is no way to keep people from binding them to normal keymaps.
>>
>> So it is sort of a combined documentation/expectation problem.  I have
>> no good idea how to approach this.
>
> Maybe an error message that explains/points to an explanation?

What should trigger an error here?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: isearch-repeat-forward: wrong overlay after failure

[Juri Linkov]

>>> Which I would call a reasonable expectation.  However, keybindings even
>>> in specialized keymaps need to be bound to interactive functions.  So if
>>> we have functions that are only intended for use in specialized keymaps,
>>> there is no way to keep people from binding them to normal keymaps.
>>>
>>> So it is sort of a combined documentation/expectation problem.  I have
>>> no good idea how to approach this.
>>
>> Maybe an error message that explains/points to an explanation?
>
> What should trigger an error here?

  (unless isearch-mode
    (error "This command doesn't work when isearch mode is not active"))

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: isearch-repeat-forward: wrong overlay after failure

[David Reitter]

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

On 12 May 2008, at 00:59, Juri Linkov wrote:
>
>  (unless isearch-mode
>    (error "This command doesn't work when isearch mode is not  
> active"))


Would this

  (unless isearch-mode
	(isearch-mode 1))

be not more reasonable?

- D

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: bug#222: isearch-repeat-forward: wrong overlay after failure

[David Reitter]

On 12 May 2008, at 08:21, David Reitter wrote:
>
> (unless isearch-mode
> 	(isearch-mode 1))

Another alternative would be to make isearch-repeat-forward simply  
work in this case because it does something useful.
We've had Command-G bound to it for three years now (i.e., in  
Aquamacs, with Command-F bound to isearch-forward) and I've never had  
any complaints from users about the functionality.

Re: isearch-repeat-forward: wrong overlay after failure

[Stefan Monnier]

>>> `isearch-repeat-forward' is not an entry point to the incremental
>>> search:
>>> it doesn't activate isearch mode neither deactivates it.  IIUC, it was
>>> never intended for non-incremental search, so it doesn't guarantee to
>>> work when isearch mode is not active.
>>> 
>>> If you want to find the next occurrence without highlighting it
>>> and without activating isearch mode, you can use the command
>>> `search-forward'.
>> 
>> isearch-repeat-forward is interactive, which is why I thought it would
>> be a decent entry point.

> Which I would call a reasonable expectation.  However, keybindings even
> in specialized keymaps need to be bound to interactive functions.  So if
> we have functions that are only intended for use in specialized keymaps,
> there is no way to keep people from binding them to normal keymaps.

> So it is sort of a combined documentation/expectation problem.  I have
> no good idea how to approach this.

It's a very general problem: many keybindings used in major-mode or
minor modes only work properly if the corresponding mode is
currently active.

Of course, many work perfectly in other contexts too, and yet many more
work 98% OK as well.

I don't think it makes sense to document each and every function
describing whether it's expected to work in other modes or not.


        Stefan

Re: bug#222: isearch-repeat-forward: wrong overlay after failure

[Juri Linkov]

>> (unless isearch-mode
>> 	(isearch-mode 1))
>
> Another alternative would be to make isearch-repeat-forward simply work in
> this case because it does something useful.

I think we need a general function that will start isearch with
an arbitrary string.  This string could be the last search string
or the word/symbol/thing at point.  The latter will provide a
useful feature currently presented in Vim (keys `*' and `#').
There is one implementation I helped to create in
http://www.emacswiki.org/cgi-bin/wiki/SearchAtPoint
This could be generalized and added to isearch.

-- 
Juri Linkov
http://www.jurta.org/emacs/