Re: isearch-push-state

[Juri Linkov <juri@jurta.org>]

> Thanks Juri.  I'm not sure I understand what you said: are you saying
> that those extra state-pushes are just unnecessary or are they
> really harmful?

They are both unnecessary and harmful for different reasons.

They are unnecessary when doing backtracking with DEL.  They add an
unnecessary and inconsistent state that puts point to the beginning
of the previous successful match, lazy-highlights matches
for the string later entered in the minibuffer, and no string
is highlighted at the current successful match.

So for instance, `C-s foo M-e C-a C-k bar C-s DEL' in a buffer
where both strings "foo" and "bar" exist, isearch enters the
state where point is on "foo", the echo area displays
"I-search: bar", "bar" is highlighted in `lazy-highlight' face,
and no string is highlighted in `isearch' face even though
the echo area says it returned to the state where it successfully
found the string "bar".

The first part of my patch removes this inconsistent state in
`isearch-edit-string', but keeps the second call to `isearch-push-state'
that records the correct state after searching for a new string.

The second part of the patch (in `isearch-ring-adjust') removes the state
that is a duplicate of the state added by `isearch-edit-string'.

> It seems they are harmful w.r.t the new failed-search-highlight
> thingy, but shouldn't the failed-search-highlight thingy try to pop
> several states until findind one that does succeed?

This is what it already tries to do, but fails when encounters an
inconsistent state, e.g. `C-s foo M-e C-a C-k bar C-s in a buffer
where only the string "foo" exists, the stack contains a state
marked as successful with the failed search string:

(["bar" "bar" 194 nil t nil nil nil nil 192 t nil]
 ["bar" "bar" 194 197 t nil nil nil nil 192 t nil] <-- wrong state
 ["foo" "foo" 197 197 t 194 nil nil nil 192 t nil]
 ["fo"  "fo"  196 196 t 194 nil nil nil 192 t nil]
 ["f"   "f"   195 195 t 194 nil nil nil 192 t nil]
 [""    ""    192 t   t nil nil nil nil 192 t nil])

> I'm not sure I like the idea that "spurious" state pushes can really
> be harmful.
>
> This said, wherever you remove those pushes, please replace them with
> a comment mentioning that it was there and why you think
> it's unnecessary.

OK, will do.

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