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 <m...@nicolasgoaziou.fr>:
> Hello,
>
> Brice Waegenire <brice....@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....@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
From 07d871f2b82ab23e184305dd3e088eaf18e6a4f3 Mon Sep 17 00:00:00 2001
From: Brice Waegeneire <brice....@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
