emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Brice Waegenire <brice.wge@gmail.com>
To: Brice Waegenire <brice.wge@gmail.com>, emacs-orgmode@gnu.org
Subject: Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
Date: Tue, 5 May 2015 16:34:29 +0200	[thread overview]
Message-ID: <CAL5NfX3+OKgxb+2pbMeAF-j8E24LE33rZ9JVjwxbbA9=a_t=ww@mail.gmail.com> (raw)
In-Reply-To: <87pp6kk7lg.fsf@nicolasgoaziou.fr>

[-- 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


  reply	other threads:[~2015-05-05 14:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAL5NfX3+OKgxb+2pbMeAF-j8E24LE33rZ9JVjwxbbA9=a_t=ww@mail.gmail.com' \
    --to=brice.wge@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).