From cea165192d25e6115adf7e309fa53b45aff2f633 Mon Sep 17 00:00:00 2001 From: Ahmed Shariff Date: Thu, 16 Sep 2021 16:49:19 -0500 Subject: [PATCH] Cleaning up functions and improvements for clarity. --- org-ql-view.el | 121 +++++++++++++++++++++++-------------------- tests/test-org-ql.el | 16 +++--- 2 files changed, 73 insertions(+), 64 deletions(-) diff --git a/org-ql-view.el b/org-ql-view.el index 90e41f8..a707694 100644 --- a/org-ql-view.el +++ b/org-ql-view.el @@ -1032,39 +1032,41 @@ current buffer. Otherwise BUFFERS-FILES is returned unchanged." (string (expand-file-name it)) (otherwise it)) list))) - (--> - ;; TODO: Test this more exhaustively. - (pcase buffers-files - ((pred listp) - (pcase (expand-files buffers-files) - ((pred (seq-set-equal-p (mapcar #'expand-file-name (org-agenda-files)))) + (let ((contracted-buffers-files + ;; TODO: Test this more exhaustively. + (pcase buffers-files + ((pred listp) + (pcase (expand-files buffers-files) + ((pred (seq-set-equal-p (mapcar #'expand-file-name (org-agenda-files)))) + "org-agenda-files") + ((and (guard (file-exists-p org-directory)) + (pred (seq-set-equal-p (org-ql-search-directories-files + :directories (list org-directory))))) + "org-directory") + (_ buffers-files))) + ((pred (equal (current-buffer))) + "buffer") + ((or 'org-agenda-files '(function org-agenda-files)) "org-agenda-files") - ((and (guard (file-exists-p org-directory)) - (pred (seq-set-equal-p (org-ql-search-directories-files - :directories (list org-directory))))) - "org-directory") - (_ buffers-files))) - ((pred (equal (current-buffer))) - "buffer") - ((or 'org-agenda-files '(function org-agenda-files)) - "org-agenda-files") - ((and (pred bufferp) (guard (buffer-file-name buffers-files))) - (buffer-file-name buffers-files)) - ((pred bufferp) - (buffer-name buffers-files)) - (_ buffers-files)) - ;; All items needs to be strings to pick duplicates when used with the extend conterpart. - ;; So making sure the buffers are convered to file names - (if (stringp it) - it - (-map - (lambda (buffer-file) - (if (bufferp buffer-file) - (--if-let (buffer-file-name buffer-file) - it - (buffer-name buffer-file)) - buffer-file)) - it))))) + ((and (pred bufferp) (guard (buffer-file-name buffers-files))) + (buffer-file-name buffers-files)) + ((pred bufferp) + (buffer-name buffers-files)) + (_ buffers-files)))) + ;; To filter duplicates with the extend counterpart of this function, + ;; this needs to be a string or a list of string. + ;; Hence, making sure the buffers are convered to file names or buffer names. + ;; Using file-names when it's a file-buffer to avoid duplicates resulting from + ;; the file-buffer and file name being entered. + (cl-typecase contracted-buffers-files + (string contracted-buffers-files) + (list (--map + (pcase-exhaustive it + ((pred stringp) it) + ((pred bufferp) (or (buffer-file-name it) + (buffer-name buffer-file)))) + contracted-buffers-files)) + (t (error (format "Value %s is not a string or a list of buffer/strings" contracted-buffers-files))))))) (defun org-ql-view--complete-buffers-files () "Return value for `org-ql-view-buffers-files' using completion. @@ -1093,30 +1095,37 @@ representation `org-ql-view-buffers-files' is returned." "Return BUFFERS-FILES expanded to a list of files or buffers. The counterpart to `org-ql-view--contract-buffers-files'. This always returns a list of string values." - (--> - (-map (lambda (buffer-file) - (pcase-exhaustive buffer-file - ("all" (--select (equal (buffer-local-value 'major-mode it) 'org-mode) - (buffer-list))) - ("org-agenda-files" (org-agenda-files)) - ("org-directory" (org-ql-search-directories-files)) - ((or "" "buffer") - (current-buffer)) - ((pred bufferp) (list buffer-file)) - ;; A single filename. - ((pred stringp) (list buffer-file)) - (_ (error (format "Value %s is not a valid buffer/file" buffer-file))))) - (-list buffers-files)) - -flatten -non-nil - ;; expanding all file-buffers to file names to avoid duplicate entries being formed - (-map (lambda (buffer-file) - (if (bufferp buffer-file) - (--if-let (buffer-file-name buffer-file) - it - (buffer-name buffer-file)) - buffer-file)) - it) - -uniq)) + (let ((expanded-buffers-files + (--> buffers-files + -list -non-nil + (-map (lambda (buffer-file) + (pcase-exhaustive buffer-file + ("all" (--select (equal (buffer-local-value 'major-mode it) 'org-mode) + (buffer-list))) + ("org-agenda-files" (org-agenda-files)) + ("org-directory" (org-ql-search-directories-files)) + ((or "" "buffer") + (current-buffer)) + ((or (pred bufferp) + ;; A single filename. + (pred stringp)) + buffer-file) + (_ (error (format "Value %s is not a valid buffer/file" buffer-file))))) + it)))) + (--> expanded-buffers-files + -flatten + ;; removing `nil' again as some values have been expanded. + -non-nil + ;; Expanding all buffers to file names or buffer names to remove duplicate entries. + (--map + (pcase-exhaustive it + ((pred bufferp) (or (buffer-file-name it) + (buffer-name it))) + ;; Any values at this point should be a buffer or string. + ;; Testing for string anyways. + ((pred stringp) it)) + it) + -uniq))) (defun org-ql-view--complete-super-groups () "Return value for `org-ql-view-super-groups' using completion." diff --git a/tests/test-org-ql.el b/tests/test-org-ql.el index dde43c0..9f76d68 100644 --- a/tests/test-org-ql.el +++ b/tests/test-org-ql.el @@ -2130,11 +2130,11 @@ with keyword arg NOW in PLIST." (expect (org-ql-view--contract-buffers-files 'org-agenda-files) :to-equal "org-agenda-files") (expect (org-ql-view--contract-buffers-files #'org-agenda-files) :to-equal "org-agenda-files")) (it "arbitarary list of buffers/files" - (let ((value1 '("a.org" "b.org")) - (value2 'a)) - (expect (org-ql-view--contract-buffers-files value1) :to-equal value1) - ;; If the value does not result to a buffer, file, or string, throws error - (expect (org-ql-view--contract-buffers-files value2) :to-throw)))) + (let ((list-of-strings '("a.org" "b.org")) + (invalid-type 'a)) + (expect (org-ql-view--contract-buffers-files list-of-strings) :to-equal list-of-strings) + ;; Signal error if value is not a buffer, file, or string. + (expect (org-ql-view--contract-buffers-files invalid-type) :to-throw)))) (describe "expanding org-ql-view-buffers-files" (it "returns all buffers with `org-mode' as the major-mode" (let ((buffers (list (generate-new-buffer "test.org") (generate-new-buffer "test.other")))) @@ -2150,7 +2150,7 @@ with keyword arg NOW in PLIST." (generate-new-buffer "test2"))))) (expect (org-ql-view--expand-buffers-files "org-agenda-files") :to-equal org-agenda-files))) (it "returns values of \"org-directory\"" - ;; Also indirectly tests `org-ql-view--expand-buffers-files' + ;; Also indirectly tests `org-ql-view--expand-buffers-files'. (let ((org-directory temp-dir)) (expect (org-ql-view--expand-buffers-files "org-directory") :to-equal temp-filenames))) (it "returns the current buffer" @@ -2163,7 +2163,7 @@ with keyword arg NOW in PLIST." (expect (org-ql-view--expand-buffers-files test-buffer) :to-equal (list (buffer-name test-buffer)))) (let ((list-of-numbers '(1 2 3)) (literal-string "random string")) - ;; If the value does not result to a buffer, file, or string, throws error + ;; Signal error if any of the values are not a buffer, file, or string. (expect (org-ql-view--expand-buffers-files list-of-numbers) :to-throw) (expect (org-ql-view--expand-buffers-files literal-string) :to-equal (list literal-string))))) (describe "testing `org-ql-view--complete-buffers-files'" @@ -2174,7 +2174,7 @@ with keyword arg NOW in PLIST." (spy-on 'completing-read-multiple :and-return-value "org-agenda-files") (expect (org-ql-view--complete-buffers-files) :to-equal temp-filenames) (expect 'org-ql-view--contract-buffers-files :to-have-been-called-with temp-filenames) - ;; Also testing if the initial values are set correctly + ;; Also testing if the initial values are set correctly. (expect 'completing-read-multiple :to-have-been-called-with "Buffers/Files: " (list 'buffer 'org-agenda-files 'org-directory 'all) nil nil "org-agenda-files")))