org-timer: let done hook access time, allow silencing notification

org-timer: let done hook access time, allow silencing notification

[Stefan van der Walt]

Hi all,

I have two small improvement suggestions for org-timer, and would like to hear your thoughts.

1. I use org-timer to run pomodoro-style clocks. When the timer finishes, I would like to generate a log entry. It would be helpful to be able to access the duration of the clock; however, in org-timer--run-countdown-timer, the variable org-timer-start-time is reset BEFORE the org-timer-done-hook is run.

Could we reset the variable *after* the hooks are run instead, so that they can access the timer value?

2. I have a custom notification for when the timer is done, installed via the done hook. I would like to suppress the existing notification, but there seems to be no option to do so. Would it make sense to provide a mechanism to override/silence the notification?

Thanks!
Stéfan

Re: org-timer: let done hook access time, allow silencing notification

[Ihor Radchenko]

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]

"Stefan van der Walt" <stefanv@berkeley.edu> writes:

> I have two small improvement suggestions for org-timer, and would like to hear your thoughts.
>
> 1. I use org-timer to run pomodoro-style clocks. When the timer finishes, I would like to generate a log entry. It would be helpful to be able to access the duration of the clock; however, in org-timer--run-countdown-timer, the variable org-timer-start-time is reset BEFORE the org-timer-done-hook is run.
>
> Could we reset the variable *after* the hooks are run instead, so that they can access the timer value?

Yes, that would make sense. See the attached tentative patch.

> 2. I have a custom notification for when the timer is done, installed via the done hook. I would like to suppress the existing notification, but there seems to be no option to do so. Would it make sense to provide a mechanism to override/silence the notification?

We can do it.
One would need to implement analogues to
org-timer-start/stop/pause/continue/set/done-hook that will hold what
Org does by default at that point (turn on/off mode line, play sound,
print message, etc). Something like
org-timer-start/stop/...-default-hook.

Then, users can additionally customize those variables to their
preference.

Patches welcome!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-timer-done-hook-Run-before-the-timer-is-stopped.patch --]
[-- Type: text/x-patch, Size: 2529 bytes --]

From ca98848895da7c540e4f20049df53d08dcadddd9 Mon Sep 17 00:00:00 2001
Message-ID: <ca98848895da7c540e4f20049df53d08dcadddd9.1736615477.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Sat, 11 Jan 2025 18:07:43 +0100
Subject: [PATCH] org-timer-done-hook: Run before the timer is stopped

* lisp/org-timer.el (org-timer--run-countdown-timer): Stop the timer
and unset variables _after_ `org-timer-done-hook' is ran.  This way,
timer data is available for the hook functions.
* etc/ORG-NEWS (~org-timer-done-hook~ is now ran before the timer is stopped):
Announce the change.

Link: https://orgmode.org/list/a25376e4-f6f6-421d-a8f2-ce280e8d2b15@app.fastmail.com
---
 etc/ORG-NEWS      |  5 +++++
 lisp/org-timer.el | 13 +++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index c9bb192de3..328accc216 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -391,6 +391,11 @@ capture ~:tree-type~ options]], the internal variable
 undocumented helper function ~org-datetree-insert-line~.
 
 ** Miscellaneous
+*** ~org-timer-done-hook~ is now ran before the timer is stopped
+
+Previously, ~org-timer-countdown-timer~ and ~org-timer-start-time~
+were unset when the hook is ran.  Now, they still hold the timer info.
+
 *** ox-latex: LaTeX images are now stored alongside the exported =.html= file
 
 Previously, LaTeX images (when HTML export does use images for LaTeX)
diff --git a/lisp/org-timer.el b/lisp/org-timer.el
index 9d4e350429..ee13fbe193 100644
--- a/lisp/org-timer.el
+++ b/lisp/org-timer.el
@@ -108,7 +108,8 @@ (defvar org-timer-set-hook nil
   "Hook run after countdown timer is set.")
 
 (defvar org-timer-done-hook nil
-  "Hook run after countdown timer reaches zero.")
+  "Hook run after countdown timer reaches zero.
+The hook is ran before the timer is actually stopped.")
 
 ;;;###autoload
 (defun org-timer-start (&optional offset)
@@ -470,11 +471,11 @@ (defun org-timer--run-countdown-timer (secs title)
         (sound org-clock-sound))
     (run-with-timer
      secs nil (lambda ()
-		(setq org-timer-countdown-timer nil
-		      org-timer-start-time nil)
-		(org-notify msg sound)
-		(org-timer-set-mode-line 'off)
-		(run-hooks 'org-timer-done-hook)))))
+	      (org-notify msg sound)
+	      (org-timer-set-mode-line 'off)
+	      (run-hooks 'org-timer-done-hook)
+              (setq org-timer-countdown-timer nil
+		    org-timer-start-time nil)))))
 
 (defun org-timer--get-timer-title ()
   "Construct timer title.
-- 
2.47.1


[-- Attachment #3: Type: text/plain, Size: 223 bytes --]


-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: org-timer: let done hook access time, allow silencing notification

[Stefan van der Walt]

On Sat, Jan 11, 2025, at 09:17, Ihor Radchenko wrote:
> "Stefan van der Walt" <stefanv@berkeley.edu> writes:
>
>> I have two small improvement suggestions for org-timer, and would like to hear your thoughts.
>>
>> 1. I use org-timer to run pomodoro-style clocks. When the timer finishes, I would like to generate a log entry. It would be helpful to be able to access the duration of the clock; however, in org-timer--run-countdown-timer, the variable org-timer-start-time is reset BEFORE the org-timer-done-hook is run.
>>
>> Could we reset the variable *after* the hooks are run instead, so that they can access the timer value?
>
> Yes, that would make sense. See the attached tentative patch.

Thanks, Ihor! That patch looks great. In the description: `ran` -> `run`.

>> 2. I have a custom notification for when the timer is done, installed via the done hook. I would like to suppress the existing notification, but there seems to be no option to do so. Would it make sense to provide a mechanism to override/silence the notification?
>
> We can do it.
> One would need to implement analogues to
> org-timer-start/stop/pause/continue/set/done-hook that will hold what
> Org does by default at that point (turn on/off mode line, play sound,
> print message, etc). Something like
> org-timer-start/stop/...-default-hook.

That would be a good generic solution. In my case, if I can suppress org-show-notification, it'd be enough. Here's what I tried (but it didn't work):

  (defun stefanv/suppress-org-notify (orig-fun &rest args)
    (cl-letf (
               ((symbol-function 'org-show-notification) (lambda (&rest _) (ignore)))
             )
      (apply orig-fun args)))
  (advice-add 'org-timer--run-countdown-timer :around #'stefanv/suppress-org-notify)

Stéfan

Re: org-timer: let done hook access time, allow silencing notification

[Ihor Radchenko]

"Stefan van der Walt" <stefanv@berkeley.edu> writes:
>>> Could we reset the variable *after* the hooks are run instead, so that they can access the timer value?
>>
>> Yes, that would make sense. See the attached tentative patch.
>
> Thanks, Ihor! That patch looks great. In the description: `ran` -> `run`.

Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=9368db471b

>>> 2. I have a custom notification for when the timer is done, installed via the done hook. I would like to suppress the existing notification, but there seems to be no option to do so. Would it make sense to provide a mechanism to override/silence the notification?
>>
>> We can do it.
>> One would need to implement analogues to
>> org-timer-start/stop/pause/continue/set/done-hook that will hold what
>> Org does by default at that point (turn on/off mode line, play sound,
>> print message, etc). Something like
>> org-timer-start/stop/...-default-hook.
>
> That would be a good generic solution. In my case, if I can suppress org-show-notification, it'd be enough. Here's what I tried (but it didn't work):
>
>   (defun stefanv/suppress-org-notify (orig-fun &rest args)
>     (cl-letf (
>                ((symbol-function 'org-show-notification) (lambda (&rest _) (ignore)))
>              )
>       (apply orig-fun args)))
>   (advice-add 'org-timer--run-countdown-timer :around #'stefanv/suppress-org-notify)

Your solution won't work because `org-timer--run-countdown-timer' does
not actually execute `org-show-notification'. It just sets up a timer to
call that function later, outside the dynamic scope you are setting up
with `cl-letf'.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>