From 4501411f1f4bc59d97a66bedc07c4a8d3f19f480 Mon Sep 17 00:00:00 2001 From: Jun Inoue Date: Thu, 30 Nov 2023 11:37:28 +0900 Subject: [PATCH 1/4] Fix autoload bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invoking org-gtd-capture with the following setup results in org-gtd--ensure-file-exists: Symbol’s value as variable is void: org-gtd-archive-location (use-package org-gtd :bind (("M-o M-g M-c" . org-gtd-capture)) :init (setq org-gtd-update-ack "3.0.0") :config (org-gtd-mode 1)) --- org-gtd-core.el | 51 ++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/org-gtd-core.el b/org-gtd-core.el index 3ec6d8c..439479d 100644 --- a/org-gtd-core.el +++ b/org-gtd-core.el @@ -160,11 +160,6 @@ See `org-todo-keywords' for definition." ;;;; Variables -(defvar org-gtd-archive-location) -(defvar org-gtd-delegate-property) -(defvar org-gtd-project-headings) -(defvar org-gtd-stuck-projects) - (defvar-local org-gtd--loading-p nil "`Org-gtd' sets this variable after it has changed the state in this buffer.") @@ -174,28 +169,32 @@ See `org-todo-keywords' for definition." (defmacro with-org-gtd-context (&rest body) "Wrap BODY... in this macro to inherit the org-gtd settings for your logic." (declare (debug t) (indent 2)) - `(let* ((org-use-property-inheritance "ORG_GTD") - (org-todo-keywords `((sequence ,(string-join `(,org-gtd-next ,org-gtd-next-suffix)) - ,(string-join `(,org-gtd-todo ,org-gtd-todo-suffix)) - ,(string-join `(,org-gtd-wait ,org-gtd-wait-suffix)) - "|" - ,(string-join `(,org-gtd-done ,org-gtd-done-suffix)) - ,(string-join `(,org-gtd-canceled ,org-gtd-canceled-suffix))))) - ;; (org-log-done 'time) - ;; (org-log-done-with-time t) - ;; (org-log-refile 'time) - (org-archive-location (funcall org-gtd-archive-location)) - ;(org-refile-use-outline-path nil) - (org-stuck-projects org-gtd-stuck-projects) - (org-odd-levels-only nil) - (org-agenda-files (org-gtd-core--agenda-files)) - (org-agenda-property-list `(,org-gtd-delegate-property))) - (unwind-protect + `(progn + (require 'org-gtd-archive) + (require 'org-gtd-projects) + (require 'org-gtd-delegate) + (let* ((org-use-property-inheritance "ORG_GTD") + (org-todo-keywords `((sequence ,(string-join `(,org-gtd-next ,org-gtd-next-suffix)) + ,(string-join `(,org-gtd-todo ,org-gtd-todo-suffix)) + ,(string-join `(,org-gtd-wait ,org-gtd-wait-suffix)) + "|" + ,(string-join `(,org-gtd-done ,org-gtd-done-suffix)) + ,(string-join `(,org-gtd-canceled ,org-gtd-canceled-suffix))))) + ;; (org-log-done 'time) + ;; (org-log-done-with-time t) + ;; (org-log-refile 'time) + (org-archive-location (funcall org-gtd-archive-location)) + ;(org-refile-use-outline-path nil) + (org-stuck-projects org-gtd-stuck-projects) + (org-odd-levels-only nil) + (org-agenda-files (org-gtd-core--agenda-files)) + (org-agenda-property-list `(,org-gtd-delegate-property))) + (unwind-protect + (progn + (advice-add 'org-agenda-files :filter-return #'org-gtd-core--uniq) + ,@body) (progn - (advice-add 'org-agenda-files :filter-return #'org-gtd-core--uniq) - ,@body) - (progn - (advice-remove 'org-agenda-files #'org-gtd-core--uniq))))) + (advice-remove 'org-agenda-files #'org-gtd-core--uniq)))))) ;;;; Functions From ee5d9f473a6264eeab6dfbf529d8fb0d12869ec9 Mon Sep 17 00:00:00 2001 From: Jun Inoue Date: Thu, 30 Nov 2023 12:51:25 +0900 Subject: [PATCH 2/4] Add missing require --- org-gtd-core.el | 1 + 1 file changed, 1 insertion(+) diff --git a/org-gtd-core.el b/org-gtd-core.el index 439479d..85a4558 100644 --- a/org-gtd-core.el +++ b/org-gtd-core.el @@ -27,6 +27,7 @@ ;;;; Requirements +(require 'f) (require 'org-agenda-property) (require 'org-gtd-backward-compatibility) From 29e589f217150de23db2aea73d286d3094a573c4 Mon Sep 17 00:00:00 2001 From: Jun Inoue Date: Thu, 30 Nov 2023 13:39:49 +0900 Subject: [PATCH 3/4] Change to requiring org-gtd instead of individual modules Change with-org-gtd-context to require org-gtd instead of individual modules like org-gtd-archive. This is because: - It's more future-proof, in case more stuff gets added to this macro. - It solves another problem, where use-package's :config section is never executed. For the latter point, consider what happens with (use-package org-gtd :bind (("M-o" . org-gtd-engage)) :init (setq org-gtd-update-ack "3.0.0") :config (org-gtd-mode 1)) With the old code, org-gtd-engage doesn't load org-gtd.el but org-gtd-archive.el et al., so the :config doesn't kick in. --- org-gtd-core.el | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/org-gtd-core.el b/org-gtd-core.el index 85a4558..0fb87bb 100644 --- a/org-gtd-core.el +++ b/org-gtd-core.el @@ -171,9 +171,7 @@ See `org-todo-keywords' for definition." "Wrap BODY... in this macro to inherit the org-gtd settings for your logic." (declare (debug t) (indent 2)) `(progn - (require 'org-gtd-archive) - (require 'org-gtd-projects) - (require 'org-gtd-delegate) + (require 'org-gtd) (let* ((org-use-property-inheritance "ORG_GTD") (org-todo-keywords `((sequence ,(string-join `(,org-gtd-next ,org-gtd-next-suffix)) ,(string-join `(,org-gtd-todo ,org-gtd-todo-suffix)) From a8cc7a84d19aa0074bb5ce9bbaa76a905bff29c7 Mon Sep 17 00:00:00 2001 From: Jun Inoue Date: Thu, 7 Dec 2023 18:03:22 +0900 Subject: [PATCH 4/4] Add tests for autoloading Add tests to check if key autoloaded functions can be called without triggering errors in a fresh Emacs session. --- test/autoload-test.el | 65 +++++++++++++++++++ .../org-gtd-archive-completed-items-test.el | 12 ++++ test/autoload-tests/org-gtd-capture-test.el | 12 ++++ .../org-gtd-clarify-item-test.el | 15 +++++ .../org-gtd-clarify-mode-test.el | 9 +++ test/autoload-tests/org-gtd-engage-test.el | 12 ++++ .../autoload-tests/org-gtd-inbox-path-test.el | 13 ++++ test/autoload-tests/org-gtd-mode-test.el | 8 +++ .../org-gtd-process-inbox-test.el | 12 ++++ .../org-gtd-review-stuck-projects-test.el | 14 ++++ .../org-gtd-show-all-next-test.el | 12 ++++ test/helpers/autoload-setup.el | 22 +++++++ test/helpers/setup.el | 14 ++++ 13 files changed, 220 insertions(+) create mode 100644 test/autoload-test.el create mode 100644 test/autoload-tests/org-gtd-archive-completed-items-test.el create mode 100644 test/autoload-tests/org-gtd-capture-test.el create mode 100644 test/autoload-tests/org-gtd-clarify-item-test.el create mode 100644 test/autoload-tests/org-gtd-clarify-mode-test.el create mode 100644 test/autoload-tests/org-gtd-engage-test.el create mode 100644 test/autoload-tests/org-gtd-inbox-path-test.el create mode 100644 test/autoload-tests/org-gtd-mode-test.el create mode 100644 test/autoload-tests/org-gtd-process-inbox-test.el create mode 100644 test/autoload-tests/org-gtd-review-stuck-projects-test.el create mode 100644 test/autoload-tests/org-gtd-show-all-next-test.el create mode 100644 test/helpers/autoload-setup.el diff --git a/test/autoload-test.el b/test/autoload-test.el new file mode 100644 index 0000000..cd7cf01 --- /dev/null +++ b/test/autoload-test.el @@ -0,0 +1,65 @@ +(load "test/helpers/setup.el") +(require 'buttercup) +(describe + "autoloaded entry point" + + (it "org-gtd-archive-completed-items" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-archive-completed-items-test.el") + :to-equal 0)) + + (it "org-gtd-capture" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-capture-test.el") + :to-equal 0)) + + (it "org-gtd-clarify-item" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-clarify-item-test.el") + :to-equal 0)) + + (it "org-gtd-clarify-mode" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-clarify-mode-test.el") + :to-equal 0)) + + (it "org-gtd-engage" + (expect + (ogt--recursive-eldev-test "autoload-tests/org-gtd-engage-test.el") + :to-equal 0)) + + (it "org-gtd-inbox-path" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-inbox-path-test.el") + :to-equal 0)) + + (it "org-gtd-mode" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-mode-test.el") + :to-equal 0)) + + (it "org-gtd-process-inbox" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-process-inbox-test.el") + :to-equal 0)) + + (it "org-gtd-review-stuck-projects" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-review-stuck-projects-test.el") + :to-equal 0)) + + (it "org-gtd-show-all-next" + (expect + (ogt--recursive-eldev-test + "autoload-tests/org-gtd-show-all-next-test.el") + :to-equal 0)) + + ) diff --git a/test/autoload-tests/org-gtd-archive-completed-items-test.el b/test/autoload-tests/org-gtd-archive-completed-items-test.el new file mode 100644 index 0000000..ec1a9d6 --- /dev/null +++ b/test/autoload-tests/org-gtd-archive-completed-items-test.el @@ -0,0 +1,12 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") +(load "test/helpers/autoload-setup.el") + +(describe + "autoload management (recursive)" + (before-each (ogt--prepare-gtd-directory)) + (after-each (ogt--clear-gtd-directory)) + + (it "org-gtd-archive-completed-items" + (org-gtd-archive-completed-items))) diff --git a/test/autoload-tests/org-gtd-capture-test.el b/test/autoload-tests/org-gtd-capture-test.el new file mode 100644 index 0000000..a0dd991 --- /dev/null +++ b/test/autoload-tests/org-gtd-capture-test.el @@ -0,0 +1,12 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") +(load "test/helpers/autoload-setup.el") + +(describe + "autoload management (recursive)" + (before-each (ogt--prepare-gtd-directory)) + (after-each (ogt--clear-gtd-directory)) + + (it "org-gtd-capture" + (org-gtd-capture nil "i"))) diff --git a/test/autoload-tests/org-gtd-clarify-item-test.el b/test/autoload-tests/org-gtd-clarify-item-test.el new file mode 100644 index 0000000..b310e30 --- /dev/null +++ b/test/autoload-tests/org-gtd-clarify-item-test.el @@ -0,0 +1,15 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") +(load "test/helpers/utils.el") +(load "test/helpers/autoload-setup.el") + +(describe + "autoload management (recursive)" + (before-each (ogt--prepare-gtd-directory)) + (after-each (ogt--clear-gtd-directory)) + + (it "org-gtd-clarify-item" + (ogt--with-temp-org-buffer + "* This is the heading to clarify" + (org-gtd-clarify-item)))) diff --git a/test/autoload-tests/org-gtd-clarify-mode-test.el b/test/autoload-tests/org-gtd-clarify-mode-test.el new file mode 100644 index 0000000..ff35e88 --- /dev/null +++ b/test/autoload-tests/org-gtd-clarify-mode-test.el @@ -0,0 +1,9 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") + +(describe + "autoload management (recursive)" + + (it "org-gtd-clarify-mode" + (org-gtd-clarify-mode))) diff --git a/test/autoload-tests/org-gtd-engage-test.el b/test/autoload-tests/org-gtd-engage-test.el new file mode 100644 index 0000000..c6ba452 --- /dev/null +++ b/test/autoload-tests/org-gtd-engage-test.el @@ -0,0 +1,12 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") +(load "test/helpers/autoload-setup.el") + +(describe + "autoload management (recursive)" + (before-each (ogt--prepare-gtd-directory)) + (after-each (ogt--clear-gtd-directory)) + + (it "org-gtd-engage" + (org-gtd-engage))) diff --git a/test/autoload-tests/org-gtd-inbox-path-test.el b/test/autoload-tests/org-gtd-inbox-path-test.el new file mode 100644 index 0000000..411e795 --- /dev/null +++ b/test/autoload-tests/org-gtd-inbox-path-test.el @@ -0,0 +1,13 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") + +(load "test/helpers/autoload-setup.el") + +(describe + "autoload management (recursive)" + (before-each (ogt--prepare-gtd-directory)) + (after-each (ogt--clear-gtd-directory)) + + (it "org-gtd-inbox-path" + (org-gtd-inbox-path))) diff --git a/test/autoload-tests/org-gtd-mode-test.el b/test/autoload-tests/org-gtd-mode-test.el new file mode 100644 index 0000000..39f3b39 --- /dev/null +++ b/test/autoload-tests/org-gtd-mode-test.el @@ -0,0 +1,8 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") + +(describe + "autoload management (recursive)" + (it "org-gtd-mode" + (org-gtd-mode))) diff --git a/test/autoload-tests/org-gtd-process-inbox-test.el b/test/autoload-tests/org-gtd-process-inbox-test.el new file mode 100644 index 0000000..52f4559 --- /dev/null +++ b/test/autoload-tests/org-gtd-process-inbox-test.el @@ -0,0 +1,12 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") +(load "test/helpers/autoload-setup.el") + +(describe + "autoload management (recursive)" + (before-each (ogt--prepare-gtd-directory)) + (after-each (ogt--clear-gtd-directory)) + + (it "org-gtd-process-inbox" + (org-gtd-process-inbox))) diff --git a/test/autoload-tests/org-gtd-review-stuck-projects-test.el b/test/autoload-tests/org-gtd-review-stuck-projects-test.el new file mode 100644 index 0000000..d097ad2 --- /dev/null +++ b/test/autoload-tests/org-gtd-review-stuck-projects-test.el @@ -0,0 +1,14 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") +(load "test/helpers/autoload-setup.el") + +(describe + "autoload management (recursive)" + (before-each (ogt--prepare-gtd-directory)) + (after-each (ogt--clear-gtd-directory)) + + (it "org-gtd-review-stuck-projects" + (org-gtd-review-stuck-projects))) + + diff --git a/test/autoload-tests/org-gtd-show-all-next-test.el b/test/autoload-tests/org-gtd-show-all-next-test.el new file mode 100644 index 0000000..485b0de --- /dev/null +++ b/test/autoload-tests/org-gtd-show-all-next-test.el @@ -0,0 +1,12 @@ +(require 'buttercup) +(setq org-gtd-update-ack "3.0.0") +(load "org-gtd-autoloads") +(load "test/helpers/autoload-setup.el") + +(describe + "autoload management (recursive)" + (before-each (ogt--prepare-gtd-directory)) + (after-each (ogt--clear-gtd-directory)) + + (it "org-gtd-show-all-next" + (org-gtd-show-all-next))) diff --git a/test/helpers/autoload-setup.el b/test/helpers/autoload-setup.el new file mode 100644 index 0000000..94d460b --- /dev/null +++ b/test/helpers/autoload-setup.el @@ -0,0 +1,22 @@ +;; NEVER put loads or requires in here. That's the point of this helper: to +;; help test things in an environment where things aren't loaded. + +(defun ogt--prepare-gtd-directory () + "Run before autoload test that needs the gtd directory to exist." + (setq org-gtd-directory (make-temp-file "org-gtd" t))) + +(defmacro ogt--with-temp-org-buffer (contents &rest body) + "Like `with-temp-buffer', but in Org mode. + +CONTENTS is inserted and point is set to the buffer's beginning +before running BODY." + (declare (debug t)) + `(with-temp-buffer + (org-mode) + (insert ,contents) + (goto-char 1) + ,@body)) + +(defun ogt--clear-gtd-directory () + "Clean up after `ogt--prepare-gtd-directory'." + (delete-directory org-gtd-directory t)) diff --git a/test/helpers/setup.el b/test/helpers/setup.el index 8eb53e2..5cd09a7 100644 --- a/test/helpers/setup.el +++ b/test/helpers/setup.el @@ -46,3 +46,17 @@ (with-current-buffer buffer (revert-buffer t t))) (kill-buffer buffer)) + +(defun ogt--recursive-eldev-test (file) + (unless (file-readable-p (file-name-concat "test" file)) + (error "Cannot find or read file %s" file)) + (with-temp-buffer + (prog1 (call-process + eldev-shell-command + nil + t + nil + "test" + "-f" + file) + (princ (buffer-substring 1 (point-max))))))