bug#14729: 24.3.50; Isearch oddity

bug#14729: 24.3.50; Isearch oddity

[Dani Moncayo]

Recipe from "emacs -Q":
  M-< b u f C-s C-s f e r

After that, the echo area indicates that, in the current Isearch, I'm
looking for "buffer", and I've found the "buf" part, but not the
remaining "fer".

That makes no sense, because that cannot happen in the current buffer,
where every instance of "buf" is followed by a "fer".


In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-06-24 on LEG570
Bzr revision: 113160 larsi@gnus.org-20130624174222-cndqc1pdsew2la60
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/usr --enable-checking CFLAGS='-O0 -g3'
 CPPFLAGS='-DGLYPH_DEBUG=1 -I/c/usr/include''

--
Dani Moncayo

bug#14729: 24.3.50; Isearch oddity

[Juri Linkov]

> Recipe from "emacs -Q":
>   M-< b u f C-s C-s f e r

I guess this key sequence should be rather:

    M-< C-s b u f C-s C-s f e r

Is this what you meant?

> After that, the echo area indicates that, in the current Isearch, I'm
> looking for "buffer", and I've found the "buf" part, but not the
> remaining "fer".
>
> That makes no sense, because that cannot happen in the current buffer,
> where every instance of "buf" is followed by a "fer".

This is an interesting state of the search.  The search is failed,
so it assumes that adding more characters to the failed search should
leave the search in the failed state.  Isn't this logically correct?
What do you expect it to do in this case?

bug#14729: 24.3.50; Isearch oddity

[Dani Moncayo]

On Fri, Jun 28, 2013 at 1:44 AM, Juri Linkov <juri@jurta.org> wrote:
>> Recipe from "emacs -Q":
>>   M-< b u f C-s C-s f e r
>
> I guess this key sequence should be rather:
>
>     M-< C-s b u f C-s C-s f e r
>
> Is this what you meant?

Yes, of course.  Silly mistake on my part.

>> After that, the echo area indicates that, in the current Isearch, I'm
>> looking for "buffer", and I've found the "buf" part, but not the
>> remaining "fer".
>>
>> That makes no sense, because that cannot happen in the current buffer,
>> where every instance of "buf" is followed by a "fer".
>
> This is an interesting state of the search.  The search is failed,
> so it assumes that adding more characters to the failed search should
> leave the search in the failed state.  Isn't this logically correct?

Of course it is correct.  If there is not match for FOO, obviously
there will be no match for FOOBAR.

> What do you expect it to do in this case?

I expect that the fontification of the echo area reflects what is
happening: that the whole search string ("buffer") is unmatched (not
only the last "fer"), since the current search begins just after the
last instance of "buf" in the buffer [1], and there is no match
(neither full nor partial) of the current search string between that
location and the end of the buffer.


------------
[1] Think of "C-s" within an Isearch session as "repeat the current
search, but starting at the current point location".

--
Dani Moncayo

bug#14729: 24.3.50; Isearch oddity

[Juri Linkov]

> I expect that the fontification of the echo area reflects what is
> happening: that the whole search string ("buffer") is unmatched (not
> only the last "fer"), since the current search begins just after the
> last instance of "buf" in the buffer [1], and there is no match
> (neither full nor partial) of the current search string between that
> location and the end of the buffer.

`isearch-fail-pos' assumes that in a failed state of search
a shorter previous successful search string should be taken
as a basis for highlighting.  In this case, the last successful match
was "buf", so an appended suffix is highlighted as failed.

bug#14729: 24.3.50; Isearch oddity

[Dani Moncayo]

On Sat, Jun 29, 2013 at 11:53 PM, Juri Linkov <juri@jurta.org> wrote:
>> I expect that the fontification of the echo area reflects what is
>> happening: that the whole search string ("buffer") is unmatched (not
>> only the last "fer"), since the current search begins just after the
>> last instance of "buf" in the buffer [1], and there is no match
>> (neither full nor partial) of the current search string between that
>> location and the end of the buffer.
>
> `isearch-fail-pos' assumes that in a failed state of search
> a shorter previous successful search string should be taken
> as a basis for highlighting.  In this case, the last successful match
> was "buf", so an appended suffix is highlighted as failed.

Then the mistake is not re-setting that "last successful match" after
each "C-s", because here, the last successful match was "buf" only
after doing "M-< C-s buf" and also after the subsequent "C-s", but
once you type the third "C-s", the whole search string ("buf") is
unmatched (because the search now begins at [1]), and that fact (fully
unmatched search string) cannot change by appending more characters to
the search string.

IOW: After the third "C-s", the search goes from [1] to the end of the
buffer, and therefore any search string starting with a character
different from "f", "e", "r", "." or whitespace will be fully
unmatched, and should be highlighted as such in the echo area.

-----------------------
[1] Position between "buf" and "fer" in the last instance of the
string "buffer".

--
Dani Moncayo

bug#14729: 24.3.50; Isearch oddity

[Drew Adams]

(There may be another way to approach this.  This is what I
have to offer.)

On way of looking at this has to do with the behavior of
`isearch-printing-char'.  If you change it to add the following
`while' sexp then the reported problem disappears:

(defun isearch-printing-char (&optional char count)
    "Append ordinary printing CHAR to the search string and search.
With a numeric prefix arg, append that many copies of CHAR."
    (interactive (list last-command-event
                       (prefix-numeric-value current-prefix-arg)))

    ;; ADDED THIS `while'.
    (while (or (not isearch-success)  isearch-error)
      (isearch-pop-state))

    (let ((char  (or char  last-command-event)))
      (when (= char ?\S-\ ) (setq char ?\  ))
      (if current-input-method
          (isearch-process-search-multibyte-characters char count)
        (isearch-process-search-char char count))))

However, that also changes the behavior in general, in this way:
Whenever there is a mismatch and you type something, what you type
replaces the mismatched part.  That is probably not the behavior
we want, in general.  It might be the behavior some people want
in general or the behavior that anyone might want some of the time.

FWIW, in Isearch+ the above `while' is conditional on the value
of a user option, `isearchp-drop-mismatch'.  Only when the option
value is `replace-last' is the `while' evaluated and thus the
state popped.

There are 3 possible values for the option.  Because you might
sometimes want different behavior, you can cycle among the
values during Isearch using `M-k' (command
`isearchp-cycle-mismatch-removal').  The values:

 nil            - Your current input is appended to the search
                  string, even if the string already has a
                  mismatched portion.

 `replace-last' - Your current input replaces the last mismatched
                  text.  You can always see your last input, even
                  if it is a mismatch.  And it is available for
                  editing using `M-e' (which puts point at the
                  mismatch position).

 anything else  - Your current input is ignored for searching
                  if it causes a mismatch: it is, in effect, not
                  added to the search string, which thus always
                  has only successful matches.

The default value is nil, which gives the same behavior as vanilla
Isearch (including the behavior that you reported): When the
search does not match and you type more text it is simply appended,
even though it does not help with matching (but in the case of a
regexp, it might). 

Value `replace-last' automatically replaces the mismatched portion
by what you type.  This also has the effect of fixing the scenario
you describe.

E.g., if you type `bufx' then the `x' is a mismatch for `buffer'.
If you then type `m', the `m' replaces the `x' (`bufm'), and the
`m' is now the mismatched portion.  If you then type `f' then the
match is extended to `buff' (there is no longer a mismatch).

Any other option value means you never see a mismatch in the
search string.  Anything you type that does not match is dropped,
as if you hadn't typed it.

As I say, there might well be another way to give you the behavior
you expect/prefer here.  The `replace-last' approach is really
designed for something other than your scenario.  It just happens
to take care of this case because what would be a (mistakenly)
highlighted `f' is immediately replaced by an unhighlighted `f'
(you never see it highlighted).

Presumably there is a way to prevent Isearch from recognizing the
reported scenario as a mismatch, rather than applying `replace-last'
as a bandaid.  Perhaps Juri has an idea.  He is more familiar with
the new Isearch state structure etc.

bug#14729: 24.3.50; Isearch oddity

[Lars Ingebrigtsen]

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

Juri Linkov <juri@jurta.org> writes:

> `isearch-fail-pos' assumes that in a failed state of search
> a shorter previous successful search string should be taken
> as a basis for highlighting.  In this case, the last successful match
> was "buf", so an appended suffix is highlighted as failed.

So, first we have:


[-- Attachment #2: Type: image/png, Size: 6050 bytes --]

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


Then we type "fer", and then get:


[-- Attachment #4: Type: image/png, Size: 10937 bytes --]

[-- Attachment #5: Type: text/plain, Size: 37 bytes --]


But...  I think I'd expect getting


[-- Attachment #6: Type: image/png, Size: 7103 bytes --]

[-- Attachment #7: Type: text/plain, Size: 125 bytes --]


instead?  Probably.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14729: [External] : bug#14729: 24.3.50; Isearch oddity

[Drew Adams]

Please read what I wrote in the thread, if you
haven't already:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=14729#20

There are different use cases and expected/preferred
behaviors.  What Isearch+ does lets users specify
their overall preference, and it lets them change
the behavior on the fly (during Isearch).

Vanilla Emacs could provide similar flexibility.
(Or it can just hard-code one such behavior, as it
does now.)

bug#14729: 24.3.50; Isearch oddity

[Juri Linkov]

>> `isearch-fail-pos' assumes that in a failed state of search
>> a shorter previous successful search string should be taken
>> as a basis for highlighting.  In this case, the last successful match
>> was "buf", so an appended suffix is highlighted as failed.
>
> So, first we have:
>
>
>
>
> Then we type "fer", and then get:
>
>
>
>
> But...  I think I'd expect getting
>
>
>
>
> instead?  Probably.

Or to turn the failing search into a successful one
since the search string matches the buffer.

OTOH, the current behavior also makes sense
since it highlights the last failing characters
typed by the user.  E.g. compare it with `C-s bufzzz'.

bug#14729: 24.3.50; Isearch oddity

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

> Or to turn the failing search into a successful one
> since the search string matches the buffer.

But we've skipped past the final match, so I think it's correct to still
say that we've skipped past, even if we add more characters.  Probably.

> OTOH, the current behavior also makes sense
> since it highlights the last failing characters
> typed by the user.  E.g. compare it with `C-s bufzzz'.

Hm...  yes...  but it does seem wrong (in one way or another) in the
original test case.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14729: 24.3.50; Isearch oddity

[Juri Linkov]

>> Or to turn the failing search into a successful one
>> since the search string matches the buffer.
>
> But we've skipped past the final match,

Actually, the final match is not skipped - point is still
on the final match.  That error is just an intermediate step
before wrapping.

> so I think it's correct to still say that we've skipped past, even if
> we add more characters.  Probably.

When the users type more characters on the wrapping search,
isn't this an indication that the users changed their mind
and decided to continue searching for more text?

>> OTOH, the current behavior also makes sense
>> since it highlights the last failing characters
>> typed by the user.  E.g. compare it with `C-s bufzzz'.
>
> Hm...  yes...  but it does seem wrong (in one way or another) in the
> original test case.

Honestly, I have no idea what is right and what is wrong in this situation 🤐

bug#14729: 24.3.50; Isearch oddity

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

>> so I think it's correct to still say that we've skipped past, even if
>> we add more characters.  Probably.
>
> When the users type more characters on the wrapping search,
> isn't this an indication that the users changed their mind
> and decided to continue searching for more text?

Or...  perhaps they're building up the string for their next (wrapped)
`C-s'.  😀

> Honestly, I have no idea what is right and what is wrong in this situation 🤐

Me neither, but it's not quite optimal the way it is now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14729: [External] : bug#14729: 24.3.50; Isearch oddity

[Drew Adams]

> > Honestly, I have no idea what is right and what
> > is wrong in this situation
> 
> Me neither, but it's not quite optimal the way it is now.

That exchange is telling.

As I suggested, let users control the behavior
(Isearch+ does that).  Don't look for a single
behavior that's "right" for everyone in all
contexts.

bug#14729: [External] : bug#14729: 24.3.50; Isearch oddity

[Juri Linkov]

>> > Honestly, I have no idea what is right and what
>> > is wrong in this situation
>>
>> Me neither, but it's not quite optimal the way it is now.
>
> That exchange is telling.
>
> As I suggested, let users control the behavior
> (Isearch+ does that).  Don't look for a single
> behavior that's "right" for everyone in all
> contexts.

This means adding a new user option for this small detail?

bug#14729: [External] : bug#14729: 24.3.50; Isearch oddity

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

> This means adding a new user option for this small detail?

Nope.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14729: [External] : bug#14729: 24.3.50; Isearch oddity

[Drew Adams]

> >> > Honestly, I have no idea what is right and what
> >> > is wrong in this situation
> >>
> >> Me neither, but it's not quite optimal the way it is now.
> >
> > That exchange is telling.
> >
> > As I suggested, let users control the behavior
> > (Isearch+ does that).  Don't look for a single
> > behavior that's "right" for everyone in all
> > contexts.
> 
> This means adding a new user option for this small detail?

I described what I do in Isearch+.  There's a
user option, AND users can choose/change the
current behavior on the fly, during Isearch.

There are of course other ways to skin this cat.

My point above was that you seem to have come to
the (correct) conclusion that one size doesn't
fit all.  How you want to go forward from that
conclusion is TBD.

bug#14729: 24.3.50; Isearch oddity

[Juri Linkov]

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

>> Or to turn the failing search into a successful one
>> since the search string matches the buffer.
>
> But we've skipped past the final match, so I think it's correct to still
> say that we've skipped past, even if we add more characters.  Probably.

The matched string is highlighted partly in the buffer,
and exactly the same parts are highlighted in the isearch prompt too -
the string "buf" is highlighted as the matched string in the buffer,
and the same string "buf" has no failure indication in the isearch prompt.
So currently there is consistency between these highlightings:


[-- Attachment #2: isearch-fail-pos.png --]
[-- Type: image/png, Size: 16310 bytes --]

bug#14729: 24.3.50; Isearch oddity

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

> The matched string is highlighted partly in the buffer,
> and exactly the same parts are highlighted in the isearch prompt too -
> the string "buf" is highlighted as the matched string in the buffer,
> and the same string "buf" has no failure indication in the isearch prompt.
> So currently there is consistency between these highlightings:

But before we type "fer", "buf" is highlit as failing, but after "fer"
is highlit as failing (in the echo area), while the highlighting in the
buffer doesn't change.  😀

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14729: 24.3.50; Isearch oddity

[Juri Linkov]

>> The matched string is highlighted partly in the buffer,
>> and exactly the same parts are highlighted in the isearch prompt too -
>> the string "buf" is highlighted as the matched string in the buffer,
>> and the same string "buf" has no failure indication in the isearch prompt.
>> So currently there is consistency between these highlightings:
>
> But before we type "fer", "buf" is highlit as failing, but after "fer"
> is highlit as failing (in the echo area), while the highlighting in the
> buffer doesn't change.  😀

This is why the subject is "Search oddity": it's working in a most
peculiar way, and the circuit's dead, and there's something wrong 😁