bug#66806: 30.0.50; [PATCH] 'project-find-regexp' passes Git submodules to the search program

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

X-Debbugs-Cc: dmitry@gutov.dev

To see this in action, you can do the following, starting from "emacs 
-Q" inside of a Git repo that contains submodules:

   M-x trace-function RET project-files RET
   C-x p g some-string RET

If you look at the trace, you'll see that the files in your submodules 
are returned from 'project-files', but so is the submodule directory. 
That's not really correct, since 'project-files' is supposed to return 
*files*, not directories. (There are already workarounds for this in 
'project-search' and 'project-query-replace-regexp'.)

By default, this is just a theoretical problem, but if you customize 
'xref-search-program-alist' and 'xref-search-program' to include some 
other program, this can cause real issues. For example, I tried to add 
"ag" to this list[1], and unfortunately, it just doesn't work in this 
case. The results for submodules get duplicated, and there's no way I 
can see with ag to search only the specified *files*, ignoring any 
directories. (Looking at the definition for ripgrep, I'm guessing the 
"-g '!*/'" is the trick for that program, but nothing similar works for ag.)

Attached are two possible patches for this: a minimal version that just 
fixes 'project-find-regexp', and a maximal version that fixes this in 
general, and should theoretically speed up 'project-search' and 
'project-query-replace-regexp' since they no longer need to call 
'file-regular-p' on every file.

Do either of these patches look ok?

[1] This is a long story, simplified for this message since the gory 
details aren't especially relevant.

[-- Attachment #2: minimal_0001-Exclude-Git-submodules-when-getting-list-of-files-fo.patch --]
[-- Type: text/plain, Size: 1093 bytes --]

From 9f9fc6a606bdd1610d440d9b130aaa30aa66597a Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 28 Oct 2023 22:11:30 -0700
Subject: [PATCH] Exclude Git submodules when getting list of files for
 'project-find-regexp'

* lisp/progmodes/project.el (project-find-regexp): Filter out
non-regular files.
---
 lisp/progmodes/project.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index fda1081eb62..e245e794318 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -960,7 +960,8 @@ project-find-regexp
          (default-directory (project-root pr))
          (files
           (if (not current-prefix-arg)
-              (project-files pr)
+              ;; XXX: See the comment in project-query-replace-regexp.
+              (cl-delete-if-not #'file-regular-p (project-files pr))
             (let ((dir (read-directory-name "Base directory: "
                                             caller-dir nil t)))
               (project--files-in-directory dir
-- 
2.25.1


[-- Attachment #3: maximal_0001-Exclude-Git-submodules-from-project-files.patch --]
[-- Type: text/plain, Size: 2684 bytes --]

From fdb7a99dcd47e3a11057fb47221a5e6f40b2db6a Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 28 Oct 2023 22:20:41 -0700
Subject: [PATCH] Exclude Git submodules from 'project-files'

* lisp/progmodes/project.el (project--vc-list-files): Exclude Git
submodules.
(project-search, project-query-replace-regexp): Remove now-unneeded
workaround.
---
 lisp/progmodes/project.el | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index fda1081eb62..92b3ed52600 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -647,6 +647,7 @@ project--vc-list-files
             (include-untracked (project--value-in-dir
                                 'project-vc-include-untracked
                                 dir))
+            (submodules (project--git-submodules))
             files)
        (setq args (append args
                           '("-c" "--exclude-standard")
@@ -680,13 +681,14 @@ project--vc-list-files
        (setq files
              (mapcar
               (lambda (file) (concat default-directory file))
-              (split-string
-               (apply #'vc-git--run-command-string nil "ls-files" args)
-               "\0" t)))
+              (cl-set-difference
+               (split-string
+                (apply #'vc-git--run-command-string nil "ls-files" args)
+                "\0" t)
+               submodules :test #'string=)))
        (when (project--vc-merge-submodules-p default-directory)
          ;; Unfortunately, 'ls-files --recurse-submodules' conflicts with '-o'.
-         (let* ((submodules (project--git-submodules))
-                (sub-files
+         (let ((sub-files
                  (mapcar
                   (lambda (module)
                     (when (file-directory-p module)
@@ -1326,8 +1328,7 @@ project-search
   (interactive "sSearch (regexp): ")
   (fileloop-initialize-search
    regexp
-   ;; XXX: See the comment in project-query-replace-regexp.
-   (cl-delete-if-not #'file-regular-p (project-files (project-current t)))
+   (project-files (project-current t))
    'default)
   (fileloop-continue))
 
@@ -1348,10 +1349,7 @@ project-query-replace-regexp
        (list from to))))
   (fileloop-initialize-replace
    from to
-   ;; XXX: Filter out Git submodules, which are not regular files.
-   ;; `project-files' can return those, which is arguably suboptimal,
-   ;; but removing them eagerly has performance cost.
-   (cl-delete-if-not #'file-regular-p (project-files (project-current t)))
+   (project-files (project-current t))
    'default)
   (fileloop-continue))
 
-- 
2.25.1