GNU bug report logs - #40940
27.0.91; project-query-replace-regexp stops too early

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: simenheg <at> runbox.com, monnier <at> IRO.UMontreal.CA, 40940 <at> debbugs.gnu.org
Subject: bug#40940: 27.0.91; project-query-replace-regexp stops too early
Date: Fri, 01 May 2020 09:57:24 +0300

> Cc: simenheg <at> runbox.com, monnier <at> IRO.UMontreal.CA, 40940 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Wed, 29 Apr 2020 23:53:46 +0300
> 
> > Also, using downcase is not entirely correct, we should
> > use isearch-no-upper-case-p instead.
> 
> I usually like isearch-no-upper-case-p behavior, but here it doesn't fit 
> the docstring, does it? There is no value of CASE-FOLD described that 
> would indicate that the function will be too smart about it.
> 
> So we'd need to update the docstring that the CASE-FOLD value of t will 
> obey the value of `search-upper-case'. Then we'll consult it as well, 
> like isearch-search does.

I don't think I follow.  isearch-no-upper-case-p is just supposed to
tell you whether a regexp includes upper-case characters that would
need the search to become case-sensitive.  All the rest of the
considerations, like the value of case-fold-search, are to be applied
by the caller.  I see no reference to either case-fold-search or
case-fold in isearch-no-upper-case-p.  So why would we need to update
its doc string?

> Alternatively, we could for now use the below patch which changes the 
> behavior less:
> 
> diff --git a/lisp/fileloop.el b/lisp/fileloop.el
> index 543963feaf..c3c55badf5 100644
> --- a/lisp/fileloop.el
> +++ b/lisp/fileloop.el
> @@ -175,14 +175,16 @@ fileloop-continue
>               (funcall fileloop--operate-function)))
>         (setq file-finished t))))
> 
> +(defun fileloop--case-fold (arg)
> +  (if (memq arg '(t nil)) arg case-fold-search))
> +
>   ;;;###autoload
>   (defun fileloop-initialize-search (regexp files case-fold)
>     (let ((last-buffer (current-buffer)))
>       (fileloop-initialize
>        files
>        (lambda ()
> -       (let ((case-fold-search
> -              (if (memq case-fold '(t nil)) case-fold case-fold-search)))
> +       (let ((case-fold-search (fileloop--case-fold case-fold)))
>            (re-search-forward regexp nil t)))
>        (lambda ()
>          (unless (eq last-buffer (current-buffer))
> @@ -203,15 +205,16 @@ fileloop-initialize-replace
>     (fileloop-initialize
>      files
>      (lambda ()
> -     (let ((case-fold-search
> -            (if (memql case-fold '(nil t)) case-fold case-fold-search)))
> +     (let ((case-fold-search (fileloop--case-fold case-fold)))
>          (if (re-search-forward from nil t)
>   	   ;; When we find a match, move back
>   	   ;; to the beginning of it so perform-replace
>   	   ;; will see it.
>   	   (goto-char (match-beginning 0)))))
>      (lambda ()
> -     (perform-replace from to t t delimited nil multi-query-replace-map))))
> +     (let ((case-fold-search (fileloop--case-fold case-fold))
> +           search-upper-case)
> +       (perform-replace from to t t delimited nil multi-query-replace-map)))))

Does that really work in the case in point?  Unless I'm missing
something, if case-fold-search is t by default, this patch would cause
the search to be case-insensitive even if the FROM regexp includes
upper-case characters.  But in that case, perform-replace will
internally decide to be case-sensitive, and we have the same failure
on our hands.  This is why the patch I proposed explicitly examined
the FROM regexp for upper-case characters.  Whereas yours doesn't.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.