unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)
@ 2015-07-31  4:06 Dima Kogan
  2015-08-02 20:40 ` Juri Linkov
  0 siblings, 1 reply; 7+ messages in thread
From: Dima Kogan @ 2015-07-31  4:06 UTC (permalink / raw)
  To: 21164

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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)
  2015-07-31  4:06 bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes) Dima Kogan
@ 2015-08-02 20:40 ` Juri Linkov
  2015-08-05 17:20   ` Artur Malabarba
  0 siblings, 1 reply; 7+ messages in thread
From: Juri Linkov @ 2015-08-02 20:40 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 21164

> 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))





^ permalink raw reply related	[flat|nested] 7+ messages in thread

* bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)
  2015-08-02 20:40 ` Juri Linkov
@ 2015-08-05 17:20   ` Artur Malabarba
  2015-08-05 18:16     ` Artur Malabarba
  2015-08-09  9:00     ` Artur Malabarba
  0 siblings, 2 replies; 7+ messages in thread
From: Artur Malabarba @ 2015-08-05 17:20 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Dima Kogan, 21164

> 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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)
  2015-08-05 17:20   ` Artur Malabarba
@ 2015-08-05 18:16     ` Artur Malabarba
  2015-08-09  9:00     ` Artur Malabarba
  1 sibling, 0 replies; 7+ messages in thread
From: Artur Malabarba @ 2015-08-05 18:16 UTC (permalink / raw)
  To: Juri Linkov, emacs-devel; +Cc: Dima Kogan, 21164

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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)
  2015-08-05 17:20   ` Artur Malabarba
  2015-08-05 18:16     ` Artur Malabarba
@ 2015-08-09  9:00     ` Artur Malabarba
  2015-08-13 22:46       ` Dima Kogan
  1 sibling, 1 reply; 7+ messages in thread
From: Artur Malabarba @ 2015-08-09  9:00 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Dima Kogan, 21164

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





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)
  2015-08-09  9:00     ` Artur Malabarba
@ 2015-08-13 22:46       ` Dima Kogan
  2015-08-14 16:39         ` Artur Malabarba
  0 siblings, 1 reply; 7+ messages in thread
From: Dima Kogan @ 2015-08-13 22:46 UTC (permalink / raw)
  To: 21164

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!





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes)
  2015-08-13 22:46       ` Dima Kogan
@ 2015-08-14 16:39         ` Artur Malabarba
  0 siblings, 0 replies; 7+ messages in thread
From: Artur Malabarba @ 2015-08-14 16:39 UTC (permalink / raw)
  Cc: 21164-done

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!
>
>
>





^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-08-14 16:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-31  4:06 bug#21164: 25.0.50; char-fold search broken for multi-line searches (sometimes) Dima Kogan
2015-08-02 20:40 ` Juri Linkov
2015-08-05 17:20   ` Artur Malabarba
2015-08-05 18:16     ` Artur Malabarba
2015-08-09  9:00     ` Artur Malabarba
2015-08-13 22:46       ` Dima Kogan
2015-08-14 16:39         ` Artur Malabarba

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).