From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kyle Meyer Subject: [RFC/PATCH] Fixes for org-timer Date: Wed, 03 Dec 2014 01:05:44 -0500 Message-ID: <874mtdjmiv.fsf@kmlap.domain.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:57950) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xw31m-0000uV-Lv for emacs-orgmode@gnu.org; Wed, 03 Dec 2014 01:03:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xw31i-0007ZE-4P for emacs-orgmode@gnu.org; Wed, 03 Dec 2014 01:03:06 -0500 Received: from mail-qa0-f46.google.com ([209.85.216.46]:61635) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xw31h-0007Ya-Vu for emacs-orgmode@gnu.org; Wed, 03 Dec 2014 01:03:02 -0500 Received: by mail-qa0-f46.google.com with SMTP id n8so3857040qaq.5 for ; Tue, 02 Dec 2014 22:03:00 -0800 (PST) Received: from localhost ([2601:6:5480:1e5:9e4e:36ff:fe3d:ae9c]) by mx.google.com with ESMTPSA id q4sm22585715qax.31.2014.12.02.22.02.58 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 02 Dec 2014 22:02:59 -0800 (PST) List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Org-mode --=-=-= Content-Type: text/plain Hello, I've attached a few patches for org-timer.el. Some additional comments on two of the patches: [PATCH 1/4] org-timer.el (org-timer): Recognize double prefix This patch makes org-timer behave as described in the docstring when it is given a double prefix argument. However, I'd actually be in favor of removing this functionality from org-timer (and org-timer-start) since it's already available directly through the interactive command org-timer-change-times-in-region. [PATCH 4/4] org-timer.el: Isolate commands of different timers This patch fixes several issues that were caused by executing a relative timer command when the countdown timer was running, or vice versa (see log message for details). Perhaps it would also be helpful to make it clearer which commands belong to the two different timers. This is clear in the manual (the relative and countdown timer have different documentation pages), but what do you think about renaming countdown timer commands to have 'countdown' after 'org-timer' (e.g., org-timer-countdown-set-timer)? Thanks. -- Kyle --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-org-timer.el-org-timer-Recognize-double-prefix.patch >From 1f4dd0e57401558a0cace1b421b1d37bca37fd15 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Mon, 1 Dec 2014 02:02:19 -0500 Subject: [PATCH 1/4] org-timer.el (org-timer): Recognize double prefix * lisp/org-timer.el (org-timer): Follow the behavior described in the docstring for a double prefix argument. --- lisp/org-timer.el | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lisp/org-timer.el b/lisp/org-timer.el index d02362c..c9710ca 100644 --- a/lisp/org-timer.el +++ b/lisp/org-timer.el @@ -177,11 +177,13 @@ (defun org-timer (&optional restart no-insert-p) If NO-INSERT-P is non-nil, return the string instead of inserting it in the buffer." (interactive "P") - (when (or (equal restart '(4)) (not org-timer-start-time)) - (org-timer-start)) - (if no-insert-p - (org-timer-value-string) - (insert (org-timer-value-string)))) + (if (equal restart '(16)) + (org-timer-start restart) + (when (or (equal restart '(4)) (not org-timer-start-time)) + (org-timer-start)) + (if no-insert-p + (org-timer-value-string) + (insert (org-timer-value-string))))) (defun org-timer-value-string () "Set the timer string." -- 2.1.3 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-org-timer.el-org-timer-start-Reset-on-pause.patch >From 96b50113f8ddba96488bac8eefdc90e31df23415 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Tue, 2 Dec 2014 22:05:08 -0500 Subject: [PATCH 2/4] org-timer.el (org-timer-start): Reset on pause * lisp/org-timer.el (org-timer-start): Reset to correct state when called with a paused timer. Since org-timer-pause-time was not returned to nil, the modeline remained stuck at the paused time. --- lisp/org-timer.el | 1 + 1 file changed, 1 insertion(+) diff --git a/lisp/org-timer.el b/lisp/org-timer.el index c9710ca..1c4153c 100644 --- a/lisp/org-timer.el +++ b/lisp/org-timer.el @@ -124,6 +124,7 @@ (defun org-timer-start (&optional offset) (setq org-timer-start-time (seconds-to-time (- (org-float-time) delta)))) + (setq org-timer-pause-time nil) (org-timer-set-mode-line 'on) (message "Timer start time set to %s, current value is %s" (format-time-string "%T" org-timer-start-time) -- 2.1.3 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0003-org-timer.el-Reset-properly-after-countdown-timer.patch >From 9914c187c3b5ff1dcabfd15a4588e7dbb676231d Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Tue, 2 Dec 2014 23:35:49 -0500 Subject: [PATCH 3/4] org-timer.el: Reset properly after countdown timer * lisp/org-timer.el (org-timer-set-timer): Reset org-timer-start-time after countdown completes. * lisp/org-timer.el (org-timer-cancel-timer): Reset org-timer-start-time when canceling countdown timer. --- lisp/org-timer.el | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lisp/org-timer.el b/lisp/org-timer.el index 1c4153c..6cecd7e 100644 --- a/lisp/org-timer.el +++ b/lisp/org-timer.el @@ -359,6 +359,7 @@ (defun org-timer-cancel-timer () (run-hooks 'org-timer-cancel-hook) (cancel-timer org-timer-current-timer) (setq org-timer-current-timer nil + org-timer-start-time nil org-timer-timer-is-countdown nil) (org-timer-set-mode-line 'off) (message "Last timer canceled"))) @@ -446,6 +447,7 @@ (defun org-timer-set-timer (&optional opt) (run-with-timer secs nil `(lambda () (setq org-timer-current-timer nil) + (setq org-timer-start-time nil) (org-notify ,(format "%s: time out" hl) ,org-clock-sound) (setq org-timer-timer-is-countdown nil) (org-timer-set-mode-line 'off) -- 2.1.3 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0004-org-timer.el-Isolate-commands-of-different-timers.patch >From 573fd67baaf17a5344fe0827641e5edb271dd599 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Tue, 2 Dec 2014 01:34:38 -0500 Subject: [PATCH 4/4] org-timer.el: Isolate commands of different timers * lisp/org-timer.el: Fix timer bugs resulting from crosstalk between commands for relative and countdown timers. Several issues were caused by relative timer functions trying to work with countdown timers. - When org-timer-start was called with a countdown timer, the modeline was updated for the new relative timer, but the countdown timer remained scheduled. - When org-timer-pause-or-continue was called with a countdown timer running, the modeline was put in a paused state, but the countdown timer remained scheduled. - When org-timer-stop was called with a countdown timer running, the timer was removed from the modeline, but the countdown timer remained scheduled. Another issue was due to a countdown timer function trying to work with a relative timer. - When org-timer-set-timer was called with a paused relative timer, the relative timer was not reset properly (org-timer-pause-time was still non-nil) and the modeline remained in the paused state of the relative timer, even though the countdown timer was scheduled with run-with-timer. To prevent all these issues, refuse to execute relative timer commands if a countdown timer is running, and vice versa. --- lisp/org-timer.el | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/lisp/org-timer.el b/lisp/org-timer.el index 6cecd7e..147528e 100644 --- a/lisp/org-timer.el +++ b/lisp/org-timer.el @@ -93,6 +93,8 @@ (defvar org-timer-done-hook nil (defvar org-timer-cancel-hook nil "Hook run before countdown timer is canceled.") +(defvar org-timer-timer-is-countdown nil) + ;;;###autoload (defun org-timer-start (&optional offset) "Set the starting time for the relative timer to now. @@ -105,8 +107,12 @@ (defun org-timer-start (&optional offset) the amount, with the default to make the first timer string in the region 0:00:00." (interactive "P") - (if (equal offset '(16)) - (call-interactively 'org-timer-change-times-in-region) + (cond + ((equal offset '(16)) + (call-interactively 'org-timer-change-times-in-region)) + (org-timer-timer-is-countdown + (user-error "Countdown timer is running. Cancel first")) + (t (let (delta def s) (if (not offset) (setq org-timer-start-time (current-time)) @@ -129,7 +135,7 @@ (defun org-timer-start (&optional offset) (message "Timer start time set to %s, current value is %s" (format-time-string "%T" org-timer-start-time) (org-timer-secs-to-hms (or delta 0))) - (run-hooks 'org-timer-start-hook)))) + (run-hooks 'org-timer-start-hook))))) (defun org-timer-pause-or-continue (&optional stop) "Pause or continue the relative timer. @@ -137,7 +143,10 @@ (defun org-timer-pause-or-continue (&optional stop) (interactive "P") (cond (stop (org-timer-stop)) - ((not org-timer-start-time) (error "No timer is running")) + (org-timer-timer-is-countdown + (user-error "Countdown timer is running")) + ((not org-timer-start-time) + (user-error "No relative timer is running")) (org-timer-pause-time ;; timer is paused, continue (setq org-timer-start-time @@ -160,6 +169,8 @@ (defun org-timer-pause-or-continue (&optional stop) (defun org-timer-stop () "Stop the relative timer." (interactive) + (when org-timer-timer-is-countdown + (user-error "Countdown timer is running")) (run-hooks 'org-timer-stop-hook) (setq org-timer-start-time nil org-timer-pause-time nil) @@ -192,7 +203,6 @@ (defun org-timer-value-string () (org-timer-secs-to-hms (abs (floor (org-timer-seconds)))))) -(defvar org-timer-timer-is-countdown nil) (defun org-timer-seconds () (if org-timer-timer-is-countdown (- (org-float-time org-timer-start-time) @@ -354,8 +364,8 @@ (defvar org-timer-current-timer nil) (defun org-timer-cancel-timer () "Cancel the current timer." (interactive) - (if (not org-timer-current-timer) - (message "No timer to cancel") + (if (not org-timer-timer-is-countdown) + (message "No countdown timer to cancel") (run-hooks 'org-timer-cancel-hook) (cancel-timer org-timer-current-timer) (setq org-timer-current-timer nil @@ -402,6 +412,9 @@ (defun org-timer-set-timer (&optional opt) minutes in the Effort property, if any. You can ignore this by using three `C-u' prefix arguments." (interactive "P") + (when (and org-timer-start-time + (not org-timer-timer-is-countdown)) + (user-error "Relative timer is running. Stop first")) (let* ((effort-minutes (org-get-at-eol 'effort-minutes 1)) (minutes (or (and (not (equal opt '(64))) effort-minutes @@ -435,10 +448,9 @@ (defun org-timer-set-timer (&optional opt) (concat "File:" (file-name-nondirectory (buffer-file-name))))) (t (error "Not in an Org buffer")))) timer-set) - (if (or (and org-timer-current-timer - (or (equal opt '(16)) - (y-or-n-p "Replace current timer? "))) - (not org-timer-current-timer)) + (if (or (not org-timer-current-timer) + (or (equal opt '(16)) + (y-or-n-p "Replace current countdown timer? "))) (progn (require 'org-clock) (when org-timer-current-timer -- 2.1.3 --=-=-=--