* [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
@ 2015-04-24 12:48 Brice Waegenire
2015-04-24 16:49 ` Kyle Meyer
0 siblings, 1 reply; 12+ messages in thread
From: Brice Waegenire @ 2015-04-24 12:48 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 280 bytes --]
Hello,
I've hacked a patch that use hh:mm:ss format instead of minutes for
org-timer-set-timer. I'm really not sure I did it in the right way,
any sugestions are welcome.
Both tests test-org-timer/set-timer andtest-org-timer/pause-timer
where adapted to the new format.
Brice.
[-- Attachment #2: 0001-org-timer.el-Use-hh-mm-ss-format-instead-of-minutes.patch --]
[-- Type: text/x-diff, Size: 2645 bytes --]
From 18bf5a0c3e2a1101ece7d83a201eb8f6ac19836d Mon Sep 17 00:00:00 2001
From: Brice Waegeneire <brice.wge@gmail.com>
Date: Fri, 24 Apr 2015 14:18:45 +0200
Subject: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
* lisp/org-timer.el (org-timer-set-timer): Use hh:mm:ss format instead of minutes.
* testing/lisp/test-org-timer.el (test-org-timer/set-timer, test-org-timer/pause-timer): Change the arguments to the new format.
TINYCHANGE
---
lisp/org-timer.el | 11 ++++-------
testing/lisp/test-org-timer.el | 4 ++--
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/lisp/org-timer.el b/lisp/org-timer.el
index 0593573..4df21d2 100644
--- a/lisp/org-timer.el
+++ b/lisp/org-timer.el
@@ -429,17 +429,14 @@ using three `C-u' prefix arguments."
(minutes (or (and (not (equal opt '(64)))
effort-minutes
(number-to-string effort-minutes))
- (and (numberp opt) (number-to-string opt))
- (and (listp opt) (not (null opt))
- (number-to-string org-timer-default-timer))
+ (and (stringp opt) (prin1 opt))
(read-from-minibuffer
- "How many minutes left? "
+ "How many time left? "
(if (not (eq org-timer-default-timer 0))
- (number-to-string org-timer-default-timer))))))
+ (prin1 org-timer-default-timer))))))
(if (not (string-match "[0-9]+" minutes))
(org-timer-show-remaining-time)
- (let* ((mins (string-to-number (match-string 0 minutes)))
- (secs (* mins 60))
+ (let* ((secs (org-timer-hms-to-secs (org-timer-fix-incomplete minutes)))
(hl (org-timer--get-timer-title)))
(if (or (not org-timer-countdown-timer)
(equal opt '(16))
diff --git a/testing/lisp/test-org-timer.el b/testing/lisp/test-org-timer.el
index 7164a5d..a6d5e7a 100644
--- a/testing/lisp/test-org-timer.el
+++ b/testing/lisp/test-org-timer.el
@@ -175,7 +175,7 @@ Also, mute output from `message'."
(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))
+ (org-timer-set-timer "10:00"))
(test-org-timer/with-current-time test-org-timer/time1
(org-timer))
(org-trim (buffer-string))))))
@@ -210,7 +210,7 @@ Also, mute output from `message'."
(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))
+ (org-timer-set-timer "10:00"))
(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
--
2.3.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-04-24 12:48 [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes Brice Waegenire
@ 2015-04-24 16:49 ` Kyle Meyer
2015-04-29 21:52 ` Brice Waegenire
0 siblings, 1 reply; 12+ messages in thread
From: Kyle Meyer @ 2015-04-24 16:49 UTC (permalink / raw)
To: Brice Waegenire; +Cc: emacs-orgmode
Brice Waegenire <brice.wge@gmail.com> wrote:
> Hello,
>
> I've hacked a patch that use hh:mm:ss format instead of minutes for
> org-timer-set-timer. I'm really not sure I did it in the right way,
> any sugestions are welcome.
[...]
Thanks.
I think it's nice to be able to specify seconds, but now you have to
type 'N:00' (or at least 'N:0') instead of 'N' to get N minutes. Should
a plain number default to minutes? I don't use org-timer very much, so
I don't have a strong preference.
> --- a/lisp/org-timer.el
> +++ b/lisp/org-timer.el
> @@ -429,17 +429,14 @@ using three `C-u' prefix arguments."
> (minutes (or (and (not (equal opt '(64)))
> effort-minutes
> (number-to-string effort-minutes))
> - (and (numberp opt) (number-to-string opt))
> - (and (listp opt) (not (null opt))
> - (number-to-string org-timer-default-timer))
By removing the listp check, you no longer get the C-u behavior
described in the docstring.
> + (and (stringp opt) (prin1 opt))
Why not `(and (stringp opt) opt)'?
> (read-from-minibuffer
> - "How many minutes left? "
> + "How many time left? "
s/many/much/. Also, it'd be nice to specify the format in the prompt.
> (if (not (eq org-timer-default-timer 0))
> - (number-to-string org-timer-default-timer))))))
> + (prin1 org-timer-default-timer))))))
The defcustom for org-timer-default-timer still says it should be a
number. If set to a number other than 0, this will fail. Perhaps
org-timer-default-timer should be updated to be a string in the hh:mm:ss
format.
--
Kyle
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-04-24 16:49 ` Kyle Meyer
@ 2015-04-29 21:52 ` Brice Waegenire
2015-04-30 0:38 ` Kyle Meyer
2015-05-01 8:47 ` Nicolas Goaziou
0 siblings, 2 replies; 12+ messages in thread
From: Brice Waegenire @ 2015-04-29 21:52 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 4236 bytes --]
I have took in consideration all of your points, is it better now?
The current patch doesn't overwrite the present behavior of
org-set-timer it only add the possibility to use hh:mm:ss format.
2015-04-24 18:49 GMT+02:00 Kyle Meyer <kyle@kyleam.com>:
> Brice Waegenire <brice.wge@gmail.com> wrote:
>> Hello,
>>
>> I've hacked a patch that use hh:mm:ss format instead of minutes for
>> org-timer-set-timer. I'm really not sure I did it in the right way,
>> any sugestions are welcome.
> [...]
>
> Thanks.
>
> I think it's nice to be able to specify seconds, but now you have to
> type 'N:00' (or at least 'N:0') instead of 'N' to get N minutes. Should
> a plain number default to minutes? I don't use org-timer very much, so
> I don't have a strong preference.
This seems like a better behavior
>
>
>> --- a/lisp/org-timer.el
>> +++ b/lisp/org-timer.el
>> @@ -429,17 +429,14 @@ using three `C-u' prefix arguments."
>> (minutes (or (and (not (equal opt '(64)))
>> effort-minutes
>> (number-to-string effort-minutes))
>> - (and (numberp opt) (number-to-string opt))
>> - (and (listp opt) (not (null opt))
>> - (number-to-string org-timer-default-timer))
>
> By removing the listp check, you no longer get the C-u behavior
> described in the docstring.
I've re-added the C-u functionality, I didn't understood the whole
meaning of those lines.
>
>> + (and (stringp opt) (prin1 opt))
>
> Why not `(and (stringp opt) opt)'?
Because I don't really know how to program, but I was already thinking
that this prin1 function wasn't the right way do to this.
>
>> (read-from-minibuffer
>> - "How many minutes left? "
>> + "How many time left? "
>
> s/many/much/. Also, it'd be nice to specify the format in the prompt.
>
>> (if (not (eq org-timer-default-timer 0))
>> - (number-to-string org-timer-default-timer))))))
>> + (prin1 org-timer-default-timer))))))
>
> The defcustom for org-timer-default-timer still says it should be a
> number. If set to a number other than 0, this will fail. Perhaps
> org-timer-default-timer should be updated to be a string in the hh:mm:ss
> format.
>
> --
> Kyle
2015-04-24 18:49 GMT+02:00 Kyle Meyer <kyle@kyleam.com>:
> Brice Waegenire <brice.wge@gmail.com> wrote:
>> Hello,
>>
>> I've hacked a patch that use hh:mm:ss format instead of minutes for
>> org-timer-set-timer. I'm really not sure I did it in the right way,
>> any sugestions are welcome.
> [...]
>
> Thanks.
>
> I think it's nice to be able to specify seconds, but now you have to
> type 'N:00' (or at least 'N:0') instead of 'N' to get N minutes. Should
> a plain number default to minutes? I don't use org-timer very much, so
> I don't have a strong preference.
>
>
>> --- a/lisp/org-timer.el
>> +++ b/lisp/org-timer.el
>> @@ -429,17 +429,14 @@ using three `C-u' prefix arguments."
>> (minutes (or (and (not (equal opt '(64)))
>> effort-minutes
>> (number-to-string effort-minutes))
>> - (and (numberp opt) (number-to-string opt))
>> - (and (listp opt) (not (null opt))
>> - (number-to-string org-timer-default-timer))
>
> By removing the listp check, you no longer get the C-u behavior
> described in the docstring.
>
>> + (and (stringp opt) (prin1 opt))
>
> Why not `(and (stringp opt) opt)'?
>
>> (read-from-minibuffer
>> - "How many minutes left? "
>> + "How many time left? "
>
> s/many/much/. Also, it'd be nice to specify the format in the prompt.
>
>> (if (not (eq org-timer-default-timer 0))
>> - (number-to-string org-timer-default-timer))))))
>> + (prin1 org-timer-default-timer))))))
>
> The defcustom for org-timer-default-timer still says it should be a
> number. If set to a number other than 0, this will fail. Perhaps
> org-timer-default-timer should be updated to be a string in the hh:mm:ss
> format.
>
> --
> Kyle
[-- Attachment #2: 0001-org-timer.el-hh-mm-ss-format-for-setting-a-timer.patch --]
[-- Type: text/x-diff, Size: 3656 bytes --]
From 8d6e379f3ed432511c613a0cf40804d2de1764b8 Mon Sep 17 00:00:00 2001
From: Brice Waegeneire <brice.wge@gmail.com>
Date: Fri, 24 Apr 2015 14:18:45 +0200
Subject: [PATCH] org-timer.el: hh:mm:ss format for setting a timer
* lisp/org-timer.el (org-timer-set-timer): Add support for hh:mm:ss format.
* testing/lisp/test-org-timer.el (test-org-timer/set-timer): Add hh:mm:ss format in the test.
---
lisp/org-timer.el | 23 ++++++++++++-----------
testing/lisp/test-org-timer.el | 8 ++++++++
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/lisp/org-timer.el b/lisp/org-timer.el
index 0593573..022125f 100644
--- a/lisp/org-timer.el
+++ b/lisp/org-timer.el
@@ -65,12 +65,12 @@ the value of the timer."
:group 'org-time
:type 'string)
-(defcustom org-timer-default-timer 0
- "The default timer when a timer is set.
+(defcustom org-timer-default-timer "0"
+ "The default timer when a timer is set, in minutes or hh:mm:ss format.
When 0, the user is prompted for a value."
:group 'org-time
:version "24.1"
- :type 'number)
+ :type 'string)
(defcustom org-timer-display 'mode-line
"When a timer is running, org-mode can display it in the mode
@@ -402,14 +402,14 @@ VALUE can be `on', `off', or `pause'."
;;;###autoload
(defun org-timer-set-timer (&optional opt)
- "Prompt for a duration and set a timer.
+ "Prompt for a duration in minutes or hh:mm:ss and set a timer.
If `org-timer-default-timer' is not zero, suggest this value as
the default duration for the timer. If a timer is already set,
prompt the user if she wants to replace it.
Called with a numeric prefix argument, use this numeric value as
-the duration of the timer.
+the duration of the timer in minutes.
Called with a `C-u' prefix arguments, use `org-timer-default-timer'
without prompting the user for a duration.
@@ -430,16 +430,17 @@ using three `C-u' prefix arguments."
effort-minutes
(number-to-string effort-minutes))
(and (numberp opt) (number-to-string opt))
- (and (listp opt) (not (null opt))
- (number-to-string org-timer-default-timer))
+ (and (listp opt) (not (null opt)) org-timer-default-timer)
+ (and (stringp opt) opt)
(read-from-minibuffer
- "How many minutes left? "
+ "How much time left? (minutes or h:mm:ss) "
(if (not (eq org-timer-default-timer 0))
- (number-to-string org-timer-default-timer))))))
+ (eval org-timer-default-timer))))))
+ (if (string-match "^[0-9]+$" minutes)
+ (setq minutes (concat minutes ":00")))
(if (not (string-match "[0-9]+" minutes))
(org-timer-show-remaining-time)
- (let* ((mins (string-to-number (match-string 0 minutes)))
- (secs (* mins 60))
+ (let* ((secs (org-timer-hms-to-secs (org-timer-fix-incomplete minutes)))
(hl (org-timer--get-timer-title)))
(if (or (not org-timer-countdown-timer)
(equal opt '(16))
diff --git a/testing/lisp/test-org-timer.el b/testing/lisp/test-org-timer.el
index 7164a5d..8abbb85 100644
--- a/testing/lisp/test-org-timer.el
+++ b/testing/lisp/test-org-timer.el
@@ -178,6 +178,14 @@ Also, mute output from `message'."
(org-timer-set-timer 10))
(test-org-timer/with-current-time test-org-timer/time1
(org-timer))
+ (org-trim (buffer-string)))))
+ (should
+ (equal "0:00:04"
+ (test-org-timer/with-temp-text ""
+ (test-org-timer/with-current-time test-org-timer/time0
+ (org-timer-set-timer "3:30"))
+ (test-org-timer/with-current-time test-org-timer/time1
+ (org-timer))
(org-trim (buffer-string))))))
(ert-deftest test-org-timer/pause-timer ()
--
2.3.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-04-29 21:52 ` Brice Waegenire
@ 2015-04-30 0:38 ` Kyle Meyer
2015-05-01 8:47 ` Nicolas Goaziou
1 sibling, 0 replies; 12+ messages in thread
From: Kyle Meyer @ 2015-04-30 0:38 UTC (permalink / raw)
To: Brice Waegenire; +Cc: emacs-orgmode
Brice Waegenire <brice.wge@gmail.com> wrote:
> I have took in consideration all of your points, is it better now?
> The current patch doesn't overwrite the present behavior of
> org-set-timer it only add the possibility to use hh:mm:ss format.
Thanks.
> From 8d6e379f3ed432511c613a0cf40804d2de1764b8 Mon Sep 17 00:00:00 2001
> From: Brice Waegeneire <brice.wge@gmail.com>
> Date: Fri, 24 Apr 2015 14:18:45 +0200
> Subject: [PATCH] org-timer.el: hh:mm:ss format for setting a timer
>
> * lisp/org-timer.el (org-timer-set-timer): Add support for hh:mm:ss format.
>
> * testing/lisp/test-org-timer.el (test-org-timer/set-timer): Add hh:mm:ss format in the test.
Minor: ChangeLog lines tend to be filled at around 72 characters.
[...]
> (read-from-minibuffer
> - "How many minutes left? "
> + "How much time left? (minutes or h:mm:ss) "
> (if (not (eq org-timer-default-timer 0))
> - (number-to-string org-timer-default-timer))))))
> + (eval org-timer-default-timer))))))
The defcustom for org-timer-default-timer now specifies a string and is
set to "0", so `(not (eq org-timer-default-timer 0))` will return t for
the default value of org-timer-default-timer. Something like
(and (not (string= org-timer-default-timer "0"))
org-timer-default-timer)
would be needed to keep the old behavior (i.e., only insert the value of
org-timer-default-timer as the initial prompt input if the user has
changed it).
> + (if (string-match "^[0-9]+$" minutes)
> + (setq minutes (concat minutes ":00")))
Minor: `when` could be used here.
Aside from that, this looks good to me. Thoughts from Nicolas or
others?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-04-29 21:52 ` Brice Waegenire
2015-04-30 0:38 ` Kyle Meyer
@ 2015-05-01 8:47 ` Nicolas Goaziou
2015-05-05 14:34 ` Brice Waegenire
1 sibling, 1 reply; 12+ messages in thread
From: Nicolas Goaziou @ 2015-05-01 8:47 UTC (permalink / raw)
To: Brice Waegenire; +Cc: emacs-orgmode
Hello,
Brice Waegenire <brice.wge@gmail.com> writes:
> I have took in consideration all of your points, is it better now?
> The current patch doesn't overwrite the present behavior of
> org-set-timer it only add the possibility to use hh:mm:ss format.
Thank you.
Some comments follow in addition to Kyle's.
> From: Brice Waegeneire <brice.wge@gmail.com>
> Date: Fri, 24 Apr 2015 14:18:45 +0200
> Subject: [PATCH] org-timer.el: hh:mm:ss format for setting a timer
>
> * lisp/org-timer.el (org-timer-set-timer): Add support for hh:mm:ss format.
>
> * testing/lisp/test-org-timer.el (test-org-timer/set-timer): Add
> hh:mm:ss format in the test.
Commit message is incomplete, i.e., you changed default value for
`org-timer-default-timer'.
> ---
> lisp/org-timer.el | 23 ++++++++++++-----------
> testing/lisp/test-org-timer.el | 8 ++++++++
> 2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/lisp/org-timer.el b/lisp/org-timer.el
> index 0593573..022125f 100644
> --- a/lisp/org-timer.el
> +++ b/lisp/org-timer.el
> @@ -65,12 +65,12 @@ the value of the timer."
> :group 'org-time
> :type 'string)
>
> -(defcustom org-timer-default-timer 0
> - "The default timer when a timer is set.
> +(defcustom org-timer-default-timer "0"
> + "The default timer when a timer is set, in minutes or hh:mm:ss format.
> When 0, the user is prompted for a value."
> :group 'org-time
> :version "24.1"
> - :type 'number)
> + :type 'string)
Since you change default value, you need to update keywords:
:version "25.1"
:package-version '(Org . "8.3")
> + (and (listp opt) (not (null opt)) org-timer-default-timer)
(and (consp opt) org-timer-default-timer)
> (read-from-minibuffer
> - "How many minutes left? "
> + "How much time left? (minutes or h:mm:ss) "
> (if (not (eq org-timer-default-timer 0))
> - (number-to-string org-timer-default-timer))))))
> + (eval org-timer-default-timer))))))
> + (if (string-match "^[0-9]+$" minutes)
> + (setq minutes (concat minutes ":00")))
Nitpick: "\\`[0-9]+\\'"
> (if (not (string-match "[0-9]+" minutes))
> (org-timer-show-remaining-time)
> - (let* ((mins (string-to-number (match-string 0 minutes)))
> - (secs (* mins 60))
> + (let* ((secs (org-timer-hms-to-secs (org-timer-fix-incomplete minutes)))
let* -> let
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-05-01 8:47 ` Nicolas Goaziou
@ 2015-05-05 14:34 ` Brice Waegenire
2015-05-07 20:09 ` Nicolas Goaziou
0 siblings, 1 reply; 12+ messages in thread
From: Brice Waegenire @ 2015-05-05 14:34 UTC (permalink / raw)
To: Brice Waegenire, emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 2781 bytes --]
Thanks for help on this!
Here is the last version of the patch taking into account all of your comments.
2015-05-01 10:47 GMT+02:00 Nicolas Goaziou <mail@nicolasgoaziou.fr>:
> Hello,
>
> Brice Waegenire <brice.wge@gmail.com> writes:
>
>> I have took in consideration all of your points, is it better now?
>> The current patch doesn't overwrite the present behavior of
>> org-set-timer it only add the possibility to use hh:mm:ss format.
>
> Thank you.
>
> Some comments follow in addition to Kyle's.
>
>> From: Brice Waegeneire <brice.wge@gmail.com>
>> Date: Fri, 24 Apr 2015 14:18:45 +0200
>> Subject: [PATCH] org-timer.el: hh:mm:ss format for setting a timer
>>
>> * lisp/org-timer.el (org-timer-set-timer): Add support for hh:mm:ss format.
>>
>> * testing/lisp/test-org-timer.el (test-org-timer/set-timer): Add
>> hh:mm:ss format in the test.
>
> Commit message is incomplete, i.e., you changed default value for
> `org-timer-default-timer'.
>
>> ---
>> lisp/org-timer.el | 23 ++++++++++++-----------
>> testing/lisp/test-org-timer.el | 8 ++++++++
>> 2 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/lisp/org-timer.el b/lisp/org-timer.el
>> index 0593573..022125f 100644
>> --- a/lisp/org-timer.el
>> +++ b/lisp/org-timer.el
>> @@ -65,12 +65,12 @@ the value of the timer."
>> :group 'org-time
>> :type 'string)
>>
>> -(defcustom org-timer-default-timer 0
>> - "The default timer when a timer is set.
>> +(defcustom org-timer-default-timer "0"
>> + "The default timer when a timer is set, in minutes or hh:mm:ss format.
>> When 0, the user is prompted for a value."
>> :group 'org-time
>> :version "24.1"
>> - :type 'number)
>> + :type 'string)
>
> Since you change default value, you need to update keywords:
>
> :version "25.1"
> :package-version '(Org . "8.3")
>
>> + (and (listp opt) (not (null opt)) org-timer-default-timer)
>
> (and (consp opt) org-timer-default-timer)
>
>> (read-from-minibuffer
>> - "How many minutes left? "
>> + "How much time left? (minutes or h:mm:ss) "
>> (if (not (eq org-timer-default-timer 0))
>> - (number-to-string org-timer-default-timer))))))
>> + (eval org-timer-default-timer))))))
>> + (if (string-match "^[0-9]+$" minutes)
>> + (setq minutes (concat minutes ":00")))
>
> Nitpick: "\\`[0-9]+\\'"
>
>> (if (not (string-match "[0-9]+" minutes))
>> (org-timer-show-remaining-time)
>> - (let* ((mins (string-to-number (match-string 0 minutes)))
>> - (secs (* mins 60))
>> + (let* ((secs (org-timer-hms-to-secs (org-timer-fix-incomplete minutes)))
>
> let* -> let
>
>
> Regards,
>
> --
> Nicolas Goaziou
[-- Attachment #2: 0001-org-timer.el-hh-mm-ss-format-for-setting-a-timer.patch --]
[-- Type: text/x-patch, Size: 3829 bytes --]
From 07d871f2b82ab23e184305dd3e088eaf18e6a4f3 Mon Sep 17 00:00:00 2001
From: Brice Waegeneire <brice.wge@gmail.com>
Date: Fri, 24 Apr 2015 14:18:45 +0200
Subject: [PATCH] org-timer.el: hh:mm:ss format for setting a timer
* lisp/org-timer.el (org-timer-set-timer): Add support for hh:mm:ss
format.
(org-timer-default-timer): Type changed from number to string.
* testing/lisp/test-org-timer.el (test-org-timer/set-timer): Add
hh:mm:ss format in the test.
---
lisp/org-timer.el | 28 +++++++++++++++-------------
testing/lisp/test-org-timer.el | 8 ++++++++
2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/lisp/org-timer.el b/lisp/org-timer.el
index 0593573..d92f7b9 100644
--- a/lisp/org-timer.el
+++ b/lisp/org-timer.el
@@ -65,12 +65,13 @@ the value of the timer."
:group 'org-time
:type 'string)
-(defcustom org-timer-default-timer 0
- "The default timer when a timer is set.
+(defcustom org-timer-default-timer "0"
+ "The default timer when a timer is set, in minutes or hh:mm:ss format.
When 0, the user is prompted for a value."
:group 'org-time
- :version "24.1"
- :type 'number)
+ :version "25.1"
+ :package-version '(Org . "8.3")
+ :type 'string)
(defcustom org-timer-display 'mode-line
"When a timer is running, org-mode can display it in the mode
@@ -402,14 +403,14 @@ VALUE can be `on', `off', or `pause'."
;;;###autoload
(defun org-timer-set-timer (&optional opt)
- "Prompt for a duration and set a timer.
+ "Prompt for a duration in minutes or hh:mm:ss and set a timer.
If `org-timer-default-timer' is not zero, suggest this value as
the default duration for the timer. If a timer is already set,
prompt the user if she wants to replace it.
Called with a numeric prefix argument, use this numeric value as
-the duration of the timer.
+the duration of the timer in minutes.
Called with a `C-u' prefix arguments, use `org-timer-default-timer'
without prompting the user for a duration.
@@ -430,16 +431,17 @@ using three `C-u' prefix arguments."
effort-minutes
(number-to-string effort-minutes))
(and (numberp opt) (number-to-string opt))
- (and (listp opt) (not (null opt))
- (number-to-string org-timer-default-timer))
+ (and (consp opt) org-timer-default-timer)
+ (and (stringp opt) opt)
(read-from-minibuffer
- "How many minutes left? "
- (if (not (eq org-timer-default-timer 0))
- (number-to-string org-timer-default-timer))))))
+ "How much time left? (minutes or h:mm:ss) "
+ (when (not (string-equal org-timer-default-timer "0"))
+ (eval org-timer-default-timer))))))
+ (when (string-match "\\`[0-9]+\\'" minutes)
+ (setq minutes (concat minutes ":00")))
(if (not (string-match "[0-9]+" minutes))
(org-timer-show-remaining-time)
- (let* ((mins (string-to-number (match-string 0 minutes)))
- (secs (* mins 60))
+ (let ((secs (org-timer-hms-to-secs (org-timer-fix-incomplete minutes)))
(hl (org-timer--get-timer-title)))
(if (or (not org-timer-countdown-timer)
(equal opt '(16))
diff --git a/testing/lisp/test-org-timer.el b/testing/lisp/test-org-timer.el
index 7164a5d..8abbb85 100644
--- a/testing/lisp/test-org-timer.el
+++ b/testing/lisp/test-org-timer.el
@@ -178,6 +178,14 @@ Also, mute output from `message'."
(org-timer-set-timer 10))
(test-org-timer/with-current-time test-org-timer/time1
(org-timer))
+ (org-trim (buffer-string)))))
+ (should
+ (equal "0:00:04"
+ (test-org-timer/with-temp-text ""
+ (test-org-timer/with-current-time test-org-timer/time0
+ (org-timer-set-timer "3:30"))
+ (test-org-timer/with-current-time test-org-timer/time1
+ (org-timer))
(org-trim (buffer-string))))))
(ert-deftest test-org-timer/pause-timer ()
--
2.3.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-05-05 14:34 ` Brice Waegenire
@ 2015-05-07 20:09 ` Nicolas Goaziou
2015-06-02 21:04 ` Brice Waegenire
0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Goaziou @ 2015-05-07 20:09 UTC (permalink / raw)
To: Brice Waegenire; +Cc: emacs-orgmode
Brice Waegenire <brice.wge@gmail.com> writes:
> Thanks for help on this!
> Here is the last version of the patch taking into account all of your
> comments.
Thank you.
> (read-from-minibuffer
> - "How many minutes left? "
> - (if (not (eq org-timer-default-timer 0))
> - (number-to-string org-timer-default-timer))))))
> + "How much time left? (minutes or h:mm:ss) "
> + (when (not (string-equal org-timer-default-timer "0"))
Nitpick: `unless'
> + (eval org-timer-default-timer))))))
Since `org-timer-default-timer' is a string, there's no need to eval it.
BTW, did you sign FSF papers already? If not, you need to add TINYCHANGE
to the end of the commit message.
Regards,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-05-07 20:09 ` Nicolas Goaziou
@ 2015-06-02 21:04 ` Brice Waegenire
2015-06-02 21:24 ` Nicolas Goaziou
0 siblings, 1 reply; 12+ messages in thread
From: Brice Waegenire @ 2015-06-02 21:04 UTC (permalink / raw)
To: Brice Waegenire, emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]
Sorry for the long delayed response.
Here is that version of this patch, at least I hope it is.
Yes I signed the papers, my number is #1011602.
2015-05-07 22:09 GMT+02:00 Nicolas Goaziou <mail@nicolasgoaziou.fr>:
> Brice Waegenire <brice.wge@gmail.com> writes:
>
>> Thanks for help on this!
>> Here is the last version of the patch taking into account all of your
>> comments.
>
> Thank you.
>
>> (read-from-minibuffer
>> - "How many minutes left? "
>> - (if (not (eq org-timer-default-timer 0))
>> - (number-to-string org-timer-default-timer))))))
>> + "How much time left? (minutes or h:mm:ss) "
>> + (when (not (string-equal org-timer-default-timer "0"))
>
> Nitpick: `unless'
>
>> + (eval org-timer-default-timer))))))
>
> Since `org-timer-default-timer' is a string, there's no need to eval it.
>
> BTW, did you sign FSF papers already? If not, you need to add TINYCHANGE
> to the end of the commit message.
>
>
> Regards,
[-- Attachment #2: 0001-org-timer.el-hh-mm-ss-format-for-setting-a-timer.patch --]
[-- Type: text/x-patch, Size: 3825 bytes --]
From b4b22ce174dd8e1aca7de8a9c7029a6ce3f6dbd1 Mon Sep 17 00:00:00 2001
From: Brice Waegeneire <brice.wge@gmail.com>
Date: Tue, 2 Jun 2015 22:49:38 +0200
Subject: [PATCH] org-timer.el: hh:mm:ss format for setting a timer
* lisp/org-timer.el (org-timer-set-timer): Add support for hh:mm:ss
format.
(org-timer-default-timer): Type changed from number to string.
* testing/lisp/test-org-timer.el (test-org-timer/set-timer): Add
hh:mm:ss format in the test.
---
lisp/org-timer.el | 28 +++++++++++++++-------------
testing/lisp/test-org-timer.el | 8 ++++++++
2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/lisp/org-timer.el b/lisp/org-timer.el
index 0593573..d20812f 100644
--- a/lisp/org-timer.el
+++ b/lisp/org-timer.el
@@ -65,12 +65,13 @@ the value of the timer."
:group 'org-time
:type 'string)
-(defcustom org-timer-default-timer 0
- "The default timer when a timer is set.
+(defcustom org-timer-default-timer "0"
+ "The default timer when a timer is set, in minutes or hh:mm:ss format.
When 0, the user is prompted for a value."
:group 'org-time
- :version "24.1"
- :type 'number)
+ :version "25.1"
+ :package-version '(Org . "8.3")
+ :type 'string)
(defcustom org-timer-display 'mode-line
"When a timer is running, org-mode can display it in the mode
@@ -402,14 +403,14 @@ VALUE can be `on', `off', or `pause'."
;;;###autoload
(defun org-timer-set-timer (&optional opt)
- "Prompt for a duration and set a timer.
+ "Prompt for a duration in minutes or hh:mm:ss and set a timer.
If `org-timer-default-timer' is not zero, suggest this value as
the default duration for the timer. If a timer is already set,
prompt the user if she wants to replace it.
Called with a numeric prefix argument, use this numeric value as
-the duration of the timer.
+the duration of the timer in minutes.
Called with a `C-u' prefix arguments, use `org-timer-default-timer'
without prompting the user for a duration.
@@ -430,16 +431,17 @@ using three `C-u' prefix arguments."
effort-minutes
(number-to-string effort-minutes))
(and (numberp opt) (number-to-string opt))
- (and (listp opt) (not (null opt))
- (number-to-string org-timer-default-timer))
+ (and (consp opt) org-timer-default-timer)
+ (and (stringp opt) opt)
(read-from-minibuffer
- "How many minutes left? "
- (if (not (eq org-timer-default-timer 0))
- (number-to-string org-timer-default-timer))))))
+ "How much time left? (minutes or h:mm:ss) "
+ (unless (not (string-equal org-timer-default-timer "0"))
+ (org-timer-default-timer))))))
+ (when (string-match "\\`[0-9]+\\'" minutes)
+ (setq minutes (concat minutes ":00")))
(if (not (string-match "[0-9]+" minutes))
(org-timer-show-remaining-time)
- (let* ((mins (string-to-number (match-string 0 minutes)))
- (secs (* mins 60))
+ (let ((secs (org-timer-hms-to-secs (org-timer-fix-incomplete minutes)))
(hl (org-timer--get-timer-title)))
(if (or (not org-timer-countdown-timer)
(equal opt '(16))
diff --git a/testing/lisp/test-org-timer.el b/testing/lisp/test-org-timer.el
index 7164a5d..8abbb85 100644
--- a/testing/lisp/test-org-timer.el
+++ b/testing/lisp/test-org-timer.el
@@ -178,6 +178,14 @@ Also, mute output from `message'."
(org-timer-set-timer 10))
(test-org-timer/with-current-time test-org-timer/time1
(org-timer))
+ (org-trim (buffer-string)))))
+ (should
+ (equal "0:00:04"
+ (test-org-timer/with-temp-text ""
+ (test-org-timer/with-current-time test-org-timer/time0
+ (org-timer-set-timer "3:30"))
+ (test-org-timer/with-current-time test-org-timer/time1
+ (org-timer))
(org-trim (buffer-string))))))
(ert-deftest test-org-timer/pause-timer ()
--
2.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-06-02 21:04 ` Brice Waegenire
@ 2015-06-02 21:24 ` Nicolas Goaziou
2015-06-03 22:00 ` Brice Waegenire
0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Goaziou @ 2015-06-02 21:24 UTC (permalink / raw)
To: Brice Waegenire; +Cc: emacs-orgmode
Brice Waegenire <brice.wge@gmail.com> writes:
> Here is that version of this patch, at least I hope it is.
Applied, Thank you.
Would you mind preparing an entry in ORG-NEWS?
Regards,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-06-02 21:24 ` Nicolas Goaziou
@ 2015-06-03 22:00 ` Brice Waegenire
2015-06-06 7:30 ` Nicolas Goaziou
0 siblings, 1 reply; 12+ messages in thread
From: Brice Waegenire @ 2015-06-03 22:00 UTC (permalink / raw)
To: Brice Waegenire, emacs-orgmode
Following is ORG-NEWS entry:
* Incompatible changes
** org-timer-default-timer type changed from number to string
If you have, in your configuration, something like =(setq
org-timer-default-timer 10)= replace it with =(setq
org-timer-default-timer "10")=.
* New features
** Countdown timer support hh:mm:ss format
In addition to setting countdown timers in minutes, they can also be
set using the hh:mm:ss format.
2015-06-02 23:24 GMT+02:00 Nicolas Goaziou <mail@nicolasgoaziou.fr>:
> Brice Waegenire <brice.wge@gmail.com> writes:
>
>> Here is that version of this patch, at least I hope it is.
>
> Applied, Thank you.
>
> Would you mind preparing an entry in ORG-NEWS?
>
>
> Regards,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-06-03 22:00 ` Brice Waegenire
@ 2015-06-06 7:30 ` Nicolas Goaziou
2015-08-05 0:00 ` Bastien Guerry
0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Goaziou @ 2015-06-06 7:30 UTC (permalink / raw)
To: Brice Waegenire; +Cc: emacs-orgmode
Brice Waegenire <brice.wge@gmail.com> writes:
> Following is ORG-NEWS entry:
>
> * Incompatible changes
> ** org-timer-default-timer type changed from number to string
> If you have, in your configuration, something like =(setq
> org-timer-default-timer 10)= replace it with =(setq
> org-timer-default-timer "10")=.
> * New features
> ** Countdown timer support hh:mm:ss format
> In addition to setting countdown timers in minutes, they can also be
> set using the hh:mm:ss format.
Added. Thank you.
Regards,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
2015-06-06 7:30 ` Nicolas Goaziou
@ 2015-08-05 0:00 ` Bastien Guerry
0 siblings, 0 replies; 12+ messages in thread
From: Bastien Guerry @ 2015-08-05 0:00 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode, Brice Waegenire
Hi Brice,
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
> Brice Waegenire <brice.wge@gmail.com> writes:
>
>> Following is ORG-NEWS entry:
>>
>> * Incompatible changes
>> ** org-timer-default-timer type changed from number to string
>> If you have, in your configuration, something like =(setq
>> org-timer-default-timer 10)= replace it with =(setq
>> org-timer-default-timer "10")=.
>> * New features
>> ** Countdown timer support hh:mm:ss format
>> In addition to setting countdown timers in minutes, they can also be
>> set using the hh:mm:ss format.
>
> Added. Thank you.
Thanks for this feature.
FWIW, I added a ad hoc mechanism to tolerate numbers as the value of
`org-timer-default-timer'.
--
Bastien
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-08-05 0:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-24 12:48 [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes Brice Waegenire
2015-04-24 16:49 ` Kyle Meyer
2015-04-29 21:52 ` Brice Waegenire
2015-04-30 0:38 ` Kyle Meyer
2015-05-01 8:47 ` Nicolas Goaziou
2015-05-05 14:34 ` Brice Waegenire
2015-05-07 20:09 ` Nicolas Goaziou
2015-06-02 21:04 ` Brice Waegenire
2015-06-02 21:24 ` Nicolas Goaziou
2015-06-03 22:00 ` Brice Waegenire
2015-06-06 7:30 ` Nicolas Goaziou
2015-08-05 0:00 ` Bastien Guerry
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).