From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [PATCH] Separate clocksum format for durations >= 1 day Date: Wed, 14 Nov 2012 16:04:05 +0100 Message-ID: <877gpo1b16.fsf@gmail.com> References: <87mwyrjj7x.fsf@gmail.com> <20121113130304.GA13301@c3po> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([208.118.235.92]:42148) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TYeZk-0003nm-WF for emacs-orgmode@gnu.org; Wed, 14 Nov 2012 10:08:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TYeZh-0001bu-St for emacs-orgmode@gnu.org; Wed, 14 Nov 2012 10:08:24 -0500 Received: from mail-wg0-f49.google.com ([74.125.82.49]:48748) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TYeZh-0001bo-LW for emacs-orgmode@gnu.org; Wed, 14 Nov 2012 10:08:21 -0500 Received: by mail-wg0-f49.google.com with SMTP id gg4so199336wgb.30 for ; Wed, 14 Nov 2012 07:08:21 -0800 (PST) In-Reply-To: <20121113130304.GA13301@c3po> (Toby Cubitt's message of "Tue, 13 Nov 2012 14:03:04 +0100") 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 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. > 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'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) ? )) > -(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". > +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. > +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. 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. 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. Regards, -- Nicolas Goaziou