Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow checking for subdirs of `denote-dired-directories' (fix #190) #191

Merged
merged 1 commit into from Nov 15, 2023

Conversation

leinfink
Copy link
Contributor

@leinfink leinfink commented Nov 13, 2023

This fixes #190.

I don't have a lot of experience in Elisp, so this might not be the best way of doing it, feel free to change things. Basically, I take the current path and build up a list of all sub-paths in it (e.g., for "/home/docs/stuff/", we get '("/home/" "/home/docs/" "/home/docs/stuff/"). As long as one of them is a member of denote-dired-directories, we should enable denote-dired-mode. This way, we don't have to actually scan for subdirectories.

@leinfink
Copy link
Contributor Author

I set the default value of the user option to nil to retain the current behavior.

@protesilaos protesilaos merged commit 36db77d into protesilaos:main Nov 15, 2023
@protesilaos
Copy link
Owner

Merged. Thank you!

protesilaos added a commit that referenced this pull request Nov 15, 2023
This change is within the ~15 line limit and thus does not require
copyright assignment to the Free Software Foundation.

It was done in pull request 191 on the GitHub mirror:
<#191>.
@protesilaos
Copy link
Owner

@leinfink What do you think about this tweak?

main 1ecbd55b46b7799348fdb9cb095357286ea30501
Author:     Protesilaos Stavrou <info@protesilaos.com>
AuthorDate: Wed Nov 15 08:20:04 2023 +0200
Commit:     Protesilaos Stavrou <info@protesilaos.com>
CommitDate: Wed Nov 15 08:20:04 2023 +0200

Parent:     d26b50b Acknowledge leinfink for commit 21f1c98
Merged:     main
Contained:  main
Follows:    2.1.0 (5)

Simplify denote-dired-mode-in-directories (tweak 21f1c98)

1 file changed, 9 insertions(+), 21 deletions(-)
denote.el | 30 +++++++++---------------------

modified   denote.el
@@ -2875,27 +2875,15 @@ (defun denote-dired-mode-in-directories ()
 
 If `denote-dired-include-subdirectories' is non-nil, also enable
 it in all subdirectories."
-  (cl-labels ((partial-paths (dir)
-                ;; e.g '("/home/", "/home/docs/", "/home/docs/stuff/")
-		(let* ((path-elements (split-string dir "/" t)))
-		  (collect-paths
-		   (cons (concat "/" (car path-elements))
-			 (cdr path-elements))
-		   nil)))
-	      (collect-paths (path-elements coll)
-		(if path-elements
-		    (collect-paths
-		     (cdr path-elements)
-		     (cons (concat (car coll) (car path-elements) "/")
-			   coll))
-		  coll)))
-    (let ((cur-path (file-truename default-directory)))
-      (when (seq-intersection
-	     (if denote-dired-include-subdirectories
-		 (partial-paths cur-path)
-	       (list cur-path))
-	     (denote-dired--modes-dirs-as-dirs))
-	(denote-dired-mode 1)))))
+  (when-let ((dirs (denote-dired--modes-dirs-as-dirs))
+             ;; Also include subdirs
+             ((and denote-dired-include-subdirectories
+                   (or (member (file-truename default-directory) (denote-dired--modes-dirs-as-dirs))
+                       (seq-some
+                        (lambda (dir)
+                          (string-prefix-p dir (file-truename default-directory)))
+                        dirs)))))
+    (denote-dired-mode 1)))
 
 ;;;; The linking facility
 

@leinfink
Copy link
Contributor Author

Great, that looks much more elegant!

@leinfink
Copy link
Contributor Author

leinfink commented Nov 15, 2023

Ah wait, shouldn't the and and or switch places in your tweak? Currently it doesn't work at all if denote-dired-include-subdirectories is nil, I think?

Like this:

(defun denote-dired-mode-in-directories ()
  "Enable `denote-dired-mode' in `denote-dired-directories'.
Add this function to `dired-mode-hook'."
  (when-let ((dirs (denote-dired--modes-dirs-as-dirs))
             ;; Also include subdirs
             ((or (member (file-truename default-directory) (denote-dired--modes-dirs-as-dirs))
		  (and denote-dired-include-subdirectories
		      (seq-some
                       (lambda (dir)
                         (string-prefix-p dir (file-truename default-directory)))
                       dirs)))))
    (denote-dired-mode 1)))

@leinfink
Copy link
Contributor Author

And perhaps make it even more compact by using the dirs binding in the member check as well, and binding current-dir to avoid repetition?

(defun denote-dired-mode-in-directories ()
  "Enable `denote-dired-mode' in `denote-dired-directories'.
Add this function to `dired-mode-hook'."
  (when-let ((dirs (denote-dired--modes-dirs-as-dirs))
	     (current-dir (file-truename default-directory))
             ;; Also include subdirs
             ((or (member current-dir dirs)
		  (and denote-dired-include-subdirectories
		       (seq-some
			(lambda (dir) (string-prefix-p dir current-dir))
			dirs)))))
    (denote-dired-mode 1)))

protesilaos added a commit that referenced this pull request Nov 15, 2023
Thanks to leinfink for the feedback in the comments of pull request
190 on the GitHub mirror: <#191>.
protesilaos added a commit that referenced this pull request Nov 15, 2023
… user option

This is a follow-up to pull request 190 on the GitHub mirror, which
added the original idea: <#191>.
Thanks to leinfink for the contribution!
@protesilaos
Copy link
Owner

Thanks for the feedback! I tweaked the code. Also updated the name of the variable to make it easier to communicate what it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include subdirectories in denote-dired-directories
2 participants