Implement predicate on directory-files-recursively for faster searching v1 APPLIED

mgmarlow: 1
 Implement predicate on directory-files-recursively for faster searching

 1 files changed, 10 insertions(+), 1 deletions(-)

I applied your changes in a new patchset here: 
https://lists.sr.ht/~protesilaos/denote/%3C20230414200616.5432-1-graham%40mgmarlow.com%3E. 
Again, apologies for the multiple threads--I'm still getting used to 
using git send-email and don't mean to be creating new threads constantly!

BTW your observations about the use of predicate is spot-on, it 
unfortunately doesn't filter out the directories from the first search, 
requiring us to manually remove the same dotfiles and 
excluded-directories with remove-seq.

[PATCH] Implement predicate on directory-files-recursively for faster searching Export this patch

---
 denote.el | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/denote.el b/denote.el
index d662f29..69d0b2d 100644
--- a/denote.el
+++ b/denote.el
@@ -760,7 +760,16 @@ whatever matches `denote-excluded-directories-regexp'."
            (string-match-p "/\\." rel)
            (when-let ((regexp denote-excluded-directories-regexp))
              (string-match-p regexp rel)))))
-   (directory-files-recursively (denote-directory) ".*" t t)))
+   (directory-files-recursively
+    (denote-directory)
+    directory-files-no-dot-files-regexp
+    t
+    (lambda (f)
+      (cond
+        ((when-let ((regexp denote-excluded-directories-regexp))
+           (not (string-match-p regexp f))))
+        ((file-readable-p f))
+        (t))))))
 
 (define-obsolete-function-alias
   'denote--subdirs
-- 
2.37.1 (Apple Git-137.1)

Protesilaos Stavrou <info@protesilaos.com>

1 year, 9 months ago

Thank you Graham!  I am CCing Wade Mealing from the other thread.

Adding a predicate function makes sense.  What caught my attention is
that 'directory-files-recursively' does not exclude ".git" even when we
do something like this:

    (directory-files-recursively
     (denote-directory)
     directory-files-no-dot-files-regexp
     :include-directories
     (lambda (f)
       (not (string-match-p "/\\.git" f))))

I am testing that match with this:

    (string-match-p "/\\.git" "/home/prot/Documents/notes/.git")

Its works as expected.  But the 'directory-files-recursively' always
lists the ".git" directory in its return value.

This makes me think that the predicate only affects the "descending
into" part of the recursive file listing.  Any subdirectories in the
root directory will still be listed.  I tested this hypothesis by adding
a "hello" directory to the root of my 'denote-directory' as well as
inside another subdirectory.  Then I did this:

    (setq denote-excluded-directories-regexp "hello")

    (seq-filter
     (lambda (f)
       (string-match-p "hello" f))
     (directory-files-recursively
      (denote-directory)
      directory-files-no-dot-files-regexp
      :include-directories
      (lambda (f)
        (and denote-excluded-directories-regexp
             (string-match-p denote-excluded-directories-regexp f)))))

The above returns the "hello" directory at the root, but not deeper.

With those granted, I rewrote the code this way:

    (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-subdirectories ()
      "Return list of subdirectories in variable `denote-directory'.
    Omit dotfiles (such as .git) unconditionally.  Also exclude
    whatever matches `denote-excluded-directories-regexp'."
      (seq-filter
       (lambda (f)
         (and (file-directory-p f)
              (or (string-match-p "\\`\\." f)
                  (not (string-match-p "/\\." f))
                  (denote--exclude-directory-regexp-p f))))
       (directory-files-recursively
        (denote-directory)
        directory-files-no-dot-files-regexp
        :include-directories
        (lambda (f)
          (or (not (string-match-p "\\`\\." f))
              (not (string-match-p "/\\." f))
              (not (denote--exclude-directory-regexp-p f)))))))

Can you test it?  What do you think?  Can we improve it?

All the best,
Protesilaos (or simply "Prot")