all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#40940: 27.0.91; project-query-replace-regexp stops too early
@ 2020-04-28 14:35 Simen Heggestøyl
  2020-04-29  3:54 ` Dmitry Gutov
  0 siblings, 1 reply; 38+ messages in thread
From: Simen Heggestøyl @ 2020-04-28 14:35 UTC (permalink / raw)
  To: 40940


Create a directory with two files in it: test-2.txt and test.txt:

test-dir
├── test-2.txt
└── test.txt

Enter the text "Foo" into test.txt.
Enter the text "foo" into test-2.txt.

Visit the directory or any of the files (the result is the same either
way).

Type 'M-x project-query-replace-regexp', choosing to replace "Foo"
with "Bar".

Confirm test-dir as the project directory in the next prompt.

The replacement process stops with point before "foo" in test-2.txt
with a message "Replaced 0 occurrences", without replacing "Foo" in
test.txt as it should have.

The bug appears both in emacs-27 as of
16fed05ba85c3d92d3c913657dd50a648ad3884a, and on master as of
771a6b68165b986b6bf9249c57ca11d310b0f0e4.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-28 14:35 bug#40940: 27.0.91; project-query-replace-regexp stops too early Simen Heggestøyl
@ 2020-04-29  3:54 ` Dmitry Gutov
  2020-04-29  7:35   ` Eli Zaretskii
  0 siblings, 1 reply; 38+ messages in thread
From: Dmitry Gutov @ 2020-04-29  3:54 UTC (permalink / raw)
  To: Simen Heggestøyl, 40940; +Cc: Stefan Monnier

On 28.04.2020 17:35, Simen Heggestøyl wrote:
> 
> Create a directory with two files in it: test-2.txt and test.txt:
> 
> test-dir
> ├── test-2.txt
> └── test.txt
> 
> Enter the text "Foo" into test.txt.
> Enter the text "foo" into test-2.txt.
> 
> Visit the directory or any of the files (the result is the same either
> way).
> 
> Type 'M-x project-query-replace-regexp', choosing to replace "Foo"
> with "Bar".
> 
> Confirm test-dir as the project directory in the next prompt.
> 
> The replacement process stops with point before "foo" in test-2.txt
> with a message "Replaced 0 occurrences", without replacing "Foo" in
> test.txt as it should have.
> 
> The bug appears both in emacs-27 as of
> 16fed05ba85c3d92d3c913657dd50a648ad3884a, and on master as of
> 771a6b68165b986b6bf9249c57ca11d310b0f0e4.

I can confirm this. Tested with the current emacs-27.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29  3:54 ` Dmitry Gutov
@ 2020-04-29  7:35   ` Eli Zaretskii
  2020-04-29  8:29     ` Simen Heggestøyl
  0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29  7:35 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, monnier, 40940

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 29 Apr 2020 06:54:22 +0300
> Cc: Stefan Monnier <monnier@IRO.UMontreal.CA>
> 
> > The replacement process stops with point before "foo" in test-2.txt
> > with a message "Replaced 0 occurrences", without replacing "Foo" in
> > test.txt as it should have.
> > 
> > The bug appears both in emacs-27 as of
> > 16fed05ba85c3d92d3c913657dd50a648ad3884a, and on master as of
> > 771a6b68165b986b6bf9249c57ca11d310b0f0e4.
> 
> I can confirm this. Tested with the current emacs-27.

Do we need to fix this in emacs-27?  Is this a regression since Emacs 26?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29  7:35   ` Eli Zaretskii
@ 2020-04-29  8:29     ` Simen Heggestøyl
  2020-04-29  8:37       ` Eli Zaretskii
  0 siblings, 1 reply; 38+ messages in thread
From: Simen Heggestøyl @ 2020-04-29  8:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, 40940, Dmitry Gutov

Eli Zaretskii <eliz@gnu.org> writes:

> Is this a regression since Emacs 26?

project-query-replace-regexp is new in Emacs 27.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29  8:29     ` Simen Heggestøyl
@ 2020-04-29  8:37       ` Eli Zaretskii
  2020-04-29  9:24         ` Eli Zaretskii
  0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29  8:37 UTC (permalink / raw)
  To: Simen Heggestøyl; +Cc: monnier, 40940, dgutov

> From: Simen Heggestøyl <simenheg@runbox.com>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  40940@debbugs.gnu.org,
>   monnier@IRO.UMontreal.CA
> Date: Wed, 29 Apr 2020 10:29:18 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Is this a regression since Emacs 26?
> 
> project-query-replace-regexp is new in Emacs 27.

OK, so we should fix this on the release branch.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29  8:37       ` Eli Zaretskii
@ 2020-04-29  9:24         ` Eli Zaretskii
  2020-04-29 10:41           ` Eli Zaretskii
  2020-04-29 14:50           ` Dmitry Gutov
  0 siblings, 2 replies; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29  9:24 UTC (permalink / raw)
  To: dgutov; +Cc: simenheg, monnier, 40940

> Date: Wed, 29 Apr 2020 11:37:53 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org, dgutov@yandex.ru
> 
> > From: Simen Heggestøyl <simenheg@runbox.com>
> > Cc: Dmitry Gutov <dgutov@yandex.ru>,  40940@debbugs.gnu.org,
> >   monnier@IRO.UMontreal.CA
> > Date: Wed, 29 Apr 2020 10:29:18 +0200
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > Is this a regression since Emacs 26?
> > 
> > project-query-replace-regexp is new in Emacs 27.
> 
> OK, so we should fix this on the release branch.

First, this is broken if the shell doesn't expand ~/ or if the Emacs
notion of the home directory is different from that of the shell.
Here's the proposed patch (ignoring the whitespace changes); OK to
push to the emacs-27 branch?

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 1f4cbe9..dbc967b 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -185,13 +185,17 @@ project--files-in-directory
   (require 'find-dired)
   (require 'xref)
   (defvar find-name-arg)
-  (let ((default-directory dir)
+  (let* ((default-directory dir)
+         (dirname (file-remote-p dir 'localname))
+         (dirname (or dirname
+                      ;; Make sure ~/ etc. in local directory name is
+                      ;; expanded and not left for the shell command
+                      ;; to interpret.
+                      (expand-file-name dir)))
          (command (format "%s %s %s -type f %s -print0"
                           find-program
-                         (file-local-name dir)
-                         (xref--find-ignores-arguments
-                          ignores
-                          (expand-file-name dir))
+                          dirname
+                          (xref--find-ignores-arguments ignores dirname)
                           (if files
                               (concat (shell-quote-argument "(")
                                       " " find-name-arg " "





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29  9:24         ` Eli Zaretskii
@ 2020-04-29 10:41           ` Eli Zaretskii
  2020-04-29 20:53             ` Dmitry Gutov
  2020-04-29 14:50           ` Dmitry Gutov
  1 sibling, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29 10:41 UTC (permalink / raw)
  To: dgutov; +Cc: simenheg, monnier, 40940

> Date: Wed, 29 Apr 2020 12:24:36 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> 
> First, this is broken if the shell doesn't expand ~/ or if the Emacs
> notion of the home directory is different from that of the shell.

The patch below seems to fix the rest.  Note that I've decided to make
the change in fileloop.el, since I think all the other callers need
the same fix.  Also, using downcase is not entirely correct, we should
use isearch-no-upper-case-p instead.

Comments?

diff --git a/lisp/fileloop.el b/lisp/fileloop.el
index 543963f..f7a199e 100644
--- a/lisp/fileloop.el
+++ b/lisp/fileloop.el
@@ -204,14 +204,24 @@ fileloop-initialize-replace
    files
    (lambda ()
      (let ((case-fold-search
-            (if (memql case-fold '(nil t)) case-fold case-fold-search)))
+            (if (memql case-fold '(nil t))
+                case-fold
+              (if (equal from (downcase from))
+                  case-fold-search
+                nil))))
        (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
+            (if (memql case-fold '(nil t))
+                case-fold
+              (if (equal from (downcase from))
+                  case-fold-search
+                nil))))
+       (perform-replace from to t t delimited nil multi-query-replace-map)))))
 
 (provide 'fileloop)
 ;;; fileloop.el ends here





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29  9:24         ` Eli Zaretskii
  2020-04-29 10:41           ` Eli Zaretskii
@ 2020-04-29 14:50           ` Dmitry Gutov
  2020-04-29 14:59             ` Eli Zaretskii
  1 sibling, 1 reply; 38+ messages in thread
From: Dmitry Gutov @ 2020-04-29 14:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940

On 29.04.2020 12:24, Eli Zaretskii wrote:
> First, this is broken if the shell doesn't expand ~/ or if the Emacs
> notion of the home directory is different from that of the shell.

The patch seems okay, but could you describe a case when the shell fails 
to expand '~/'? Is that about cmd.exe? Cygwin?

AFAICT the patch doesn't fix the same problem for remote hosts, is that 
a problem?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 14:50           ` Dmitry Gutov
@ 2020-04-29 14:59             ` Eli Zaretskii
  2020-04-29 15:42               ` Dmitry Gutov
  2020-04-29 15:49               ` Michael Albinus
  0 siblings, 2 replies; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29 14:59 UTC (permalink / raw)
  To: Dmitry Gutov, Michael Albinus; +Cc: simenheg, monnier, 40940

> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 29 Apr 2020 17:50:53 +0300
> 
> On 29.04.2020 12:24, Eli Zaretskii wrote:
> > First, this is broken if the shell doesn't expand ~/ or if the Emacs
> > notion of the home directory is different from that of the shell.
> 
> The patch seems okay, but could you describe a case when the shell fails 
> to expand '~/'? Is that about cmd.exe? Cygwin?

I've bumped into it with cmd.exe on MS-Windows, yes.

> AFAICT the patch doesn't fix the same problem for remote hosts, is that 
> a problem?

No, I don't think so.  A remote host cannot be a non-Posix system,
AFAIK.  Michael, can you confirm?  (And in any case, this issue should
be handled in the file-remote-p handler.)





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 14:59             ` Eli Zaretskii
@ 2020-04-29 15:42               ` Dmitry Gutov
  2020-04-29 16:04                 ` Eli Zaretskii
  2020-04-29 15:49               ` Michael Albinus
  1 sibling, 1 reply; 38+ messages in thread
From: Dmitry Gutov @ 2020-04-29 15:42 UTC (permalink / raw)
  To: Eli Zaretskii, Michael Albinus; +Cc: simenheg, monnier, 40940

On 29.04.2020 17:59, Eli Zaretskii wrote:
>> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Wed, 29 Apr 2020 17:50:53 +0300
>>
>> On 29.04.2020 12:24, Eli Zaretskii wrote:
>>> First, this is broken if the shell doesn't expand ~/ or if the Emacs
>>> notion of the home directory is different from that of the shell.
>>
>> The patch seems okay, but could you describe a case when the shell fails
>> to expand '~/'? Is that about cmd.exe? Cygwin?
> 
> I've bumped into it with cmd.exe on MS-Windows, yes.

I see. Then LGTM, thank you.

Could you clarify, though, does that mean that no project commands are 
currently working on Windows, without this patch? Or does that only 
affect the "transient" project type?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 14:59             ` Eli Zaretskii
  2020-04-29 15:42               ` Dmitry Gutov
@ 2020-04-29 15:49               ` Michael Albinus
  2020-04-29 15:58                 ` Dmitry Gutov
  2020-04-29 16:10                 ` Eli Zaretskii
  1 sibling, 2 replies; 38+ messages in thread
From: Michael Albinus @ 2020-04-29 15:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940, Dmitry Gutov

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> AFAICT the patch doesn't fix the same problem for remote hosts, is that
>> a problem?
>
> No, I don't think so.  A remote host cannot be a non-Posix system,
> AFAIK.

A remote host can be (kind of) non-Posix, for example not expanding
"~/". But this is a general problem then accessing such a host, and one
shall avoid to use a remote file name containing the tilde.

> Michael, can you confirm?  (And in any case, this issue should
> be handled in the file-remote-p handler.)

Tramp checks for all remote hosts whether they can expand "~/" (or
"~user/" in general). If possible, it does.

Your patch uses `expand-file-name', and that's all what we have to say
to Tramp.

Best regards, Michael.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 15:49               ` Michael Albinus
@ 2020-04-29 15:58                 ` Dmitry Gutov
  2020-04-29 16:10                 ` Eli Zaretskii
  1 sibling, 0 replies; 38+ messages in thread
From: Dmitry Gutov @ 2020-04-29 15:58 UTC (permalink / raw)
  To: Michael Albinus, Eli Zaretskii; +Cc: simenheg, monnier, 40940

On 29.04.2020 18:49, Michael Albinus wrote:
> Your patch uses `expand-file-name', and that's all what we have to say
> to Tramp.

I think it uses it only for non-Tramp dirs, doesn't it?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 15:42               ` Dmitry Gutov
@ 2020-04-29 16:04                 ` Eli Zaretskii
  2020-04-29 16:11                   ` Dmitry Gutov
  0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29 16:04 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, michael.albinus, monnier, 40940

> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 29 Apr 2020 18:42:41 +0300
> 
> >> The patch seems okay, but could you describe a case when the shell fails
> >> to expand '~/'? Is that about cmd.exe? Cygwin?
> > 
> > I've bumped into it with cmd.exe on MS-Windows, yes.
> 
> I see. Then LGTM, thank you.

Thanks, pushed to the emacs-27 branch.

> Could you clarify, though, does that mean that no project commands are 
> currently working on Windows, without this patch? Or does that only 
> affect the "transient" project type?

Only the "transient" projects, AFAICT.  At least I tried with a Git
repository under ~/, and got absolute file names in the list.  I hope
Mercurial does the same.  You can test it easily by running

   (project-files (project-current t))

in a suitable set up directory.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 15:49               ` Michael Albinus
  2020-04-29 15:58                 ` Dmitry Gutov
@ 2020-04-29 16:10                 ` Eli Zaretskii
  2020-04-29 16:22                   ` Michael Albinus
  1 sibling, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29 16:10 UTC (permalink / raw)
  To: Michael Albinus; +Cc: simenheg, monnier, 40940, dgutov

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  simenheg@runbox.com,
>   monnier@IRO.UMontreal.CA,  40940@debbugs.gnu.org
> Date: Wed, 29 Apr 2020 17:49:46 +0200
> 
> > Michael, can you confirm?  (And in any case, this issue should
> > be handled in the file-remote-p handler.)
> 
> Tramp checks for all remote hosts whether they can expand "~/" (or
> "~user/" in general). If possible, it does.
> 
> Your patch uses `expand-file-name', and that's all what we have to say
> to Tramp.

My patch only calls expand-file-name on local file names.  For remote
file names, I've left the result of the call to file-remote-p
unaltered.  Would that be a problem if the file name, after stripping
the remote protocol parts, started with "~/" ?  And if so, what should
be done here about these cases?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 16:04                 ` Eli Zaretskii
@ 2020-04-29 16:11                   ` Dmitry Gutov
  0 siblings, 0 replies; 38+ messages in thread
From: Dmitry Gutov @ 2020-04-29 16:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, michael.albinus, monnier, 40940

On 29.04.2020 19:04, Eli Zaretskii wrote:
>> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Wed, 29 Apr 2020 18:42:41 +0300
>>
>>>> The patch seems okay, but could you describe a case when the shell fails
>>>> to expand '~/'? Is that about cmd.exe? Cygwin?
>>>
>>> I've bumped into it with cmd.exe on MS-Windows, yes.
>>
>> I see. Then LGTM, thank you.
> 
> Thanks, pushed to the emacs-27 branch.

Thanks.

>> Could you clarify, though, does that mean that no project commands are
>> currently working on Windows, without this patch? Or does that only
>> affect the "transient" project type?
> 
> Only the "transient" projects, AFAICT.  At least I tried with a Git
> repository under ~/, and got absolute file names in the list.  I hope
> Mercurial does the same.  You can test it easily by running
> 
>     (project-files (project-current t))
> 
> in a suitable set up directory.

It does. Very good.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 16:10                 ` Eli Zaretskii
@ 2020-04-29 16:22                   ` Michael Albinus
  2020-04-29 16:44                     ` Eli Zaretskii
  0 siblings, 1 reply; 38+ messages in thread
From: Michael Albinus @ 2020-04-29 16:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940, dgutov

Eli Zaretskii <eliz@gnu.org> writes:

>> Tramp checks for all remote hosts whether they can expand "~/" (or
>> "~user/" in general). If possible, it does.
>>
>> Your patch uses `expand-file-name', and that's all what we have to say
>> to Tramp.
>
> My patch only calls expand-file-name on local file names.  For remote
> file names, I've left the result of the call to file-remote-p
> unaltered.  Would that be a problem if the file name, after stripping
> the remote protocol parts, started with "~/" ?  And if so, what should
> be done here about these cases?

Call `expand-file-name' on the remote file name, like
'(expand-file-name "/ssh:user@host:~/")'. Tramp expands it to
"/ssh:user@host:/home/user/", in case the "/home/user" is the home
directory of "user" on "host".

Best regards, Michael.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 16:22                   ` Michael Albinus
@ 2020-04-29 16:44                     ` Eli Zaretskii
  2020-04-29 18:20                       ` Dmitry Gutov
  0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29 16:44 UTC (permalink / raw)
  To: Michael Albinus; +Cc: simenheg, monnier, 40940, dgutov

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: dgutov@yandex.ru,  simenheg@runbox.com,  monnier@IRO.UMontreal.CA,
>   40940@debbugs.gnu.org
> Date: Wed, 29 Apr 2020 18:22:06 +0200
> 
> > My patch only calls expand-file-name on local file names.  For remote
> > file names, I've left the result of the call to file-remote-p
> > unaltered.  Would that be a problem if the file name, after stripping
> > the remote protocol parts, started with "~/" ?  And if so, what should
> > be done here about these cases?
> 
> Call `expand-file-name' on the remote file name, like
> '(expand-file-name "/ssh:user@host:~/")'. Tramp expands it to
> "/ssh:user@host:/home/user/", in case the "/home/user" is the home
> directory of "user" on "host".

Thanks.

Dmitry, this means my change should be reworked to call
expand-file-name before file-local-name, right?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 16:44                     ` Eli Zaretskii
@ 2020-04-29 18:20                       ` Dmitry Gutov
  2020-04-29 18:38                         ` Michael Albinus
  2020-04-29 18:44                         ` Eli Zaretskii
  0 siblings, 2 replies; 38+ messages in thread
From: Dmitry Gutov @ 2020-04-29 18:20 UTC (permalink / raw)
  To: Eli Zaretskii, Michael Albinus; +Cc: simenheg, monnier, 40940

On 29.04.2020 19:44, Eli Zaretskii wrote:
> Dmitry, this means my change should be reworked to call
> expand-file-name before file-local-name, right?

Seems so.

Something like this?

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index dbc967b885..f80b4328bc 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -185,17 +185,16 @@ project--files-in-directory
    (require 'find-dired)
    (require 'xref)
    (defvar find-name-arg)
-  (let* ((default-directory dir)
-         (dirname (file-remote-p dir 'localname))
-         (dirname (or dirname
-                      ;; Make sure ~/ etc. in local directory name is
-                      ;; expanded and not left for the shell command
-                      ;; to interpret.
-                      (expand-file-name dir)))
+  (let* ((dir
+          ;; Make sure ~/ etc. in local directory name is
+          ;; expanded and not left for the shell command
+          ;; to interpret.
+          (expand-file-name dir))
+         (default-directory dir)
           (command (format "%s %s %s -type f %s -print0"
                            find-program
-                          dirname
-                          (xref--find-ignores-arguments ignores dirname)
+                          dir
+                          (xref--find-ignores-arguments ignores dir)
                            (if files
                                (concat (shell-quote-argument "(")
                                        " " find-name-arg " "





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 18:20                       ` Dmitry Gutov
@ 2020-04-29 18:38                         ` Michael Albinus
  2020-04-29 19:09                           ` Dmitry Gutov
  2020-04-29 18:44                         ` Eli Zaretskii
  1 sibling, 1 reply; 38+ messages in thread
From: Michael Albinus @ 2020-04-29 18:38 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, monnier, 40940

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 29.04.2020 19:44, Eli Zaretskii wrote:
>> Dmitry, this means my change should be reworked to call
>> expand-file-name before file-local-name, right?
>
> Seems so.
>
> Something like this?
>
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index dbc967b885..f80b4328bc 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -185,17 +185,16 @@ project--files-in-directory
>    (require 'find-dired)
>    (require 'xref)
>    (defvar find-name-arg)
> -  (let* ((default-directory dir)
> -         (dirname (file-remote-p dir 'localname))
> -         (dirname (or dirname
> -                      ;; Make sure ~/ etc. in local directory name is
> -                      ;; expanded and not left for the shell command
> -                      ;; to interpret.
> -                      (expand-file-name dir)))
> +  (let* ((dir
> +          ;; Make sure ~/ etc. in local directory name is
> +          ;; expanded and not left for the shell command
> +          ;; to interpret.
> +          (expand-file-name dir))
> +         (default-directory dir)
>           (command (format "%s %s %s -type f %s -print0"
>                            find-program
> -                          dirname
> -                          (xref--find-ignores-arguments ignores dirname)
> +                          dir
> +                          (xref--find-ignores-arguments ignores dir)
>                            (if files
>                                (concat (shell-quote-argument "(")
>                                        " " find-name-arg " "

No. dir is a remote file name. So you must still declare dirname as
(file-local-name dir).

Best regards, Michael.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 18:20                       ` Dmitry Gutov
  2020-04-29 18:38                         ` Michael Albinus
@ 2020-04-29 18:44                         ` Eli Zaretskii
  2020-04-29 18:56                           ` Michael Albinus
  1 sibling, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29 18:44 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, michael.albinus, monnier, 40940

> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 29 Apr 2020 21:20:06 +0300
> 
> On 29.04.2020 19:44, Eli Zaretskii wrote:
> > Dmitry, this means my change should be reworked to call
> > expand-file-name before file-local-name, right?
> 
> Seems so.
> 
> Something like this?

Yes, but the original code called file-local-name, whereas this one
doesn't.  We had the call to file-local-name for a reason, didn't we?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 18:44                         ` Eli Zaretskii
@ 2020-04-29 18:56                           ` Michael Albinus
  0 siblings, 0 replies; 38+ messages in thread
From: Michael Albinus @ 2020-04-29 18:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940, Dmitry Gutov

Eli Zaretskii <eliz@gnu.org> writes:

> Yes, but the original code called file-local-name, whereas this one
> doesn't.  We had the call to file-local-name for a reason, didn't we?

Yes. The process shall always use the local file name in its arguments.

Best regards, Michael.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 18:38                         ` Michael Albinus
@ 2020-04-29 19:09                           ` Dmitry Gutov
  2020-04-29 19:15                             ` Michael Albinus
  2020-04-29 19:26                             ` Eli Zaretskii
  0 siblings, 2 replies; 38+ messages in thread
From: Dmitry Gutov @ 2020-04-29 19:09 UTC (permalink / raw)
  To: Michael Albinus; +Cc: simenheg, monnier, 40940

On 29.04.2020 21:38, Michael Albinus wrote:
> No. dir is a remote file name. So you must still declare dirname as
> (file-local-name dir).

Right, thank you. I also see no reason not to pass the local name to 
xref--find-ignores-arguments (xref-matches-in-directory ends up doing 
that already).

So the patch will be:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index dbc967b885..f5f4092bab 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -186,16 +186,14 @@ project--files-in-directory
    (require 'xref)
    (defvar find-name-arg)
    (let* ((default-directory dir)
-         (dirname (file-remote-p dir 'localname))
-         (dirname (or dirname
-                      ;; Make sure ~/ etc. in local directory name is
-                      ;; expanded and not left for the shell command
-                      ;; to interpret.
-                      (expand-file-name dir)))
+         ;; Make sure ~/ etc. in local directory name is
+         ;; expanded and not left for the shell command
+         ;; to interpret.
+         (localdir (file-local-name (expand-file-name dir)))
           (command (format "%s %s %s -type f %s -print0"
                            find-program
-                          dirname
-                          (xref--find-ignores-arguments ignores dirname)
+                          localdir
+                          (xref--find-ignores-arguments ignores localdir)
                            (if files
                                (concat (shell-quote-argument "(")
                                        " " find-name-arg " "





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 19:09                           ` Dmitry Gutov
@ 2020-04-29 19:15                             ` Michael Albinus
  2020-04-29 19:26                             ` Eli Zaretskii
  1 sibling, 0 replies; 38+ messages in thread
From: Michael Albinus @ 2020-04-29 19:15 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, monnier, 40940

Dmitry Gutov <dgutov@yandex.ru> writes:

> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index dbc967b885..f5f4092bab 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -186,16 +186,14 @@ project--files-in-directory
>    (require 'xref)
>    (defvar find-name-arg)
>    (let* ((default-directory dir)
> -         (dirname (file-remote-p dir 'localname))
> -         (dirname (or dirname
> -                      ;; Make sure ~/ etc. in local directory name is
> -                      ;; expanded and not left for the shell command
> -                      ;; to interpret.
> -                      (expand-file-name dir)))
> +         ;; Make sure ~/ etc. in local directory name is
> +         ;; expanded and not left for the shell command
> +         ;; to interpret.
> +         (localdir (file-local-name (expand-file-name dir)))
>           (command (format "%s %s %s -type f %s -print0"
>                            find-program
> -                          dirname
> -                          (xref--find-ignores-arguments ignores dirname)
> +                          localdir
> +                          (xref--find-ignores-arguments ignores localdir)
>                            (if files
>                                (concat (shell-quote-argument "(")
>                                        " " find-name-arg " "

LGTM (I haven't tested, 'tho).

Best regards, Michael.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 19:09                           ` Dmitry Gutov
  2020-04-29 19:15                             ` Michael Albinus
@ 2020-04-29 19:26                             ` Eli Zaretskii
  1 sibling, 0 replies; 38+ messages in thread
From: Eli Zaretskii @ 2020-04-29 19:26 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, michael.albinus, monnier, 40940

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 29 Apr 2020 22:09:12 +0300
> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> 
> On 29.04.2020 21:38, Michael Albinus wrote:
> > No. dir is a remote file name. So you must still declare dirname as
> > (file-local-name dir).
> 
> Right, thank you. I also see no reason not to pass the local name to 
> xref--find-ignores-arguments (xref-matches-in-directory ends up doing 
> that already).
> 
> So the patch will be:

Yes, I think so.

Thanks.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 10:41           ` Eli Zaretskii
@ 2020-04-29 20:53             ` Dmitry Gutov
  2020-04-30 20:16               ` Juri Linkov
  2020-05-01  6:57               ` Eli Zaretskii
  0 siblings, 2 replies; 38+ messages in thread
From: Dmitry Gutov @ 2020-04-29 20:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940

On 29.04.2020 13:41, Eli Zaretskii wrote:
> The patch below seems to fix the rest.  Note that I've decided to make
> the change in fileloop.el, since I think all the other callers need
> the same fix.

Naturally.

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

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

  (provide 'fileloop)
  ;;; fileloop.el ends here





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 20:53             ` Dmitry Gutov
@ 2020-04-30 20:16               ` Juri Linkov
  2020-05-01  6:00                 ` Eli Zaretskii
  2020-05-01  6:57               ` Eli Zaretskii
  1 sibling, 1 reply; 38+ messages in thread
From: Juri Linkov @ 2020-04-30 20:16 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, monnier, 40940

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

Recently we changed hi-lock.el to follow the same logic as
isearch-no-upper-case-p, so this will make the behavior
consistent across different packages.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-30 20:16               ` Juri Linkov
@ 2020-05-01  6:00                 ` Eli Zaretskii
  0 siblings, 0 replies; 38+ messages in thread
From: Eli Zaretskii @ 2020-05-01  6:00 UTC (permalink / raw)
  To: Juri Linkov; +Cc: simenheg, monnier, 40940, dgutov

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  simenheg@runbox.com,
>   monnier@IRO.UMontreal.CA,  40940@debbugs.gnu.org
> Date: Thu, 30 Apr 2020 23:16:04 +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.
> 
> Recently we changed hi-lock.el to follow the same logic as
> isearch-no-upper-case-p, so this will make the behavior
> consistent across different packages.

Btw, looking at the other callers of fileloop-initialize-search, don't
get their logic backwards?  If the string typed by the user does NOT
include upper-case character, that means the search SHOULD obey the
default value of case-fold-search.  Or am I missing something?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-04-29 20:53             ` Dmitry Gutov
  2020-04-30 20:16               ` Juri Linkov
@ 2020-05-01  6:57               ` Eli Zaretskii
  2020-05-01 15:27                 ` Dmitry Gutov
  1 sibling, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-05-01  6:57 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, monnier, 40940

> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@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.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-01  6:57               ` Eli Zaretskii
@ 2020-05-01 15:27                 ` Dmitry Gutov
  2020-05-01 15:45                   ` Eli Zaretskii
  0 siblings, 1 reply; 38+ messages in thread
From: Dmitry Gutov @ 2020-05-01 15:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940

On 01.05.2020 09:57, Eli Zaretskii wrote:
> 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?

Sorry if that was unclear: I think we'd need to update the docstring of 
fileloop-initialize-replace. Which doesn't offer any hints that the 
logic of isearch-no-upper-case-p will be employed.

Also see the part about obeying the value of search-upper-case.

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

Since we bind search-upper-case to nil in this patch, perform-replace 
won't try to alter the value of case-fold-search internally.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-01 15:27                 ` Dmitry Gutov
@ 2020-05-01 15:45                   ` Eli Zaretskii
  2020-05-01 23:21                     ` Dmitry Gutov
  0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-05-01 15:45 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, monnier, 40940

> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 1 May 2020 18:27:45 +0300
> 
> On 01.05.2020 09:57, Eli Zaretskii wrote:
> > 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?
> 
> Sorry if that was unclear: I think we'd need to update the docstring of 
> fileloop-initialize-replace. Which doesn't offer any hints that the 
> logic of isearch-no-upper-case-p will be employed.

Ah, okay.  Agreed.

> > 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.
> 
> Since we bind search-upper-case to nil in this patch, perform-replace 
> won't try to alter the value of case-fold-search internally.

But that's contrary to how query-replace works, isn't it?





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-01 15:45                   ` Eli Zaretskii
@ 2020-05-01 23:21                     ` Dmitry Gutov
  2020-05-02  6:44                       ` Eli Zaretskii
  0 siblings, 1 reply; 38+ messages in thread
From: Dmitry Gutov @ 2020-05-01 23:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940

On 01.05.2020 18:45, Eli Zaretskii wrote:
>> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Fri, 1 May 2020 18:27:45 +0300
>>
>> On 01.05.2020 09:57, Eli Zaretskii wrote:
>>> 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?
>>
>> Sorry if that was unclear: I think we'd need to update the docstring of
>> fileloop-initialize-replace. Which doesn't offer any hints that the
>> logic of isearch-no-upper-case-p will be employed.
> 
> Ah, okay.  Agreed.

Should I leave that to you? At least the "updated docstring" part.

>>> 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.
>>
>> Since we bind search-upper-case to nil in this patch, perform-replace
>> won't try to alter the value of case-fold-search internally.
> 
> But that's contrary to how query-replace works, isn't it?

I suppose. But query-replace documents that aspects of its behavior:

   Matching is independent of case if `case-fold-search' is non-nil and
   FROM-STRING has no uppercase letters.

Though it doesn't mention the search-upper-case variable.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-01 23:21                     ` Dmitry Gutov
@ 2020-05-02  6:44                       ` Eli Zaretskii
  2020-05-02  7:56                         ` Eli Zaretskii
  2020-05-03  1:43                         ` Dmitry Gutov
  0 siblings, 2 replies; 38+ messages in thread
From: Eli Zaretskii @ 2020-05-02  6:44 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, monnier, 40940

> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 2 May 2020 02:21:57 +0300
> 
> >> Sorry if that was unclear: I think we'd need to update the docstring of
> >> fileloop-initialize-replace. Which doesn't offer any hints that the
> >> logic of isearch-no-upper-case-p will be employed.
> > 
> > Ah, okay.  Agreed.
> 
> Should I leave that to you? At least the "updated docstring" part.

I can update the doc string, yes.  But I don't think it's a good idea
to divide the coding part between us, though.  So please show the
code, and I will suggest the doc change.

> >> Since we bind search-upper-case to nil in this patch, perform-replace
> >> won't try to alter the value of case-fold-search internally.
> > 
> > But that's contrary to how query-replace works, isn't it?
> 
> I suppose. But query-replace documents that aspects of its behavior:
> 
>    Matching is independent of case if `case-fold-search' is non-nil and
>    FROM-STRING has no uppercase letters.

Sure.  Wouldn't users of project-query-replace-regexp expect the same?


> Though it doesn't mention the search-upper-case variable.

Ugh!  That should be fixed, of course.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-02  6:44                       ` Eli Zaretskii
@ 2020-05-02  7:56                         ` Eli Zaretskii
  2020-05-03  1:43                         ` Dmitry Gutov
  1 sibling, 0 replies; 38+ messages in thread
From: Eli Zaretskii @ 2020-05-02  7:56 UTC (permalink / raw)
  To: dgutov; +Cc: simenheg, monnier, 40940

> Date: Sat, 02 May 2020 09:44:30 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> 
> > Though it doesn't mention the search-upper-case variable.
> 
> Ugh!  That should be fixed, of course.

Now done on the emacs-27 branch.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-02  6:44                       ` Eli Zaretskii
  2020-05-02  7:56                         ` Eli Zaretskii
@ 2020-05-03  1:43                         ` Dmitry Gutov
  2020-05-03  6:54                           ` Simen Heggestøyl
  2020-05-03 17:10                           ` Eli Zaretskii
  1 sibling, 2 replies; 38+ messages in thread
From: Dmitry Gutov @ 2020-05-03  1:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940

On 02.05.2020 09:44, Eli Zaretskii wrote:
> I can update the doc string, yes.  But I don't think it's a good idea
> to divide the coding part between us, though.  So please show the
> code, and I will suggest the doc change.

Here's the comprehensive fix I'd like to see:

diff --git a/lisp/fileloop.el b/lisp/fileloop.el
index 543963feaf..69825fdcae 100644
--- a/lisp/fileloop.el
+++ b/lisp/fileloop.el
@@ -181,8 +181,7 @@ fileloop-initialize-search
      (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 regexp case-fold)))
           (re-search-forward regexp nil t)))
       (lambda ()
         (unless (eq last-buffer (current-buffer))
@@ -190,6 +189,15 @@ fileloop-initialize-search
           (message "Scanning file %s...found" buffer-file-name))
         nil))))

+(defun fileloop--case-fold (regexp case-fold)
+  (let ((value
+         (if (memql case-fold '(nil t))
+             case-fold
+           case-fold-search)))
+    (if (and value search-upper-case)
+        (isearch-no-upper-case-p regexp t)
+      value)))
+
  ;;;###autoload
  (defun fileloop-initialize-replace (from to files case-fold &optional 
delimited)
    "Initialize a new round of query&replace on several files.
@@ -203,15 +211,15 @@ 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 from 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 from case-fold)))
+       (perform-replace from to t t delimited nil 
multi-query-replace-map)))))

  (provide 'fileloop)
  ;;; fileloop.el ends here





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-03  1:43                         ` Dmitry Gutov
@ 2020-05-03  6:54                           ` Simen Heggestøyl
  2020-05-03 17:10                           ` Eli Zaretskii
  1 sibling, 0 replies; 38+ messages in thread
From: Simen Heggestøyl @ 2020-05-03  6:54 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: monnier, 40940

Dmitry Gutov <dgutov@yandex.ru> writes:

> Here's the comprehensive fix I'd like to see:

This works as expected in my testing.

> +         (if (memql case-fold '(nil t))

AKA (booleanp case-fold) if you'd like.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-03  1:43                         ` Dmitry Gutov
  2020-05-03  6:54                           ` Simen Heggestøyl
@ 2020-05-03 17:10                           ` Eli Zaretskii
  2020-05-03 17:28                             ` Dmitry Gutov
  1 sibling, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-05-03 17:10 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: simenheg, monnier, 40940

> Cc: simenheg@runbox.com, monnier@IRO.UMontreal.CA, 40940@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 3 May 2020 04:43:26 +0300
> 
> +(defun fileloop--case-fold (regexp case-fold)
> +  (let ((value
> +         (if (memql case-fold '(nil t))
> +             case-fold
> +           case-fold-search)))
> +    (if (and value search-upper-case)
> +        (isearch-no-upper-case-p regexp t)
> +      value)))

LGTM, thanks.  But don't you need to require isearch to get
isearch-no-upper-case-p? or to autoload it?

Here's the doc string I promised:

  (defun fileloop-initialize-replace (from to files case-fold &optional delimited)
    "Initialize a new round of query&replace on several files.
  FROM is a regexp and TO is the replacement to use.
  FILES describes the files, as in `fileloop-initialize'.
  CASE-FOLD can be t, nil, or `default':
    if it is nil, matching of FROM is case-sensitive.
    if it is t, matching of FROM is case-insensitive, except
       when `search-upper-case' is non-nil and FROM includes
       upper-case letters.
    if it is `default', the function uses the value of
       `case-fold-search' instead.
  DELIMITED if non-nil means replace only word-delimited matches."





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-03 17:10                           ` Eli Zaretskii
@ 2020-05-03 17:28                             ` Dmitry Gutov
  2020-05-03 23:58                               ` Dmitry Gutov
  0 siblings, 1 reply; 38+ messages in thread
From: Dmitry Gutov @ 2020-05-03 17:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940

On 03.05.2020 20:10, Eli Zaretskii wrote:
> LGTM, thanks.  But don't you need to require isearch to get
> isearch-no-upper-case-p? or to autoload it?

isearch is in loadup.el, isn't it?

 > Here's the doc string I promised:

Looks good, thanks.





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

* bug#40940: 27.0.91; project-query-replace-regexp stops too early
  2020-05-03 17:28                             ` Dmitry Gutov
@ 2020-05-03 23:58                               ` Dmitry Gutov
  0 siblings, 0 replies; 38+ messages in thread
From: Dmitry Gutov @ 2020-05-03 23:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: simenheg, monnier, 40940-done

On 03.05.2020 20:28, Dmitry Gutov wrote:
>  > Here's the doc string I promised:
> 
> Looks good, thanks.

Patch installed, thanks to all.





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

end of thread, other threads:[~2020-05-03 23:58 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-28 14:35 bug#40940: 27.0.91; project-query-replace-regexp stops too early Simen Heggestøyl
2020-04-29  3:54 ` Dmitry Gutov
2020-04-29  7:35   ` Eli Zaretskii
2020-04-29  8:29     ` Simen Heggestøyl
2020-04-29  8:37       ` Eli Zaretskii
2020-04-29  9:24         ` Eli Zaretskii
2020-04-29 10:41           ` Eli Zaretskii
2020-04-29 20:53             ` Dmitry Gutov
2020-04-30 20:16               ` Juri Linkov
2020-05-01  6:00                 ` Eli Zaretskii
2020-05-01  6:57               ` Eli Zaretskii
2020-05-01 15:27                 ` Dmitry Gutov
2020-05-01 15:45                   ` Eli Zaretskii
2020-05-01 23:21                     ` Dmitry Gutov
2020-05-02  6:44                       ` Eli Zaretskii
2020-05-02  7:56                         ` Eli Zaretskii
2020-05-03  1:43                         ` Dmitry Gutov
2020-05-03  6:54                           ` Simen Heggestøyl
2020-05-03 17:10                           ` Eli Zaretskii
2020-05-03 17:28                             ` Dmitry Gutov
2020-05-03 23:58                               ` Dmitry Gutov
2020-04-29 14:50           ` Dmitry Gutov
2020-04-29 14:59             ` Eli Zaretskii
2020-04-29 15:42               ` Dmitry Gutov
2020-04-29 16:04                 ` Eli Zaretskii
2020-04-29 16:11                   ` Dmitry Gutov
2020-04-29 15:49               ` Michael Albinus
2020-04-29 15:58                 ` Dmitry Gutov
2020-04-29 16:10                 ` Eli Zaretskii
2020-04-29 16:22                   ` Michael Albinus
2020-04-29 16:44                     ` Eli Zaretskii
2020-04-29 18:20                       ` Dmitry Gutov
2020-04-29 18:38                         ` Michael Albinus
2020-04-29 19:09                           ` Dmitry Gutov
2020-04-29 19:15                             ` Michael Albinus
2020-04-29 19:26                             ` Eli Zaretskii
2020-04-29 18:44                         ` Eli Zaretskii
2020-04-29 18:56                           ` Michael Albinus

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.