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