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