From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brice Waegenire Subject: Re: [PATCH] org-timer.el: Use hh:mm:ss format instead of minutes Date: Wed, 29 Apr 2015 23:52:58 +0200 Message-ID: References: <874mo58oa5.fsf@kmlap.domain.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=001a11c11d02a04fd40514e4009c Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:39440) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnZuf-0006eZ-Pm for emacs-orgmode@gnu.org; Wed, 29 Apr 2015 17:53:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YnZud-0000u6-Sf for emacs-orgmode@gnu.org; Wed, 29 Apr 2015 17:53:01 -0400 Received: from mail-qc0-x231.google.com ([2607:f8b0:400d:c01::231]:35388) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnZud-0000u1-Mv for emacs-orgmode@gnu.org; Wed, 29 Apr 2015 17:52:59 -0400 Received: by qcbii10 with SMTP id ii10so20342283qcb.2 for ; Wed, 29 Apr 2015 14:52:59 -0700 (PDT) In-Reply-To: <874mo58oa5.fsf@kmlap.domain.org> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: emacs-orgmode@gnu.org --001a11c11d02a04fd40514e4009c Content-Type: text/plain; charset=UTF-8 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 : > Brice Waegenire 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 : > Brice Waegenire 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 --001a11c11d02a04fd40514e4009c Content-Type: text/x-diff; charset=US-ASCII; name="0001-org-timer.el-hh-mm-ss-format-for-setting-a-timer.patch" Content-Disposition: attachment; filename="0001-org-timer.el-hh-mm-ss-format-for-setting-a-timer.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i93a3yff0 RnJvbSA4ZDZlMzc5ZjNlZDQzMjUxMWM2MTNhMGNmNDA4MDRkMmRlMTc2NGI4IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBCcmljZSBXYWVnZW5laXJlIDxicmljZS53Z2VAZ21haWwuY29t PgpEYXRlOiBGcmksIDI0IEFwciAyMDE1IDE0OjE4OjQ1ICswMjAwClN1YmplY3Q6IFtQQVRDSF0g b3JnLXRpbWVyLmVsOiBoaDptbTpzcyBmb3JtYXQgZm9yIHNldHRpbmcgYSB0aW1lcgoKKiBsaXNw L29yZy10aW1lci5lbCAob3JnLXRpbWVyLXNldC10aW1lcik6IEFkZCBzdXBwb3J0IGZvciBoaDpt bTpzcyBmb3JtYXQuCgoqIHRlc3RpbmcvbGlzcC90ZXN0LW9yZy10aW1lci5lbCAodGVzdC1vcmct dGltZXIvc2V0LXRpbWVyKTogQWRkIGhoOm1tOnNzIGZvcm1hdCBpbiB0aGUgdGVzdC4KLS0tCiBs aXNwL29yZy10aW1lci5lbCAgICAgICAgICAgICAgfCAyMyArKysrKysrKysrKystLS0tLS0tLS0t LQogdGVzdGluZy9saXNwL3Rlc3Qtb3JnLXRpbWVyLmVsIHwgIDggKysrKysrKysKIDIgZmlsZXMg Y2hhbmdlZCwgMjAgaW5zZXJ0aW9ucygrKSwgMTEgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEv bGlzcC9vcmctdGltZXIuZWwgYi9saXNwL29yZy10aW1lci5lbAppbmRleCAwNTkzNTczLi4wMjIx MjVmIDEwMDY0NAotLS0gYS9saXNwL29yZy10aW1lci5lbAorKysgYi9saXNwL29yZy10aW1lci5l bApAQCAtNjUsMTIgKzY1LDEyIEBAIHRoZSB2YWx1ZSBvZiB0aGUgdGltZXIuIgogICA6Z3JvdXAg J29yZy10aW1lCiAgIDp0eXBlICdzdHJpbmcpCiAKLShkZWZjdXN0b20gb3JnLXRpbWVyLWRlZmF1 bHQtdGltZXIgMAotICAiVGhlIGRlZmF1bHQgdGltZXIgd2hlbiBhIHRpbWVyIGlzIHNldC4KKyhk ZWZjdXN0b20gb3JnLXRpbWVyLWRlZmF1bHQtdGltZXIgIjAiCisgICJUaGUgZGVmYXVsdCB0aW1l ciB3aGVuIGEgdGltZXIgaXMgc2V0LCBpbiBtaW51dGVzIG9yIGhoOm1tOnNzIGZvcm1hdC4KIFdo ZW4gMCwgdGhlIHVzZXIgaXMgcHJvbXB0ZWQgZm9yIGEgdmFsdWUuIgogICA6Z3JvdXAgJ29yZy10 aW1lCiAgIDp2ZXJzaW9uICIyNC4xIgotICA6dHlwZSAnbnVtYmVyKQorICA6dHlwZSAnc3RyaW5n KQogCiAoZGVmY3VzdG9tIG9yZy10aW1lci1kaXNwbGF5ICdtb2RlLWxpbmUKICAgIldoZW4gYSB0 aW1lciBpcyBydW5uaW5nLCBvcmctbW9kZSBjYW4gZGlzcGxheSBpdCBpbiB0aGUgbW9kZQpAQCAt NDAyLDE0ICs0MDIsMTQgQEAgVkFMVUUgY2FuIGJlIGBvbicsIGBvZmYnLCBvciBgcGF1c2UnLiIK IAogOzs7IyMjYXV0b2xvYWQKIChkZWZ1biBvcmctdGltZXItc2V0LXRpbWVyICgmb3B0aW9uYWwg b3B0KQotICAiUHJvbXB0IGZvciBhIGR1cmF0aW9uIGFuZCBzZXQgYSB0aW1lci4KKyAgIlByb21w dCBmb3IgYSBkdXJhdGlvbiBpbiBtaW51dGVzIG9yIGhoOm1tOnNzIGFuZCBzZXQgYSB0aW1lci4K IAogSWYgYG9yZy10aW1lci1kZWZhdWx0LXRpbWVyJyBpcyBub3QgemVybywgc3VnZ2VzdCB0aGlz IHZhbHVlIGFzCiB0aGUgZGVmYXVsdCBkdXJhdGlvbiBmb3IgdGhlIHRpbWVyLiAgSWYgYSB0aW1l ciBpcyBhbHJlYWR5IHNldCwKIHByb21wdCB0aGUgdXNlciBpZiBzaGUgd2FudHMgdG8gcmVwbGFj ZSBpdC4KIAogQ2FsbGVkIHdpdGggYSBudW1lcmljIHByZWZpeCBhcmd1bWVudCwgdXNlIHRoaXMg bnVtZXJpYyB2YWx1ZSBhcwotdGhlIGR1cmF0aW9uIG9mIHRoZSB0aW1lci4KK3RoZSBkdXJhdGlv biBvZiB0aGUgdGltZXIgaW4gbWludXRlcy4KIAogQ2FsbGVkIHdpdGggYSBgQy11JyBwcmVmaXgg YXJndW1lbnRzLCB1c2UgYG9yZy10aW1lci1kZWZhdWx0LXRpbWVyJwogd2l0aG91dCBwcm9tcHRp bmcgdGhlIHVzZXIgZm9yIGEgZHVyYXRpb24uCkBAIC00MzAsMTYgKzQzMCwxNyBAQCB1c2luZyB0 aHJlZSBgQy11JyBwcmVmaXggYXJndW1lbnRzLiIKIAkJCSAgIGVmZm9ydC1taW51dGVzCiAJCQkg ICAobnVtYmVyLXRvLXN0cmluZyBlZmZvcnQtbWludXRlcykpCiAJCSAgICAgKGFuZCAobnVtYmVy cCBvcHQpIChudW1iZXItdG8tc3RyaW5nIG9wdCkpCi0JCSAgICAgKGFuZCAobGlzdHAgb3B0KSAo bm90IChudWxsIG9wdCkpCi0JCQkgIChudW1iZXItdG8tc3RyaW5nIG9yZy10aW1lci1kZWZhdWx0 LXRpbWVyKSkKKwkJICAgICAoYW5kIChsaXN0cCBvcHQpIChub3QgKG51bGwgb3B0KSkgb3JnLXRp bWVyLWRlZmF1bHQtdGltZXIpCisJCSAgICAgKGFuZCAoc3RyaW5ncCBvcHQpIG9wdCkKIAkJICAg ICAocmVhZC1mcm9tLW1pbmlidWZmZXIKLQkJICAgICAgIkhvdyBtYW55IG1pbnV0ZXMgbGVmdD8g IgorCQkgICAgICAiSG93IG11Y2ggdGltZSBsZWZ0PyAobWludXRlcyBvciBoOm1tOnNzKSAiCiAJ CSAgICAgIChpZiAobm90IChlcSBvcmctdGltZXItZGVmYXVsdC10aW1lciAwKSkKLQkJCSAgKG51 bWJlci10by1zdHJpbmcgb3JnLXRpbWVyLWRlZmF1bHQtdGltZXIpKSkpKSkKKwkJCSAgKGV2YWwg b3JnLXRpbWVyLWRlZmF1bHQtdGltZXIpKSkpKSkKKyAgICAoaWYgKHN0cmluZy1tYXRjaCAiXlsw LTldKyQiIG1pbnV0ZXMpCisJKHNldHEgbWludXRlcyAoY29uY2F0IG1pbnV0ZXMgIjowMCIpKSkK ICAgICAoaWYgKG5vdCAoc3RyaW5nLW1hdGNoICJbMC05XSsiIG1pbnV0ZXMpKQogCShvcmctdGlt ZXItc2hvdy1yZW1haW5pbmctdGltZSkKLSAgICAgIChsZXQqICgobWlucyAoc3RyaW5nLXRvLW51 bWJlciAobWF0Y2gtc3RyaW5nIDAgbWludXRlcykpKQotCSAgICAgKHNlY3MgKCogbWlucyA2MCkp CisgICAgICAobGV0KiAoKHNlY3MgKG9yZy10aW1lci1obXMtdG8tc2VjcyAob3JnLXRpbWVyLWZp eC1pbmNvbXBsZXRlIG1pbnV0ZXMpKSkKIAkgICAgIChobCAob3JnLXRpbWVyLS1nZXQtdGltZXIt dGl0bGUpKSkKIAkoaWYgKG9yIChub3Qgb3JnLXRpbWVyLWNvdW50ZG93bi10aW1lcikKIAkJKGVx dWFsIG9wdCAnKDE2KSkKZGlmZiAtLWdpdCBhL3Rlc3RpbmcvbGlzcC90ZXN0LW9yZy10aW1lci5l bCBiL3Rlc3RpbmcvbGlzcC90ZXN0LW9yZy10aW1lci5lbAppbmRleCA3MTY0YTVkLi44YWJiYjg1 IDEwMDY0NAotLS0gYS90ZXN0aW5nL2xpc3AvdGVzdC1vcmctdGltZXIuZWwKKysrIGIvdGVzdGlu Zy9saXNwL3Rlc3Qtb3JnLXRpbWVyLmVsCkBAIC0xNzgsNiArMTc4LDE0IEBAIEFsc28sIG11dGUg b3V0cHV0IGZyb20gYG1lc3NhZ2UnLiIKIAkgICAgICAob3JnLXRpbWVyLXNldC10aW1lciAxMCkp CiAJICAgICh0ZXN0LW9yZy10aW1lci93aXRoLWN1cnJlbnQtdGltZSB0ZXN0LW9yZy10aW1lci90 aW1lMQogCSAgICAgIChvcmctdGltZXIpKQorCSAgICAob3JnLXRyaW0gKGJ1ZmZlci1zdHJpbmcp KSkpKQorICAoc2hvdWxkCisgICAoZXF1YWwgIjA6MDA6MDQiCisJICAodGVzdC1vcmctdGltZXIv d2l0aC10ZW1wLXRleHQgIiIKKwkgICAgKHRlc3Qtb3JnLXRpbWVyL3dpdGgtY3VycmVudC10aW1l IHRlc3Qtb3JnLXRpbWVyL3RpbWUwCisJICAgICAgKG9yZy10aW1lci1zZXQtdGltZXIgIjM6MzAi KSkKKwkgICAgKHRlc3Qtb3JnLXRpbWVyL3dpdGgtY3VycmVudC10aW1lIHRlc3Qtb3JnLXRpbWVy L3RpbWUxCisJICAgICAgKG9yZy10aW1lcikpCiAJICAgIChvcmctdHJpbSAoYnVmZmVyLXN0cmlu ZykpKSkpKQogCiAoZXJ0LWRlZnRlc3QgdGVzdC1vcmctdGltZXIvcGF1c2UtdGltZXIgKCkKLS0g CjIuMy43Cgo= --001a11c11d02a04fd40514e4009c--