emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [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).