bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Dima Kogan]

Hi. I'm using a very recent emacs built from the git HEAD. Sometime in
the recent past the default C-s behavior was changed to include
char-folding by default. There's a bug here. Suppose I have a buffer
containing the following C source.

----------------------------------------------------
int a(void)
{
  for(unsigned long x = 0;
      x < 10;
      x += 2)
  {
    nvm_flash_erase_app_page( x );
  }
}

int b(void)
{
  for(unsigned long x = 0;
      x < 10;
      x += 2)
  {
  }
}
----------------------------------------------------


Note that the two functions are identical. I place the point at the
start of one of the 'for' statements, then C-s to enter char-folding
isearch, then C-w to grab some amount of text to search for. While I'm
grabbing text that's still on the 'for' line, isearch sees the other
match, highlights it, and I can jump to it by hitting C-s. However, if I
hit C-w enough times to go to the next line, the other match is no
longer seen. This resolves when I turn off char-folding.

bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Juri Linkov]

> Hi. I'm using a very recent emacs built from the git HEAD. Sometime in
> the recent past the default C-s behavior was changed to include
> char-folding by default. There's a bug here. Suppose I have a buffer
> containing the following C source.
>
> ----------------------------------------------------
> int a(void)
> {
>   for(unsigned long x = 0;
>       x < 10;
>       x += 2)
>   {
>     nvm_flash_erase_app_page( x );
>   }
> }
>
> int b(void)
> {
>   for(unsigned long x = 0;
>       x < 10;
>       x += 2)
>   {
>   }
> }
> ----------------------------------------------------
>
>
> Note that the two functions are identical. I place the point at the
> start of one of the 'for' statements, then C-s to enter char-folding
> isearch, then C-w to grab some amount of text to search for. While I'm
> grabbing text that's still on the 'for' line, isearch sees the other
> match, highlights it, and I can jump to it by hitting C-s. However, if I
> hit C-w enough times to go to the next line, the other match is no
> longer seen. This resolves when I turn off char-folding.

Thank you for the bug report.  This can be fixed by a small patch:

diff --git a/lisp/character-fold.el b/lisp/character-fold.el
index bf5ae59..db77845 100644
--- a/lisp/character-fold.el
+++ b/lisp/character-fold.el
@@ -123,7 +123,7 @@ (defun character-fold-to-regexp (string &optional lax)
       (apply #'concat
         (mapcar (lambda (c) (let ((out (or (aref character-fold-table c)
                                       (regexp-quote (string c)))))
-                         (if (and lax (memq c '(?\s ?\t ?\r ?\n )))
+                         (if (memq c '(?\s ?\t ?\r ?\n ))
                              (concat out "+")
                            out)))
                 string))

Later we could also see how to handle both lax-at-the-end-of-the-search-string
and lax-a-sequence-of-whitespace-chars.  Maybe something like:

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 8d4bf24..74b7e56 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2613,7 +2613,11 @@ (defun isearch-search-fun-default ()
 			      (length (isearch--state-string
                                        (car isearch-cmds))))))))
 	(funcall
-	 (if isearch-forward #'re-search-forward #'re-search-backward)
+         (if (and isearch-lax-whitespace search-whitespace-regexp)
+             (if isearch-forward
+                 're-search-forward-lax-whitespace
+               're-search-backward-lax-whitespace)
+           (if isearch-forward #'re-search-forward #'re-search-backward))
 	 (if (functionp isearch-word)
 	     (funcall isearch-word string lax)
 	   (word-search-regexp string lax))

bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Artur Malabarba]

> Thank you for the bug report.  This can be fixed by a small patch:
>
> diff --git a/lisp/character-fold.el b/lisp/character-fold.el
> index bf5ae59..db77845 100644
> --- a/lisp/character-fold.el
> +++ b/lisp/character-fold.el
> @@ -123,7 +123,7 @@ (defun character-fold-to-regexp (string &optional lax)
>        (apply #'concat
>          (mapcar (lambda (c) (let ((out (or (aref character-fold-table c)
>                                        (regexp-quote (string c)))))
> -                         (if (and lax (memq c '(?\s ?\t ?\r ?\n )))
> +                         (if (memq c '(?\s ?\t ?\r ?\n ))

Before applying this, I'd like to figure out why lax is nil here.
IIUC, it is supposed to be t whenever isearch-lax-whitespace is
non-nil.

When I test use-case in the bug report I get that this function is
immediately invoked 3 times. And lax is t in the first, but nil in the
following two.

bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Artur Malabarba]

There is some logic in `isearch-search-fun-default' that I don't quite
understand, and it's giving me trouble.
The following expression is used to decide whether lax-whitespace
matching should be used.

;; Use lax versions to not fail at the end of the word while
;; the user adds and removes characters in the search string
;; (or when using nonincremental word isearch)
(let ((lax (not (or isearch-nonincremental
                    (null (car isearch-cmds))
                    (eq (length isearch-string)
                        (length (isearch--state-string
                                 (car isearch-cmds))))))))
  ...)

I don't understand the purpose of the last clause `(eq (...) (...))'.
For me, the only effect that it has is to disable lax while isearch is
looking for matches beyond the current one.

For instance, here's what happens with me:

1. Type C-s SPC to start isearching for a space.
2. All of the clauses evaluate to nil, and the `isearch-word' function
is called with LAX being t (all good).
3. Immediately (without me typing anything), isearch will start
looking for the next match, but this time the last clause will
evaluate to t. So the `isearch-word' function will be called with LAX
being nil, and some of the upcoming matches will be missed.
4. Step 3 is repeated to find more matches, always with lax being nil.

Re: bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Artur Malabarba]

There is some logic in `isearch-search-fun-default' that I don't quite
understand, and it's giving me trouble.
The following expression is used to decide whether lax-whitespace
matching should be used.

;; Use lax versions to not fail at the end of the word while
;; the user adds and removes characters in the search string
;; (or when using nonincremental word isearch)
(let ((lax (not (or isearch-nonincremental
                    (null (car isearch-cmds))
                    (eq (length isearch-string)
                        (length (isearch--state-string
                                 (car isearch-cmds))))))))
  ...)

I don't understand the purpose of the last clause `(eq (...) (...))'.
For me, the only effect that it has is to disable lax while isearch is
looking for matches beyond the current one.

For instance, here's what happens with me:

1. Type C-s SPC to start isearching for a space.
2. All of the clauses evaluate to nil, and the `isearch-word' function
is called with LAX being t (all good).
3. Immediately (without me typing anything), isearch will start
looking for the next match, but this time the last clause will
evaluate to t. So the `isearch-word' function will be called with LAX
being nil, and some of the upcoming matches will be missed.
4. Step 3 is repeated to find more matches, always with lax being nil.

RE: bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Drew Adams]

Please don't cross-post bug threads to emacs-devel.

If there is something you wish to raise for discussion in emacs-devel,
then do that explicitly.

Or if you want to draw attention in emacs-devel to a particular bug
thread for some reason, then post a single message here that points
to the bug thread.

Re: bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Artur Malabarba]

> Please don't cross-post bug threads to emacs-devel.
>
> If there is something you wish to raise for discussion in emacs-devel,
> then do that explicitly.
>
> Or if you want to draw attention in emacs-devel to a particular bug
> thread for some reason, then post a single message here that points
> to the bug thread.

Sorry. Won't do it again.

bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Artur Malabarba]

I just pushed commit a5bdb87 which should fix this.
Dima, could you confirm that it solves your issue?

bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Dima Kogan]

Artur Malabarba <bruce.connor.am@gmail.com> writes:

> I just pushed commit a5bdb87 which should fix this.
> Dima, could you confirm that it solves your issue?

Yes, that patch resolves the issue. Thanks!

bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)

[Artur Malabarba]

Great!

2015-08-13 23:46 GMT+01:00 Dima Kogan <dima@secretsauce.net>:
> Artur Malabarba <bruce.connor.am@gmail.com> writes:
>
>> I just pushed commit a5bdb87 which should fix this.
>> Dima, could you confirm that it solves your issue?
>
> Yes, that patch resolves the issue. Thanks!
>
>
>