From: mgmarlow <graham@mgmarlow.com>

Great suggestions! I'm wondering whether we can completely normalize the
use of directory-files-recursively between the two functions. Is there a
reason for them to be different? In my head, I'd imagine the file search
itself is identical--all that needs to change is the use of seq-remove
depending on whether we want to return files or subdirectories.

I brought in your proposed changes and hoisted the predicate function
so it can be shared among both denote-directory-files and
denote-directory-subdirectories.

---
 denote.el | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/denote.el b/denote.el
index d662f29..488c6b1 100644
--- a/denote.el
+++ b/denote.el
@@ -711,6 +711,28 @@ FILE must be an absolute path."
   (string-prefix-p (denote-directory)
                    (expand-file-name default-directory)))
 
+(defun denote--exclude-directory-regexp-p (file)
+  "Return non-nil if FILE matches `denote-excluded-directories-regexp'."
+  (and denote-excluded-directories-regexp
+       (string-match-p denote-excluded-directories-regexp file)))
+
+(defun denote--directory-all-files-recursively ()
+  "Return list of all files in variable `denote-directory'.
+Avoids traversing dotfiles (unconditionally) and whatever matches
+`denote-excluded-directories-regexp'."
+  (directory-files-recursively
+     (denote-directory)
+     directory-files-no-dot-files-regexp
+     :include-directories
+     (lambda (f)
+       (cond
+        ((string-match-p "\\`\\." f) nil)
+        ((string-match-p "/\\." f) nil)
+        ((denote--exclude-directory-regexp-p f) nil)
+        ((file-readable-p f))
+        (t)))
+     :follow-symlinks))
+
 (defun denote-directory-files ()
   "Return list of absolute file paths in variable `denote-directory'.
 
@@ -726,17 +748,7 @@ value, as explained in its doc string."
    (seq-remove
     (lambda (f)
       (not (denote-file-has-identifier-p f)))
-    (directory-files-recursively
-     (denote-directory)
-     directory-files-no-dot-files-regexp
-     :include-directories
-     (lambda (f)
-       (cond
-        ((when-let ((regexp denote-excluded-directories-regexp))
-           (not (string-match-p regexp f))))
-        ((file-readable-p f))
-        (t)))
-     :follow-symlinks))))
+    (denote--directory-all-files-recursively))))
 
 (defun denote-directory-text-only-files ()
   "Return list of text files in variable `denote-directory'.
@@ -750,7 +762,7 @@ Filter `denote-directory-files' using `denote-file-is-note-p'."
 
 (defun denote-directory-subdirectories ()
   "Return list of subdirectories in variable `denote-directory'.
-Omit dotfiles (such as .git) unconditionally.  Also exclude
+Omit dotfiles (such as .git) unconditionally. Also exclude
 whatever matches `denote-excluded-directories-regexp'."
   (seq-remove
    (lambda (filename)
@@ -758,9 +770,8 @@ whatever matches `denote-excluded-directories-regexp'."
        (or (not (file-directory-p filename))
            (string-match-p "\\`\\." rel)
            (string-match-p "/\\." rel)
-           (when-let ((regexp denote-excluded-directories-regexp))
-             (string-match-p regexp rel)))))
-   (directory-files-recursively (denote-directory) ".*" t t)))
+           (denote--exclude-directory-regexp-p rel))))
+   (denote--directory-all-files-recursively)))
 
 (define-obsolete-function-alias
   'denote--subdirs
-- 
2.34.1

> From: Graham Marlow <graham@mgmarlow.com>
> Date: Fri, 14 Apr 2023 13:06:16 -0700

> [... 14 lines elided]

>  denote.el | 41 ++++++++++++++++++++++++++---------------
>  1 file changed, 26 insertions(+), 15 deletions(-)

> [... 74 lines elided]

Thank you!  Perhaps Wade Mealing can try this patch to check if it works
as expected in that setup?

-- 
Protesilaos Stavrou
https://protesilaos.com

> Thank you!  Perhaps Wade Mealing can try this patch to check if it works
> as expected in that setup?

For myself I was able to reproduce the problem in my local testing by 
cloning one or two node repositories into my denote directory and 
invoking denote-directory-subdirectories.

|cd ~/denote/source|
|npx create-react-app my-app1|
|npx create-react-app my-app2|

||

|| Before the call to denote-directory-subdirectories takes 5-8 seconds. 
After applying the changes in this patch it's instantaneous. Likewise 
with denote-subdirectory.

Apologies, I had replied in html mode and didnt notice.

This significantly improves the performance, from an 8-10 second (with
warm cache), to near instantaneous.

I'm very happy with that.

On Wed, Apr 19, 2023 at 3:04 PM Graham Marlow <graham@mgmarlow.com> wrote:
>
> > Thank you!  Perhaps Wade Mealing can try this patch to check if it works
> > as expected in that setup?
>
> For myself I was able to reproduce the problem in my local testing by
> cloning one or two node repositories into my denote directory and
> invoking denote-directory-subdirectories.
>
> |cd ~/denote/source|
> |npx create-react-app my-app1|
> |npx create-react-app my-app2|
>
> ||
>
> || Before the call to denote-directory-subdirectories takes 5-8 seconds.
> After applying the changes in this patch it's instantaneous. Likewise
> with denote-subdirectory.
>


-- 
Wade Mealing

> From: Wade Mealing <wmealing@gmail.com>
> Date: Wed, 19 Apr 2023 17:41:34 +1000
>
> Apologies, I had replied in html mode and didnt notice.
>
> This significantly improves the performance, from an 8-10 second (with
> warm cache), to near instantaneous.
>
> I'm very happy with that.

Very well!  I installed the patch and pushed the changes.  Thank you
both for the feedback!

-- 
Protesilaos Stavrou
https://protesilaos.com