From 3343c19fc3e544d6b7cfde3f9eb62b28a77577ee Mon Sep 17 00:00:00 2001 From: contrapunctus Date: Mon, 7 Sep 2020 20:43:23 +0530 Subject: [PATCH] Change before-out-functions; modify affected hook functions This is basically to try and see if we can * introduce some consistency - this particular hook behaves differently from the others; * remove some complexity - each hook function executes a lot of logical gymnastics, and returns t/nil for reasons which are not immediately obvious; these then have to be explained in each docstring; * have a UX more consistent with the rest of Emacs - users usually press C-g to quit a prompt. --- README.md | 18 +++++++++--------- elisp/chronometrist-key-values.el | 18 ++++++------------ elisp/chronometrist.el | 10 ++++++---- 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 678bc44..270f3e3 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,8 @@ Largely modelled after the Android application, [A Time Tracker](https://github. **IMPORTANT: with version v0.3, chronometrist no longer uses timeclock as a dependency and will use its own s-expression-based backend. A command to migrate the timeclock-file, `chronometrist-migrate-timelog-file->sexp-file`, is provided.** +**IMPORTANT: `chronometrist-before-out-functions` now behaves just like other hooks - it runs all of its functions unconditionally, instead of stopping when one of them returns nil.** + ## Comparisons ### timeclock.el Compared to timeclock.el, Chronometrist @@ -136,15 +138,13 @@ Another one, prompting the user if they have uncommitted changes in a git reposi (defun my-commit-prompt () "Prompt user if `default-directory' is a dirty Git repository. -Return t if the user answers yes, if the repository is clean, or -if there is no Git repository. - -Return nil (and run `magit-status') if the user answers no." - (cond ((not (magit-anything-modified-p)) t) - ((yes-or-no-p - (format "You have uncommitted changes in %S. Really clock out? " - default-directory)) t) - (t (magit-status) nil))) +If the user answers \"no\", cancel clocking out and run +`magit-status'." + (or (not (magit-anything-modified-p)) + (yes-or-no-p + (format "You have uncommitted changes in %S. Really clock out? " + default-directory)) + (throw 'chronometrist-hook-quit (magit-status)))) (add-hook 'chronometrist-before-out-functions 'my-commit-prompt) ``` diff --git a/elisp/chronometrist-key-values.el b/elisp/chronometrist-key-values.el index 89d04f1..5e99674 100644 --- a/elisp/chronometrist-key-values.el +++ b/elisp/chronometrist-key-values.el @@ -203,8 +203,7 @@ INITIAL-INPUT is as used in `completing-read'." (defun chronometrist-tags-add (&rest _args) "Read tags from the user; add them to the last entry in `chronometrist-file'. -_ARGS are ignored. This function always returns t, so it can be -used in `chronometrist-before-out-functions'." +_ARGS are ignored." (unless chronometrist--skip-detail-prompts (let* ((last-expr (chronometrist-last)) (last-name (plist-get last-expr :name)) @@ -220,8 +219,7 @@ used in `chronometrist-before-out-functions'." (reverse it) (cl-remove-duplicates it :test #'equal) (reverse it) - (chronometrist-append-to-last it nil))))) - t) + (chronometrist-append-to-last it nil)))))) ;;;; KEY-VALUES ;;;; (defgroup chronometrist-key-values nil @@ -392,8 +390,7 @@ In the resulting buffer, users can run `chronometrist-kv-accept' to add them to the last s-expression in `chronometrist-file', or `chronometrist-kv-reject' to cancel. -_ARGS are ignored. This function always returns t, so it can be -used in `chronometrist-before-out-functions'." +_ARGS are ignored." (unless chronometrist--skip-detail-prompts (let* ((buffer (get-buffer-create chronometrist-kv-buffer-name)) (first-key-p t) @@ -430,8 +427,7 @@ used in `chronometrist-before-out-functions'." (if (string-empty-p input) (throw 'empty-input nil) (chronometrist-value-insert value))))) - (chronometrist-sexp-reindent-buffer)))) - t) + (chronometrist-sexp-reindent-buffer))))) ;;;; COMMANDS ;;;; (defun chronometrist-kv-accept () @@ -456,8 +452,7 @@ used in `chronometrist-before-out-functions'." (defvar chronometrist--skip-detail-prompts nil) (defun chronometrist-skip-query-prompt (task) - "Offer to skip tag/key-value prompts and reuse last-used details. -This function always returns t, so it can be used in `chronometrist-before-out-functions'." + "Offer to skip tag/key-value prompts and reuse last-used details." ;; find latest interval for TASK; if it has tags or key-values, prompt (let (plist) ;; iterate over events in reverse @@ -471,8 +466,7 @@ This function always returns t, so it can be used in `chronometrist-before-out-f (yes-or-no-p (format "Skip prompt and use last-used tags/key-values? %S " plist)) (setq chronometrist--skip-detail-prompts t) - (chronometrist-append-to-last (plist-get plist :tags) plist)) - t)) + (chronometrist-append-to-last (plist-get plist :tags) plist)))) (defun chronometrist-skip-query-reset (_task) "Enable prompting for tags and key-values. diff --git a/elisp/chronometrist.el b/elisp/chronometrist.el index 5eae8e5..816bef3 100644 --- a/elisp/chronometrist.el +++ b/elisp/chronometrist.el @@ -295,13 +295,15 @@ is the name of the task to be clocked out of.") (defun chronometrist-run-functions-and-clock-in (task) "Run hooks and clock in to TASK." - (run-hook-with-args 'chronometrist-before-in-functions task) - (chronometrist-in task) - (run-hook-with-args 'chronometrist-after-in-functions task)) + (catch 'chronometrist-hook-quit + (run-hook-with-args 'chronometrist-before-in-functions task) + (chronometrist-in task) + (run-hook-with-args 'chronometrist-after-in-functions task))) (defun chronometrist-run-functions-and-clock-out (task) "Run hooks and clock out of TASK." - (when (run-hook-with-args-until-failure 'chronometrist-before-out-functions task) + (catch 'chronometrist-hook-quit + (run-hook-with-args 'chronometrist-before-out-functions task) (chronometrist-out) (run-hook-with-args 'chronometrist-after-out-functions task)))