From: Kyle Meyer <kyle@kyleam.com>
To: Org-mode <emacs-orgmode@gnu.org>
Subject: [RFC/PATCH] Fixes for org-timer
Date: Wed, 03 Dec 2014 01:05:44 -0500 [thread overview]
Message-ID: <874mtdjmiv.fsf@kmlap.domain.org> (raw)
[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-timer.el-org-timer-Recognize-double-prefix.patch --]
[-- Type: text/x-diff, Size: 1220 bytes --]
From 1f4dd0e57401558a0cace1b421b1d37bca37fd15 Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kyle@kyleam.com>
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-org-timer.el-org-timer-start-Reset-on-pause.patch --]
[-- Type: text/x-diff, Size: 963 bytes --]
From 96b50113f8ddba96488bac8eefdc90e31df23415 Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kyle@kyleam.com>
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-org-timer.el-Reset-properly-after-countdown-timer.patch --]
[-- Type: text/x-diff, Size: 1282 bytes --]
From 9914c187c3b5ff1dcabfd15a4588e7dbb676231d Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kyle@kyleam.com>
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-org-timer.el-Isolate-commands-of-different-timers.patch --]
[-- Type: text/x-diff, Size: 5424 bytes --]
From 573fd67baaf17a5344fe0827641e5edb271dd599 Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kyle@kyleam.com>
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
next reply other threads:[~2014-12-03 6:03 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-03 6:05 Kyle Meyer [this message]
2014-12-03 17:16 ` [RFC/PATCH] Fixes for org-timer Nicolas Goaziou
2014-12-03 18:54 ` Kyle Meyer
2014-12-03 21:39 ` Nicolas Goaziou
2014-12-07 8:11 ` Kyle Meyer
2014-12-09 9:18 ` Nicolas Goaziou
2014-12-13 9:46 ` Achim Gratz
2014-12-13 16:03 ` Nicolas Goaziou
2014-12-13 16:15 ` Achim Gratz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874mtdjmiv.fsf@kmlap.domain.org \
--to=kyle@kyleam.com \
--cc=emacs-orgmode@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).