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

[Dmitry Gutov <dmitry@gutov.dev>]

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

Hi Jim,

On 29/10/2023 07:36, Jim Porter wrote:
> 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.

I kept this unfortunate situation around because every obvious fix 
brought non-negligible performance regressions: the version with 
file-regular-p slowed one of my examples (Mozilla's repo) by 370%. Your 
cl-set-difference version slowed it down by 10-14% -- better, but still 
something that seemed worse on balance when I tried this before, so I 
preferred to work around it in command implementations: the "-s" or 
"--no-messages" flags xref-search-program-alist.

And that's not to mention usage over Tramp (which would be affected by 
the +1 process call that you mentioned as well, but that seems unavoidable).

Anyway, after recent experience micro-optimizing list operations, I came 
up with this version where the impact seems minimal.

WDYT?

[-- Attachment #2: project-no-submodules.diff --]
[-- Type: text/x-patch, Size: 1834 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index fda1081eb62..e3e7654e687 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,24 @@ 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)))
+              (let* ((files
+                      (split-string
+                       (apply #'vc-git--run-command-string nil "ls-files" args)
+                       "\0" t))
+                     ref)
+                (when submodules
+                  ;; Hand-optimized version of nset-difference.
+                  (while (member (car files) submodules)
+                    (setq files (cdr files)))
+                  (setq ref files)
+                  (while ref
+                    (if (member (cadr ref) submodules)
+                        (setcdr ref (cddr ref))
+                      (setq ref (cdr ref)))))
+                files)))
        (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)