From: Brice Waegenire <brice.wge@gmail.com>
To: emacs-orgmode@gnu.org
Subject: Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes
Date: Wed, 29 Apr 2015 23:52:58 +0200 [thread overview]
Message-ID: <CAL5NfX2YMpwX6pSK_taJQbWKLq9bZfYUjHTqC-=RobSd2mrvzA@mail.gmail.com> (raw)
In-Reply-To: <874mo58oa5.fsf@kmlap.domain.org>
[-- 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
next prev parent reply other threads:[~2015-04-29 21:53 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 [this message]
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
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='CAL5NfX2YMpwX6pSK_taJQbWKLq9bZfYUjHTqC-=RobSd2mrvzA@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).