Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
Hi Brice, Nicolas Goaziou m...@nicolasgoaziou.fr writes: Brice Waegenire brice@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
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
Brice Waegenire brice@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,
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
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 m...@nicolasgoaziou.fr: Brice Waegenire brice@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,
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
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 m...@nicolasgoaziou.fr: Brice Waegenire brice@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, From b4b22ce174dd8e1aca7de8a9c7029a6ce3f6dbd1 Mon Sep 17 00:00:00 2001 From: Brice Waegeneire brice@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
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
Brice Waegenire brice@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,
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
Brice Waegenire brice@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,
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
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? -
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
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
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
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 k...@kyleam.com: Brice Waegenire brice@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 k...@kyleam.com: Brice Waegenire brice@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 From 8d6e379f3ed432511c613a0cf40804d2de1764b8 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. * 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
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
Brice Waegenire brice@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@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?
Re: [O] [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
Brice Waegenire brice@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
