~protesilaos/denote

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

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

Details
Message ID
<20230414000311.1981-1-graham@mgmarlow.com>
DKIM signature
missing
Download raw message
Patch: +10 -1
---
 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)
Details
Message ID
<87edonhvy0.fsf@protesilaos.com>
In-Reply-To
<20230414000311.1981-1-graham@mgmarlow.com> (view parent)
DKIM signature
missing
Download raw message
> From: mgmarlow <graham@mgmarlow.com>
> Date: Thu, 13 Apr 2023 17:03:11 -0700
>
> ---
>  denote.el | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

> [... 23 lines elided]

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

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<76ed9fe2-d597-f7b9-5e59-717aeb77c3c3@mgmarlow.com>
In-Reply-To
<87edonhvy0.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
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.
Reply to thread Export thread (mbox)