From mboxrd@z Thu Jan 1 00:00:00 1970 From: Toby Cubitt Subject: Re: [PATCH] Separate clocksum format for durations >= 1 day Date: Wed, 14 Nov 2012 16:37:10 +0100 Message-ID: <20121114153710.GA15676@c3po> References: <877gpo1b16.fsf@gmail.com> Reply-To: Toby Cubitt Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from eggs.gnu.org ([208.118.235.92]:38567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TYf1P-0005FR-T5 for emacs-orgmode@gnu.org; Wed, 14 Nov 2012 10:37:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TYf1M-000374-Pt for emacs-orgmode@gnu.org; Wed, 14 Nov 2012 10:36:59 -0500 Received: from sanddollar.geekisp.com ([216.168.135.167]:39743) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1TYf1M-00036A-Ee for emacs-orgmode@gnu.org; Wed, 14 Nov 2012 10:36:56 -0500 Content-Disposition: inline In-Reply-To: <877gpo1b16.fsf@gmail.com> 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 On Wed, Nov 14, 2012 at 04:04:05PM +0100, Nicolas Goaziou wrote: > Toby Cubitt writes: > > Thanks for your work. Some comments below. > > > The second patch: > > - further extends org-time-clocksum-format to allow separate month and > > year components (where a month is taken to be 30 days, a year to be 365 > > days). > > I suggest to add week instead of month, as the duration of the former is > more stable and [1;52[ range is still readable. Yes, not sure why I didn't think of adding weeks. But using months and years is optional and not the default, so I see no great harm in including them too (as long as the fact that 1 month = 30 days and 1 year = 365 days here is well documented). > > The reason for retaining separate org-time-clocksum-format and > > org-time-clocksum-fractional-format's is that (i) it doesn't make much > > sense to have a list of formats for separate components when using the > > fractional format (see Nicolas' examples earlier in this discussion > > thread); > > Then, my examples weren't clear. It is useful to have a list of formats > when using fractional time as the unit used may change: > > 1.2 d or 1.2 h I can easily allow org-time-clocksum-fractional-format to be a list of formats. But "1d 3.4h" doesn't seem very useful to me. Probably it should work a bit differently: format the time as a fractional quantity, using the largest time unit which will give a non-zero integer part. Does that sound reasonable? > > I'm not wedded to new customization type I've used in > > org-time-clocksum-format. If you prefer a plist, or a different ordering > > of the format strings in the list, or a different customization ui, > > that's fine by me. > > I think a plist would be clearer. More on that below. > > > + (org-add-props (concat (format "%s " (make-string l ?*)) > > + (org-minutes-to-clocksum-string time) > > + (format "%s" (make-string (- 16 l) ?\ ))) > > Shouldn't it be: > > (org-add-props (concat (make-string l ?*) " " > (org-minutes-to-clocksum-string time) > (make-string (- 16 l) ? )) Yes. This was an artifact of converting the previous code over to org-minutes-to-clocksum-string. > > -(defcustom org-time-clocksum-format "%d:%02d" > > +(defcustom org-time-clocksum-format '(":%02d" "%d" "%dd ") ;"%d:%02d" > > "The format string used when creating CLOCKSUM lines. > > -This is also used when org-mode generates a time duration." > > +This is also used when org-mode generates a time duration. > > This is not about your patch, but while you're working in this area: in > documentation, it should be "Org mode". OK. > > +The value can be a single format string containing two > > +%-sequences, which will be filled with the number of hours and > > +minutes in that order. > > Ok, for backward compatibility. Note that, for a major release (8.0), > such changes are acceptable even without it. OK, but in this case I think the single-format-string option is still useful. It gives users a simpler way of customizing the format if they don't want to do anything fancy. > > +Alternatively, the value can be a list of up to three format > > +strings. In this case, the first format string in the list is > > +used for the number of minutes, the second for the number of > > +hours, and the third for the number of days if the duration is > > +longer than 1 day. The complete formatted duration is obtained by > > +concatenating these in the order: days, minutes, hours. > > + > > +If the list contains fewer than three format strings, it > > +restricts the largest time unit in the formatted duration to be > > +the largest one in the list. A two-element list means the > > +duration will always be expressed in hours and minutes, a > > +one-element list means the duration will always be expressed in > > +minutes." > > There, I think we would benefit from using a plist. Indeed, > a two-element list might mean that duration should be expressed in days > and hours instead. Also I suggest to report duration targeted at missing > format strings to the smaller unit. Good idea. This adds (yet more) flexibility, and it makes customization more transparent too. > So: > > '(:day nil :hour "%d" :minute ":%02d") > > will be the equivalent of the current default format string. Then we can > specify that "%d:%02d" is still available but should be deprecated. I don't see any pressing need to deprecate the old format. > It would also allow to skip months/weeks. > > > +(defalias 'org-minutes-to-hh:mm-string 'org-minutes-to-clocksum-string) > > +(make-obsolete 'org-minutes-to-hh:mm-string 'org-minutes-to-clocksum-string > > + "org-mode version 7.9.3") > > Good idea. Don't forget to fix the WHEN argument when the patch is applied. I'll post an updated patch when I get time to make the changes. Cheers, Toby -- Dr T. S. Cubitt Mathematics and Quantum Information group Department of Mathematics Complutense University Madrid, Spain email: tsc25@cantab.net web: www.dr-qubit.org