bug#13402: 24.2.92 pretest: bugs in isearch-yank-line in info page. [Patch]

[Alan Mackenzie]

Hello again, Juri.

On Wed, Feb 20, 2013 at 12:49:04PM +0200, Juri Linkov wrote:

> I think you are overestimating the seriousness of the problem.

It is going to be a big problem for a small number of users.  I dare say
the majority won't notice.

> The highlighting effect that you don't like might seem peculiar
> but it has a logical explanation.  Trying to change its logic to
> more complicated will introduce other problems that might seem illogical
> to other users.

Just as activating `search-whitespace-regexp' has done, you mean.  ;-)

> For example, consider the following test case:

> With `search-whitespace-regexp' customized to nil, try to put the cursor
> to the second space character in the string "   Each entry" and type
> `C-M-s SPC'.  It lazy-highlights the previous space character too
> (it's the first space character in the string).  This is right.

> Now with your patch applied, add the character "+" to the search regexp,
> so that a complete search regexp is " +" and the cursor is on the second
> space character in the string "   Each entry".  The result is that
> the first space character is not lazy-highlighted.  I think this is wrong.

:-).  I'm not sure.  It has a certain logic behind it.  I think a missing
lazy-highlight is much less disturbing than an obtrusive one.

> If you assume that the boundary of the current match should begin only
> from the leading character of the search string, then you have to allow
> the preceding match to be lazy-highlighted separately in its own right
> (in the above case the first space character in the string) because
> it matches the search regexp.

That match overlaps the current match.

> IOW, what is more logical to do according to your approach is not to hide
> the lazy-highlighted match under point, but split it to two matches
> where the match preceding the current search position is lazy-highlighted
> and the second match under point is hidden.

I don't agree.  I think a match should either be highlighted completely
or not at all.  But we might be splitting hairs at this point.

> > We can't really document this either.

> Actually, documenting this effect is a very good idea
> that in any case should be done in emacs-24.

> > I'm surprised how difficult the fix is.

> That's is why I prefer to refrain from making hasty changes that might
> cause more problems in unexpected places making other users unhappy.

I think I must now reluctantly agree with you here.  There just aren't
enough pretests left before Emacs 24.3 to test it properly.  I grepped -l
isearch, and it came up with over 70 matching files.  grepping for
isearch-lazy-highlight produced just 4 matches, the already identified
files plus .../lisp/cedet/semantic/senator.el.

How about installing the change in the trunk so that people can try it
out?

Anyhow, just for the sake of completeness, here is the part of the patch
which fixes ispell.e:



=== modified file 'lisp/textmodes/ispell.el'
*** lisp/textmodes/ispell.el    2013-01-01 09:11:05 +0000
--- lisp/textmodes/ispell.el    2013-02-20 21:11:47 +0000
***************
*** 2491,2506 ****
        (delete-overlay ispell-overlay)))
    (if (and ispell-lazy-highlight (boundp 'lazy-highlight-cleanup))
        (if highlight
!         (let ((isearch-string
!                (concat
!                 "\\b"
!                 (regexp-quote (buffer-substring-no-properties start end))
!                 "\\b"))
!               (isearch-regexp t)
!               (isearch-case-fold-search nil))
!           (isearch-lazy-highlight-new-loop
!            (if (boundp 'reg-start) reg-start)
!            (if (boundp 'reg-end)   reg-end)))
        (lazy-highlight-cleanup lazy-highlight-cleanup)
        (setq isearch-lazy-highlight-last-string nil))))
  
--- 2491,2509 ----
        (delete-overlay ispell-overlay)))
    (if (and ispell-lazy-highlight (boundp 'lazy-highlight-cleanup))
        (if highlight
!         (save-excursion
!           (goto-char start)
!           (let ((isearch-string
!                  (concat
!                   "\\b"
!                   (regexp-quote (buffer-substring-no-properties start end))
!                   "\\b"))
!                 (isearch-regexp t)
!                 (isearch-case-fold-search nil)
!                 (isearch-other-end end))
!             (isearch-lazy-highlight-new-loop
!              (if (boundp 'reg-start) reg-start)
!              (if (boundp 'reg-end)   reg-end))))
        (lazy-highlight-cleanup lazy-highlight-cleanup)
        (setq isearch-lazy-highlight-last-string nil))))
  

-- 
Alan Mackenzie (Nuremberg, Germany).