emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [RFC/PATCH] Fixes for org-timer
@ 2014-12-03  6:05 Kyle Meyer
  2014-12-03 17:16 ` Nicolas Goaziou
  0 siblings, 1 reply; 9+ messages in thread
From: Kyle Meyer @ 2014-12-03  6:05 UTC (permalink / raw)
  To: Org-mode

[-- 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC/PATCH] Fixes for org-timer
  2014-12-03  6:05 [RFC/PATCH] Fixes for org-timer Kyle Meyer
@ 2014-12-03 17:16 ` Nicolas Goaziou
  2014-12-03 18:54   ` Kyle Meyer
  0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Goaziou @ 2014-12-03 17:16 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: Org-mode

Hello,

Kyle Meyer <kyle@kyleam.com> writes:

> I've attached a few patches for org-timer.el.

Thank you.

> 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.

It saves a keybinding (`org-timer' has one, but not
`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).

Great.

>     Perhaps it would alsbo 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)?

When I fixed a bug there recently, I was indeed surprised the code was
very confusing. For example, `org-timer-stop' is for relative timers
whereas `org-timer-cancel' is for countdown ones.

Instead of renaming, I think we could make both timer types use the same
API (minus, maybe, start functions). Also, some comments at the
beginning of the library could help.

AFAIK, there are no tests for timers. Adding a new "test-timer.el" with
a few tests would be nice, if you have some spare time and energy.

WDYT?


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/PATCH] Fixes for org-timer
  2014-12-03 17:16 ` Nicolas Goaziou
@ 2014-12-03 18:54   ` Kyle Meyer
  2014-12-03 21:39     ` Nicolas Goaziou
  0 siblings, 1 reply; 9+ messages in thread
From: Kyle Meyer @ 2014-12-03 18:54 UTC (permalink / raw)
  To: Org-mode

Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
[...]
>>     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.
>
> It saves a keybinding (`org-timer' has one, but not
> `org-timer-change-times-in-region').

Fair enough (I don't think it's likely to be used regularly, so I wasn't
giving that much weight).

[...]
>>     Perhaps it would alsbo 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)?
>
> When I fixed a bug there recently, I was indeed surprised the code was
> very confusing. For example, `org-timer-stop' is for relative timers
> whereas `org-timer-cancel' is for countdown ones.

Agreed. I initially went there hunting one bug.

> Instead of renaming, I think we could make both timer types use the same
> API (minus, maybe, start functions).

Yes, I think this is a good way to go.

As you mention, the start functions (org-timer, org-timer-start, and
org-timer-set-timer) would probably be difficult to merge given the
arguments they take.  org-timer-stop and org-timer-cancel should be
simple to merge.  Currently, there is no function to pause the countdown
timer, so I'll need to teach org-timer-pause-or-continue how to do that.

Do you prefer to add fixes for the current design, or just move directly
to merging the two APIs?

> Also, some comments at the beginning of the library could help.
>
> AFAIK, there are no tests for timers. Adding a new "test-timer.el" with
> a few tests would be nice, if you have some spare time and energy.

Yes, both of those would be good to have, and I'm happy to add them.

Thanks for your comments.

--
Kyle

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/PATCH] Fixes for org-timer
  2014-12-03 18:54   ` Kyle Meyer
@ 2014-12-03 21:39     ` Nicolas Goaziou
  2014-12-07  8:11       ` Kyle Meyer
  0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Goaziou @ 2014-12-03 21:39 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: Org-mode

Kyle Meyer <kyle@kyleam.com> writes:

> Do you prefer to add fixes for the current design, or just move directly
> to merging the two APIs?

The latter sounds better.

>> Also, some comments at the beginning of the library could help.
>>
>> AFAIK, there are no tests for timers. Adding a new "test-timer.el" with
>> a few tests would be nice, if you have some spare time and energy.
>
> Yes, both of those would be good to have, and I'm happy to add them.
>
> Thanks for your comments.

Thank you for your work.

Regards,

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/PATCH] Fixes for org-timer
  2014-12-03 21:39     ` Nicolas Goaziou
@ 2014-12-07  8:11       ` Kyle Meyer
  2014-12-09  9:18         ` Nicolas Goaziou
  0 siblings, 1 reply; 9+ messages in thread
From: Kyle Meyer @ 2014-12-07  8:11 UTC (permalink / raw)
  To: Org-mode

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

Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> Kyle Meyer <kyle@kyleam.com> writes:
>
>> Do you prefer to add fixes for the current design, or just move
>> directly to merging the two APIs?
>
> The latter sounds better.

I've attached updated patches.


[-- 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 da0454ea110de0b4effb59ec220d8e15960fbf84 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/2] 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-Merge-API-for-the-two-timers.patch --]
[-- Type: text/x-diff, Size: 31198 bytes --]

From 981cee236e91cd40a006eb5eef43095e449b6cc9 Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kyle@kyleam.com>
Date: Sun, 7 Dec 2014 02:24:54 -0500
Subject: [PATCH 2/2] org-timer.el: Merge API for the two timers

* lisp/org-timer.el (org-timer-stop): Support countdown timers in addition
to relative timers.

* lisp/org-timer.el (org-timer-cancel-timer): Remove function.

* lisp/org-timer.el (org-timer-pause-or-continue): Support countdown
timers in addition to relative timers.

* testing/lisp/test-org-timer.el: New file.

* doc/org.texi: Merge relative and countdown timer nodes.

Several previous issues are fixed with these changes.

- org-timer-set-timer and org-timer-cancel-timer did not reset
  org-timer-start-time after countdown completed.

- Because org-timer-start did not return org-timer-pause-time to nil,
  the modeline remained stuck at the paused time.

- 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.

- 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.

- Running org-timer-set-timer at the beginning of an empty buffer
  resulted in an args-out-of-range error (due to the org-get-at-eol
  call).
---
 doc/org.texi                   |  84 ++++++-------
 etc/ORG-NEWS                   |   5 +
 lisp/org-timer.el              | 207 ++++++++++++++++++-------------
 testing/lisp/test-org-timer.el | 273 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 438 insertions(+), 131 deletions(-)
 create mode 100644 testing/lisp/test-org-timer.el

diff --git a/doc/org.texi b/doc/org.texi
index f9436b3..f7444ce 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -462,8 +462,7 @@ Dates and times
 * Deadlines and scheduling::    Planning your work
 * Clocking work time::          Tracking how long you spend on a task
 * Effort estimates::            Planning work effort in advance
-* Relative timer::              Notes with a running timer
-* Countdown timer::             Starting a countdown timer for a task
+* Timers::                      Notes with a running timer
 
 Creating timestamps
 
@@ -5790,8 +5789,7 @@ is used in a much wider sense.
 * Deadlines and scheduling::    Planning your work
 * Clocking work time::          Tracking how long you spend on a task
 * Effort estimates::            Planning work effort in advance
-* Relative timer::              Notes with a running timer
-* Countdown timer::             Starting a countdown timer for a task
+* Timers::                      Notes with a running timer
 @end menu
 
 
@@ -6794,60 +6792,56 @@ with the @kbd{/} key in the agenda (@pxref{Agenda commands}).  If you have
 these estimates defined consistently, two or three key presses will narrow
 down the list to stuff that fits into an available time slot.
 
-@node Relative timer
-@section Taking notes with a relative timer
+@node Timers
+@section Taking notes with a timer
 @cindex relative timer
+@cindex countdown timer
+@kindex ;
+
+Org provides provides two types of timers.  There is a relative timer that
+counts up, which can be useful when taking notes during, for example, a
+meeting or a video viewing.  There is also a countdown timer.
+
+The relative and countdown are started with separate commands.
+
+@table @kbd
+@orgcmd{C-c C-x 0,org-timer-start}
+Start or reset the relative timer.  By default, the timer is set to 0.  When
+called with a @kbd{C-u} prefix, prompt the user for a starting offset.  If
+there is a timer string at point, this is taken as the default, providing a
+convenient way to restart taking notes after a break in the process.  When
+called with a double prefix argument @kbd{C-u C-u}, change all timer strings
+in the active region by a certain amount.  This can be used to fix timer
+strings if the timer was not started at exactly the right moment.
+@orgcmd{C-c C-x ;,org-timer-set-timer}
+Start a countdown timer.  The user is prompted for a duration.
+@code{org-timer-default-timer} sets the default countdown value.  Giving a
+prefix numeric argument overrides this default value.  This command is
+available as @kbd{;} in agenda buffers.
+@end table
 
-When taking notes during, for example, a meeting or a video viewing, it can
-be useful to have access to times relative to a starting time.  Org provides
-such a relative timer and make it easy to create timed notes.
+Once started, relative and countdown timers are controlled with the same
+commands.
 
 @table @kbd
 @orgcmd{C-c C-x .,org-timer}
-Insert a relative time into the buffer.  The first time you use this, the
-timer will be started.  When called with a prefix argument, the timer is
-restarted.
+Insert the value of the current relative or countdown timer into the buffer.
+If no timer is running, the relative timer will be started.  When called with
+a prefix argument, the relative timer is restarted.
 @orgcmd{C-c C-x -,org-timer-item}
-Insert a description list item with the current relative time.  With a prefix
-argument, first reset the timer to 0.
+Insert a description list item with the value of the current relative or
+countdown timer.  With a prefix argument, first reset the relative timer to
+0.
 @orgcmd{M-@key{RET},org-insert-heading}
 Once the timer list is started, you can also use @kbd{M-@key{RET}} to insert
 new timer items.
-@c for key sequences with a comma, command name macros fail :(
-@kindex C-c C-x ,
-@item C-c C-x ,
-Pause the timer, or continue it if it is already paused
-(@command{org-timer-pause-or-continue}).
-@c removed the sentence because it is redundant to the following item
-@kindex C-u C-c C-x ,
-@item C-u C-c C-x ,
+@orgcmd{C-c C-x \,,org-timer-pause-or-continue}
+Pause the timer, or continue it if it is already paused.
+@orgcmd{C-c C-x _,org-timer-stop}
 Stop the timer.  After this, you can only start a new timer, not continue the
 old one.  This command also removes the timer from the mode line.
-@orgcmd{C-c C-x 0,org-timer-start}
-Reset the timer without inserting anything into the buffer.  By default, the
-timer is reset to 0.  When called with a @kbd{C-u} prefix, reset the timer to
-specific starting offset.  The user is prompted for the offset, with a
-default taken from a timer string at point, if any, So this can be used to
-restart taking notes after a break in the process.  When called with a double
-prefix argument @kbd{C-u C-u}, change all timer strings in the active region
-by a certain amount.  This can be used to fix timer strings if the timer was
-not started at exactly the right moment.
 @end table
 
-@node Countdown timer
-@section Countdown timer
-@cindex Countdown timer
-@kindex C-c C-x ;
-@kindex ;
-
-Calling @code{org-timer-set-timer} from an Org mode buffer runs a countdown
-timer.  Use @kbd{;} from agenda buffers, @key{C-c C-x ;} everywhere else.
-
-@code{org-timer-set-timer} prompts the user for a duration and displays a
-countdown timer in the modeline.  @code{org-timer-default-timer} sets the
-default countdown value.  Giving a prefix numeric argument overrides this
-default value.
-
 @node Capture - Refile - Archive
 @chapter Capture - Refile - Archive
 @cindex capture
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 5ce53c5..f719886 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -72,6 +72,8 @@ This function inserted a Beamer specific template at point or in
 current subtree.  Use ~org-export-insert-default-template~ instead, as
 it provides more features and covers all export back-ends.  It is also
 accessible from the export dispatcher.
+*** Removed function ~org-timer-cancel-timer~
+~org-timer-stop~ now stops both relative and countdown timers.
 ** Removed options
 *** ~org-list-empty-line-terminates-plain-lists~ is deprecated
 It will be kept in code base until next release, for backward
@@ -174,6 +176,9 @@ special blocks and images.  See docstring for more information.
 Headlines, for which the property ~UNNUMBERED~ is non-nil, are now
 exported without section numbers irrespective of their levels.  The
 property is inherited by children.
+*** Countdown timers can now be paused.
+~org-timer-pause-time~ wil now pause and restart both relative and
+countdown timers.
 ** Miscellaneous
 *** Strip all meta data from ITEM special property
 ITEM special property does not contain TODO, priority or tags anymore.
diff --git a/lisp/org-timer.el b/lisp/org-timer.el
index c9710ca..f35e8be 100644
--- a/lisp/org-timer.el
+++ b/lisp/org-timer.el
@@ -1,4 +1,4 @@
-;;; org-timer.el --- The relative timer code for Org-mode
+;;; org-timer.el --- Timer code for Org mode
 
 ;; Copyright (C) 2008-2014 Free Software Foundation, Inc.
 
@@ -24,13 +24,20 @@
 ;;
 ;;; Commentary:
 
-;; This file contains the relative timer code for Org-mode
+;; This file implements two types of timers for Org buffers:
+;;
+;; - A relative timer that counts up (from 0 or a specified offset)
+;; - A countdown timer that counts down from a specified time
+;;
+;; The relative and countdown timers differ in their entry points.
+;; Use `org-timer' or `org-timer-start' to start the relative timer,
+;; and `org-timer-set-timer' to start the countdown timer.
 
 ;;; Code:
 
 (require 'org)
+(require 'org-clock)
 
-(declare-function org-notify "org-clock" (notification &optional play-sound))
 (declare-function org-agenda-error "org-agenda" ())
 
 (defvar org-timer-start-time nil
@@ -39,13 +46,22 @@ (defvar org-timer-start-time nil
 (defvar org-timer-pause-time nil
   "Time when the timer was paused.")
 
+(defvar org-timer-countdown-timer nil
+  "Current countdown timer.
+This is a timer object if there is an active countdown timer,
+'paused' if there is a paused countdown timer, and nil
+otherwise.")
+
+(defvar org-timer-countdown-timer-title nil
+  "Title for notification displayed when a countdown finishes.")
+
 (defconst org-timer-re "\\([-+]?[0-9]+\\):\\([0-9]\\{2\\}\\):\\([0-9]\\{2\\}\\)"
   "Regular expression used to match timer stamps.")
 
 (defcustom org-timer-format "%s "
   "The format to insert the time of the timer.
 This format must contain one instance of \"%s\" which will be replaced by
-the value of the relative timer."
+the value of the timer."
   :group 'org-time
   :type 'string)
 
@@ -76,13 +92,13 @@ (defvar org-timer-start-hook nil
   "Hook run after relative timer is started.")
 
 (defvar org-timer-stop-hook nil
-  "Hook run before relative timer is stopped.")
+  "Hook run before relative or countdown timer is stopped.")
 
 (defvar org-timer-pause-hook nil
-  "Hook run before relative timer is paused.")
+  "Hook run before relative or countdown timer is paused.")
 
 (defvar org-timer-continue-hook nil
-  "Hook run after relative timer is continued.")
+  "Hook run after relative or countdown timer is continued.")
 
 (defvar org-timer-set-hook nil
   "Hook run after countdown timer is set.")
@@ -90,9 +106,6 @@ (defvar org-timer-set-hook nil
 (defvar org-timer-done-hook nil
   "Hook run after countdown timer reaches zero.")
 
-(defvar org-timer-cancel-hook nil
-  "Hook run before countdown timer is canceled.")
-
 ;;;###autoload
 (defun org-timer-start (&optional offset)
   "Set the starting time for the relative timer to now.
@@ -105,8 +118,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-countdown-timer
+    (user-error "Countdown timer is running.  Cancel first"))
+   (t
     (let (delta def s)
       (if (not offset)
 	  (setq org-timer-start-time (current-time))
@@ -123,45 +140,66 @@ (defun org-timer-start (&optional offset)
 	  (setq delta (org-timer-hms-to-secs (org-timer-fix-incomplete s)))))
 	(setq org-timer-start-time
 	      (seconds-to-time
-	       (- (org-float-time) delta))))
+	       ;; Pass `current-time' result to `org-float-time'
+	       ;; (instead of calling without arguments) so that only
+	       ;; `current-time' has to be overriden in tests.
+	       (- (org-float-time (current-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)
 	       (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.
+  "Pause or continue the relative or countdown timer.
 With prefix arg STOP, stop it entirely."
   (interactive "P")
   (cond
    (stop (org-timer-stop))
    ((not org-timer-start-time) (error "No timer is running"))
    (org-timer-pause-time
-    ;; timer is paused, continue
-    (setq org-timer-start-time
-	  (seconds-to-time
-	   (-
-	    (org-float-time)
-	    (- (org-float-time org-timer-pause-time)
-	       (org-float-time org-timer-start-time))))
-	  org-timer-pause-time nil)
-    (org-timer-set-mode-line 'on)
-    (run-hooks 'org-timer-continue-hook)
-    (message "Timer continues at %s" (org-timer-value-string)))
+    (let ((start-secs (org-float-time org-timer-start-time))
+	  (pause-secs (org-float-time org-timer-pause-time)))
+      (if org-timer-countdown-timer
+	  (progn
+	    (let ((new-secs (- start-secs pause-secs)))
+	      (setq org-timer-countdown-timer
+		    (org-timer--run-countdown-timer
+		     new-secs org-timer-countdown-timer-title))
+	      (setq org-timer-start-time
+		    (time-add (current-time) (seconds-to-time new-secs)))))
+	(setq org-timer-start-time
+	      ;; Pass `current-time' result to `org-float-time'
+	      ;; (instead of calling without arguments) so that only
+	      ;; `current-time' has to be overriden in tests.
+	      (seconds-to-time (- (org-float-time (current-time))
+				  (- pause-secs start-secs)))))
+      (setq org-timer-pause-time nil)
+      (org-timer-set-mode-line 'on)
+      (run-hooks 'org-timer-continue-hook)
+      (message "Timer continues at %s" (org-timer-value-string))))
    (t
     ;; pause timer
+    (when org-timer-countdown-timer
+      (cancel-timer org-timer-countdown-timer)
+      (setq org-timer-countdown-timer 'pause))
     (run-hooks 'org-timer-pause-hook)
     (setq org-timer-pause-time (current-time))
     (org-timer-set-mode-line 'pause)
     (message "Timer paused at %s" (org-timer-value-string)))))
 
 (defun org-timer-stop ()
-  "Stop the relative timer."
+  "Stop the relative or countdown timer."
   (interactive)
+  (unless org-timer-start-time
+    (user-error "No timer running"))
+  (when (timerp org-timer-countdown-timer)
+    (cancel-timer org-timer-countdown-timer))
   (run-hooks 'org-timer-stop-hook)
   (setq org-timer-start-time nil
-	org-timer-pause-time nil)
+	org-timer-pause-time nil
+	org-timer-countdown-timer nil)
   (org-timer-set-mode-line 'off)
   (message "Timer stopped"))
 
@@ -191,11 +229,10 @@ (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
+  (if org-timer-countdown-timer
       (- (org-float-time org-timer-start-time)
-	 (org-float-time (current-time)))
+	 (org-float-time (or org-timer-pause-time (current-time))))
     (- (org-float-time (or org-timer-pause-time (current-time)))
        (org-float-time org-timer-start-time))))
 
@@ -290,7 +327,7 @@ (defvar org-timer-mode-line-timer nil)
 (defvar org-timer-mode-line-string nil)
 
 (defun org-timer-set-mode-line (value)
-  "Set the mode-line display of the relative timer.
+  "Set the mode-line display for relative or countdown timer.
 VALUE can be `on', `off', or `pause'."
   (when (or (eq org-timer-display 'mode-line)
 	    (eq org-timer-display 'both))
@@ -349,35 +386,20 @@ (defun org-timer-update-mode-line ()
 	  (concat " <" (substring (org-timer-value-string) 0 -1) ">"))
     (force-mode-line-update)))
 
-(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")
-    (run-hooks 'org-timer-cancel-hook)
-    (cancel-timer org-timer-current-timer)
-    (setq org-timer-current-timer nil
-	  org-timer-timer-is-countdown nil)
-    (org-timer-set-mode-line 'off)
-    (message "Last timer canceled")))
-
 (defun org-timer-show-remaining-time ()
   "Display the remaining time before the timer ends."
   (interactive)
   (require 'time)
-  (if (not org-timer-current-timer)
+  (if (not org-timer-countdown-timer)
       (message "No timer set")
     (let* ((rtime (decode-time
-		   (time-subtract (timer--time org-timer-current-timer)
+		   (time-subtract (timer--time org-timer-countdown-timer)
 				  (current-time))))
 	   (rsecs (nth 0 rtime))
 	   (rmins (nth 1 rtime)))
       (message "%d minute(s) %d seconds left before next time out"
 	       rmins rsecs))))
 
-(defvar org-clock-sound)
-
 ;;;###autoload
 (defun org-timer-set-timer (&optional opt)
   "Prompt for a duration and set a timer.
@@ -400,7 +422,10 @@ (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")
-  (let* ((effort-minutes (org-get-at-eol 'effort-minutes 1))
+  (when (and org-timer-start-time
+	     (not org-timer-countdown-timer))
+    (user-error "Relative timer is running.  Stop first"))
+  (let* ((effort-minutes (ignore-errors (org-get-at-eol 'effort-minutes 1)))
 	 (minutes (or (and (not (equal opt '(64)))
 			   effort-minutes
 			   (number-to-string effort-minutes))
@@ -415,47 +440,57 @@ (defun org-timer-set-timer (&optional opt)
 	(org-timer-show-remaining-time)
       (let* ((mins (string-to-number (match-string 0 minutes)))
 	     (secs (* mins 60))
-	     (hl (cond
-		  ((string-match "Org Agenda" (buffer-name))
-		   (let* ((marker (or (get-text-property (point) 'org-marker)
-				      (org-agenda-error)))
-			  (hdmarker (or (get-text-property (point) 'org-hd-marker)
-					marker))
-			  (pos (marker-position marker)))
-		     (with-current-buffer (marker-buffer marker)
-		       (widen)
-		       (goto-char pos)
-		       (org-show-entry)
-		       (or (ignore-errors (org-get-heading))
-			   (concat "File:" (file-name-nondirectory (buffer-file-name)))))))
-		  ((derived-mode-p 'org-mode)
-		   (or (ignore-errors (org-get-heading))
-		       (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))
+	     (hl (org-timer--get-timer-title)))
+	(if (or (not org-timer-countdown-timer)
+		(equal opt '(16))
+		(y-or-n-p "Replace current timer? "))
 	    (progn
-	      (require 'org-clock)
-	      (when org-timer-current-timer
-		(cancel-timer org-timer-current-timer))
-	      (setq org-timer-current-timer
-		    (run-with-timer
-		     secs nil `(lambda ()
-				 (setq org-timer-current-timer nil)
-				 (org-notify ,(format "%s: time out" hl) ,org-clock-sound)
-				 (setq org-timer-timer-is-countdown nil)
-				 (org-timer-set-mode-line 'off)
-				 (run-hooks 'org-timer-done-hook))))
+	      (when (timerp org-timer-countdown-timer)
+		(cancel-timer org-timer-countdown-timer))
+	      (setq org-timer-countdown-timer-title
+		    (org-timer--get-timer-title))
+	      (setq org-timer-countdown-timer
+		    (org-timer--run-countdown-timer
+		     secs org-timer-countdown-timer-title))
 	      (run-hooks 'org-timer-set-hook)
-	      (setq org-timer-timer-is-countdown t
-		    org-timer-start-time
-		    (time-add (current-time) (seconds-to-time (* mins 60))))
+	      (setq org-timer-start-time
+		    (time-add (current-time) (seconds-to-time secs)))
+	      (setq org-timer-pause-time nil)
 	      (org-timer-set-mode-line 'on))
 	  (message "No timer set"))))))
 
+(defun org-timer--run-countdown-timer (secs title)
+  "Start countdown timer that will last SECS.
+TITLE will be appended to the notification message displayed when
+time is up."
+  (let ((msg (format "%s: time out" title)))
+    (run-with-timer
+     secs nil `(lambda ()
+		 (setq org-timer-countdown-timer nil
+		       org-timer-start-time nil)
+		 (org-notify ,msg ,org-clock-sound)
+		 (org-timer-set-mode-line 'off)
+		 (run-hooks 'org-timer-done-hook)))))
+
+(defun org-timer--get-timer-title ()
+  "Construct timer title from heading or file name of Org buffer."
+  (cond
+   ((string-match "Org Agenda" (buffer-name))
+    (let* ((marker (or (get-text-property (point) 'org-marker)
+		       (org-agenda-error)))
+	   (hdmarker (or (get-text-property (point) 'org-hd-marker)
+			 marker)))
+      (with-current-buffer (marker-buffer marker)
+	(org-with-wide-buffer
+	 (goto-char hdmarker)
+	 (org-show-entry)
+	 (or (ignore-errors (org-get-heading))
+	     (buffer-name))))))
+   ((derived-mode-p 'org-mode)
+    (or (ignore-errors (org-get-heading))
+	(buffer-name)))
+   (t (error "Not in an Org buffer"))))
+
 (provide 'org-timer)
 
 ;; Local variables:
diff --git a/testing/lisp/test-org-timer.el b/testing/lisp/test-org-timer.el
new file mode 100644
index 0000000..6409b12
--- /dev/null
+++ b/testing/lisp/test-org-timer.el
@@ -0,0 +1,273 @@
+;;; test-org-timer.el --- Tests for org-timer.el
+
+;; Copyright (C) 2014  Kyle Meyer
+
+;; Author: Kyle Meyer <kyle@kyleam.com>
+
+;; This file is not part of GNU Emacs.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(defmacro test-org-timer/with-temp-text (text &rest body)
+  "Like `org-test-with-temp-text', but set timer-specific variables.
+Also, mute output from `message'."
+  (declare (indent 1))
+  `(cl-letf (((symbol-function 'message) (lambda (&rest args) nil)))
+     (org-test-with-temp-text ,text
+       (let (org-timer-start-time
+	     org-timer-pause-time
+	     org-timer-countdown-timer
+	     org-timer-display)
+	 (unwind-protect (progn ,@body)
+	   (when (timerp org-timer-countdown-timer)
+	     (cancel-timer org-timer-countdown-timer)))))))
+
+(defmacro test-org-timer/with-current-time (time &rest body)
+  "Run BODY, setting `current-time' output to TIME."
+  (declare (indent 1))
+  `(cl-letf (((symbol-function 'current-time) (lambda () ,time)))
+     ,@body))
+
+\f
+;;; Time conversion and formatting
+
+(ert-deftest test-org-timer/secs-to-hms ()
+  "Test conversion between HMS format and seconds."
+  ;; Seconds to HMS, and back again
+  (should
+   (equal "0:00:30"
+	  (org-timer-secs-to-hms 30)))
+  (should
+   (equal 30
+	  (org-timer-hms-to-secs (org-timer-secs-to-hms 30))))
+  ;; Minutes to HMS, and back again
+  (should
+   (equal "0:02:10"
+	  (org-timer-secs-to-hms 130)))
+  (should
+   (equal 130
+	  (org-timer-hms-to-secs (org-timer-secs-to-hms 130))))
+  ;; Hours to HMS, and back again
+  (should
+   (equal "1:01:30"
+	  (org-timer-secs-to-hms 3690)))
+  (should
+   (equal 3690
+	  (org-timer-hms-to-secs (org-timer-secs-to-hms 3690))))
+  ;; Negative seconds to HMS, and back again
+  (should
+   (equal "-1:01:30"
+	  (org-timer-secs-to-hms -3690)))
+  (should
+   (equal -3690
+	  (org-timer-hms-to-secs (org-timer-secs-to-hms -3690)))))
+
+(ert-deftest test-org-timer/fix-incomplete ()
+  "Test conversion to complete HMS format."
+  ;; No fix is needed.
+  (should
+   (equal "1:02:03"
+	  (org-timer-fix-incomplete "1:02:03")))
+  ;; Hour is missing.
+  (should
+   (equal "0:02:03"
+	  (org-timer-fix-incomplete "02:03")))
+  ;; Minute is missing.
+  (should
+   (equal "0:00:03"
+	  (org-timer-fix-incomplete "03"))))
+
+(ert-deftest test-org-timer/change-times ()
+  "Test changing HMS format by offset."
+  ;; Add time.
+  (should
+   (equal "
+1:31:15
+4:00:55"
+	  (org-test-with-temp-text "
+0:00:25
+2:30:05"
+	    (org-timer-change-times-in-region (point-min) (point-max)
+					      "1:30:50")
+	    (buffer-string))))
+  ;; Subtract time.
+  (should
+   (equal "
+-1:30:25
+0:59:15"
+	  (org-test-with-temp-text "
+0:00:25
+2:30:05"
+	    (org-timer-change-times-in-region (point-min) (point-max)
+					      "-1:30:50")
+	    (buffer-string)))))
+
+\f
+;;; Timers
+
+;; Dummy times for overriding `current-time'
+(defvar test-org-timer/time0 '(21635 62793 797149 675000))
+;; Add 3 minutes and 26 seconds.
+(defvar test-org-timer/time1
+  (time-add test-org-timer/time0 (seconds-to-time 206)))
+;; Add 2 minutes and 41 seconds (6 minutes and 7 seconds total).
+(defvar test-org-timer/time2
+  (time-add test-org-timer/time1 (seconds-to-time 161)))
+;; Add 4 minutes and 55 seconds (11 minutes and 2 seconds total).
+(defvar test-org-timer/time3
+  (time-add test-org-timer/time2 (seconds-to-time 295)))
+
+(ert-deftest test-org-timer/start-relative ()
+  "Test starting relative timer."
+  ;; Insert plain timer string, starting with `org-timer-start'.
+  (should
+   (equal "0:03:26"
+	  (test-org-timer/with-temp-text ""
+	    (test-org-timer/with-current-time test-org-timer/time0
+	      (org-timer-start))
+	    (test-org-timer/with-current-time test-org-timer/time1
+	      (org-timer))
+	    (org-trim (buffer-string)))))
+  ;; Insert item timer string.
+  (should
+   (equal "- 0:03:26 ::"
+	  (test-org-timer/with-temp-text ""
+	    (test-org-timer/with-current-time test-org-timer/time0
+	      (org-timer-start))
+	    (test-org-timer/with-current-time test-org-timer/time1
+	      (org-timer-item))
+	    (org-trim (buffer-string)))))
+  ;; Start with `org-timer'.
+  (should
+   (equal "0:00:00 0:03:26"
+	  (test-org-timer/with-temp-text ""
+	    (test-org-timer/with-current-time test-org-timer/time0
+	      (org-timer))
+	    (test-org-timer/with-current-time test-org-timer/time1
+	      (org-timer))
+	    (org-trim (buffer-string)))))
+  ;; Restart with `org-timer'.
+  (should
+   (equal "0:00:00"
+	  (test-org-timer/with-temp-text ""
+	    (test-org-timer/with-current-time test-org-timer/time0
+	      (org-timer-start))
+	    (test-org-timer/with-current-time test-org-timer/time1
+	      (org-timer '(4)))
+	    (org-trim (buffer-string))))))
+
+(ert-deftest test-org-timer/set-timer ()
+  "Test setting countdown timer."
+  (should
+   (equal "0:06:34"
+	  (test-org-timer/with-temp-text ""
+	    (test-org-timer/with-current-time test-org-timer/time0
+	      (org-timer-set-timer 10))
+	    (test-org-timer/with-current-time test-org-timer/time1
+	      (org-timer))
+	    (org-trim (buffer-string))))))
+
+(ert-deftest test-org-timer/pause-timer ()
+  "Test pausing relative and countdown timers."
+  ;; Pause relative timer.
+  (should
+   (equal "0:03:26"
+	  (test-org-timer/with-temp-text ""
+	    (test-org-timer/with-current-time test-org-timer/time0
+	      (org-timer-start))
+	    (test-org-timer/with-current-time test-org-timer/time1
+	      (org-timer-pause-or-continue))
+	    (org-timer)
+	    (org-trim (buffer-string)))))
+  ;; Pause then continue relative timer.
+  (should
+   (equal "0:08:21"
+	  (test-org-timer/with-temp-text ""
+	    (test-org-timer/with-current-time test-org-timer/time0
+	      (org-timer-start))
+	    (test-org-timer/with-current-time test-org-timer/time1
+	      (org-timer-pause-or-continue))
+	    (test-org-timer/with-current-time test-org-timer/time2
+	      (org-timer-pause-or-continue))
+	    (test-org-timer/with-current-time test-org-timer/time3
+	      (org-timer))
+	    (org-trim (buffer-string)))))
+  ;; Pause then continue countdown timer.
+  (should
+   (equal "0:01:39"
+	  (test-org-timer/with-temp-text ""
+	    (test-org-timer/with-current-time test-org-timer/time0
+	      (org-timer-set-timer 10))
+	    (test-org-timer/with-current-time test-org-timer/time1
+	      (org-timer-pause-or-continue))
+	    (test-org-timer/with-current-time test-org-timer/time2
+	      (org-timer-pause-or-continue))
+	    (test-org-timer/with-current-time test-org-timer/time3
+	      (org-timer))
+	    (org-trim (buffer-string))))))
+
+(ert-deftest test-org-timer/stop ()
+  "Test stopping relative and countdown timers."
+  ;; Stop running relative timer.
+  (test-org-timer/with-temp-text ""
+    (test-org-timer/with-current-time test-org-timer/time0
+      (org-timer-start))
+    (test-org-timer/with-current-time test-org-timer/time1
+      (org-timer-stop))
+    (should-not org-timer-start-time))
+  ;; Stop paused relative timer.
+  (test-org-timer/with-temp-text ""
+    (test-org-timer/with-current-time test-org-timer/time0
+      (org-timer-start))
+    (test-org-timer/with-current-time test-org-timer/time1
+      (org-timer-pause-or-continue)
+      (org-timer-stop))
+    (should-not org-timer-start-time)
+    (should-not org-timer-pause-time))
+  ;; Stop running countdown timer.
+  (test-org-timer/with-temp-text ""
+    (test-org-timer/with-current-time test-org-timer/time0
+      (org-timer-set-timer 10))
+    (test-org-timer/with-current-time test-org-timer/time1
+      (org-timer-stop))
+    (should-not org-timer-start-time)
+    (should-not org-timer-countdown-timer))
+  ;; Stop paused countdown timer.
+  (test-org-timer/with-temp-text ""
+    (test-org-timer/with-current-time test-org-timer/time0
+      (org-timer-set-timer 10))
+    (test-org-timer/with-current-time test-org-timer/time1
+      (org-timer-pause-or-continue)
+      (org-timer-stop))
+    (should-not org-timer-start-time)
+    (should-not org-timer-pause-time)
+    (should-not org-timer-countdown-timer)))
+
+(ert-deftest test-org-timer/other-timer-error ()
+  "Test for error when other timer running."
+  ;; Relative timer is running.
+  (should-error
+   (test-org-timer/with-temp-text ""
+     (org-timer-start)
+     (org-timer-set-timer 10)))
+  ;; Countdown timer is running.
+  (should-error
+   (test-org-timer/with-temp-text ""
+     (org-timer-set-timer 10)
+     (org-timer-start))))
+
+(provide 'test-org-timer)
+;;; test-org-timer.el end here
-- 
2.1.3


[-- Attachment #4: Type: text/plain, Size: 11 bytes --]



-- 
Kyle

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC/PATCH] Fixes for org-timer
  2014-12-07  8:11       ` Kyle Meyer
@ 2014-12-09  9:18         ` Nicolas Goaziou
  2014-12-13  9:46           ` Achim Gratz
  0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Goaziou @ 2014-12-09  9:18 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: Org-mode

Kyle Meyer <kyle@kyleam.com> writes:

> I've attached updated patches.

Applied (with a minor change in `org-timer--get-timer-title'). Thank you.

Regards,

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/PATCH] Fixes for org-timer
  2014-12-09  9:18         ` Nicolas Goaziou
@ 2014-12-13  9:46           ` Achim Gratz
  2014-12-13 16:03             ` Nicolas Goaziou
  0 siblings, 1 reply; 9+ messages in thread
From: Achim Gratz @ 2014-12-13  9:46 UTC (permalink / raw)
  To: emacs-orgmode

Nicolas Goaziou writes:
> Applied (with a minor change in `org-timer--get-timer-title'). Thank you.

The new tests don't work on Emacs23, mainly due to the use of cl-letf.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/PATCH] Fixes for org-timer
  2014-12-13  9:46           ` Achim Gratz
@ 2014-12-13 16:03             ` Nicolas Goaziou
  2014-12-13 16:15               ` Achim Gratz
  0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Goaziou @ 2014-12-13 16:03 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Hello,

Achim Gratz <Stromeko@nexgo.de> writes:

> The new tests don't work on Emacs23, mainly due to the use of cl-letf.

I fixed the `cl-letf' issue, thank you. Is there anything else to do?

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/PATCH] Fixes for org-timer
  2014-12-13 16:03             ` Nicolas Goaziou
@ 2014-12-13 16:15               ` Achim Gratz
  0 siblings, 0 replies; 9+ messages in thread
From: Achim Gratz @ 2014-12-13 16:15 UTC (permalink / raw)
  To: emacs-orgmode

Nicolas Goaziou writes:
>> The new tests don't work on Emacs23, mainly due to the use of cl-letf.
>
> I fixed the `cl-letf' issue, thank you. Is there anything else to do?

Not with respect to this change, the tests are now all passing.

However, Emacs master seems to have changed the signature for
call-process-shell-command, so Org might need to work around this with a
new compatibility macro.

In toplevel form:
org-clock.el:1094:13:Warning: call-process-shell-command called with 6
    arguments, but accepts only 1-4



Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-12-13 16:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-03  6:05 [RFC/PATCH] Fixes for org-timer Kyle Meyer
2014-12-03 17:16 ` 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

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).