[PATCH] Separate clocksum format for durations >= 1 day

[PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

Personally, I find the time duration "123:15" much harder to parse
mentally than "5d 3:15".

The attached patch adds a new customization option
`org-time-clocksum-days-format'. When non-nil, this is used instead of
`org-time-clocksum-format' for clocksum durations longer than 1 day. It
gets passed three values: # days, # hours, # mins. (Note that you don't
have to use all three in the format if, say, you don't feel the need to
display the minutes for such long durations.)

In the patch, I've set the default value for this new customization
option to a non-nil value. If you prefer to keep the current behaviour as
the default, just make the default value nil.

Toby


PS: I guess the logical extrapolation of this is to add even more
`org-time-clocksum-[months|years|decades]-format' options. (Or, probably
better, abandon printf formats for long durations and just add an
`org-time-clocksum-format-function' option, leaving it up users to define
a function to format the time as they wish.)

I haven't done this in the patch, because I think "64d 3:15" is no harder
to parse than "2m 4d 3:15" (plus there's the thorny issue of how many
days should be in a month). And by the time you get to "535d 3:15"
vs. "2y 5d 3:15", the duration is so long that you probably don't care
much about the exact value, except that it's a very long-running task
indeed!
-- 
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

[-- Attachment #2: 0001-Allow-separate-format-for-clocksum-lines-longer-than.patch --]
[-- Type: text/x-patch, Size: 1883 bytes --]

From 2891e0500265df24461d85493e70c1d31c095408 Mon Sep 17 00:00:00 2001
From: "Toby S. Cubitt" <tsc25@cantab.net>
Date: Wed, 17 Oct 2012 20:48:41 +0200
Subject: [PATCH] Allow separate format for clocksum lines longer than 1 day.

Configured by new org-time-clocksum-days-format customization option.
---
 lisp/org-colview.el |    7 +++++--
 lisp/org.el         |    6 ++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 9d58b5f..244458f 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1058,8 +1058,11 @@ Don't set this, this is meant for dynamic scoping.")
    ((memq fmt '(estimate)) (org-estimate-print n printf))
    ((not (numberp n)) "")
    ((memq fmt '(add_times max_times min_times mean_times))
-    (let* ((h (floor n)) (m (floor (+ 0.5 (* 60 (- n h))))))
-      (format org-time-clocksum-format h m)))
+    (let* ((h (floor n)) (m (floor (+ 0.5 (* 60 (- n h))))) (d (/ h 24)))
+      (if (or (= d 0) (null org-time-clocksum-days-format))
+	  (format org-time-clocksum-format h m)
+	(setq h (- h (* 24 d)))
+	(format org-time-clocksum-days-format d h m))))
    ((eq fmt 'checkbox)
     (cond ((= n (floor n)) "[X]")
 	  ((> n 1.) "[-]")
diff --git a/lisp/org.el b/lisp/org.el
index 2aa70bd..2eb65d6 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2720,6 +2720,12 @@ This is also used when org-mode generates a time duration."
   :group 'org-time
   :type 'string)
 
+(defcustom org-time-clocksum-days-format "%dd %d:%02d"
+  "The format string used for CLOCKSUM lines longer than 1 day.
+If null, fall back to `org-time-clocksum-format'."
+  :group 'org-time
+  :type '(choice (string :tag "format") (const :tag "disabled" nil)))
+
 (defcustom org-time-clocksum-use-fractional nil
   "If non-nil, \\[org-clock-display] uses fractional times.
 org-mode generates a time duration."
-- 
1.7.8.6

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Hello,

Toby Cubitt <tsc25@cantab.net> writes:

> Personally, I find the time duration "123:15" much harder to parse
> mentally than "5d 3:15".
>
> The attached patch adds a new customization option
> `org-time-clocksum-days-format'. When non-nil, this is used instead of
> `org-time-clocksum-format' for clocksum durations longer than 1 day. It
> gets passed three values: # days, # hours, # mins. (Note that you don't
> have to use all three in the format if, say, you don't feel the need to
> display the minutes for such long durations.)
>
> In the patch, I've set the default value for this new customization
> option to a non-nil value. If you prefer to keep the current behaviour as
> the default, just make the default value nil.
>
> Toby
>
>
> PS: I guess the logical extrapolation of this is to add even more
> `org-time-clocksum-[months|years|decades]-format' options. (Or, probably
> better, abandon printf formats for long durations and just add an
> `org-time-clocksum-format-function' option, leaving it up users to define
> a function to format the time as they wish.)
>
> I haven't done this in the patch, because I think "64d 3:15" is no harder
> to parse than "2m 4d 3:15" (plus there's the thorny issue of how many
> days should be in a month). And by the time you get to "535d 3:15"
> vs. "2y 5d 3:15", the duration is so long that you probably don't care
> much about the exact value, except that it's a very long-running task
> indeed!

Thanks for your patch.

I like the idea, but it would be better to avoid introducing yet another
defcustom for this. There is already:

  - org-time-clocksum-format
  - org-time-clocksum-use-fractional
  - org-time-clocksum-fractional-format

As you suggest, I think a better plan is to replace all of them with
a single `org-time-clocksum-display-function'. Its expected value would
be a function accepting 2 arguments: hours and minutes, as numbers and
it should return a string.

We can also provide default functions for current behaviour (i.e.
fractional time and Hs:MM) and for the one you suggest.

It's more work, but it simplifies the whole thing in the end.

What do you think? Do you want to give it a try?


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Mon, Nov 05, 2012 at 10:14:27AM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> 
> > Personally, I find the time duration "123:15" much harder to parse
> > mentally than "5d 3:15".
> >
> > The attached patch adds a new customization option
> > `org-time-clocksum-days-format'. When non-nil, this is used instead of
> > `org-time-clocksum-format' for clocksum durations longer than 1 day. It
> > gets passed three values: # days, # hours, # mins. (Note that you don't
> > have to use all three in the format if, say, you don't feel the need to
> > display the minutes for such long durations.)
> >
> > In the patch, I've set the default value for this new customization
> > option to a non-nil value. If you prefer to keep the current behaviour as
> > the default, just make the default value nil.
> >
> > Toby
> >
> >
> > PS: I guess the logical extrapolation of this is to add even more
> > `org-time-clocksum-[months|years|decades]-format' options. (Or, probably
> > better, abandon printf formats for long durations and just add an
> > `org-time-clocksum-format-function' option, leaving it up users to define
> > a function to format the time as they wish.)
> >
> > I haven't done this in the patch, because I think "64d 3:15" is no harder
> > to parse than "2m 4d 3:15" (plus there's the thorny issue of how many
> > days should be in a month). And by the time you get to "535d 3:15"
> > vs. "2y 5d 3:15", the duration is so long that you probably don't care
> > much about the exact value, except that it's a very long-running task
> > indeed!
> 
> Thanks for your patch.
> 
> I like the idea, but it would be better to avoid introducing yet another
> defcustom for this. There is already:
> 
>   - org-time-clocksum-format
>   - org-time-clocksum-use-fractional
>   - org-time-clocksum-fractional-format
> 
> As you suggest, I think a better plan is to replace all of them with
> a single `org-time-clocksum-display-function'. Its expected value would
> be a function accepting 2 arguments: hours and minutes, as numbers and
> it should return a string.
> 
> We can also provide default functions for current behaviour (i.e.
> fractional time and Hs:MM) and for the one you suggest.
> 
> It's more work, but it simplifies the whole thing in the end.
> 
> What do you think? Do you want to give it a try?

Sounds like a good plan, and it won't be very difficult to implement.

I'm happy to have a go, but I'm somewhat short on spare time just at the
moment, so it might take me a few weeks to get around to it.

Best,

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Achim Gratz]

Nicolas Goaziou writes:
> As you suggest, I think a better plan is to replace all of them with
> a single `org-time-clocksum-display-function'. Its expected value would
> be a function accepting 2 arguments: hours and minutes, as numbers and
> it should return a string.

Actually, it seems an even better plan IMHO to just have one single
function that takes a format string and the total time in minutes or
seconds and allow customization of the format string.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Mon, Nov 05, 2012 at 11:47:12AM +0100, Achim Gratz wrote:
> Nicolas Goaziou writes:
> > As you suggest, I think a better plan is to replace all of them with
> > a single `org-time-clocksum-display-function'. Its expected value would
> > be a function accepting 2 arguments: hours and minutes, as numbers and
> > it should return a string.
> 
> Actually, it seems an even better plan IMHO to just have one single
> function that takes a format string and the total time in minutes or
> seconds and allow customization of the format string.

A format string isn't sufficient. It requires the number of time
components (days, hours, minutes, etc.) to be fixed in advance. Whereas a
function can decide whether to display e.g. days+hours or hours+minutes
depending on whether the time is longer or shorter than 24h.

A defcustom that can either be a format string *or* a function might be
an option.

Or maybe I've misunderstood what you're proposing?

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Achim Gratz]

Toby Cubitt writes:
> A format string isn't sufficient. It requires the number of time
> components (days, hours, minutes, etc.) to be fixed in advance. Whereas a
> function can decide whether to display e.g. days+hours or hours+minutes
> depending on whether the time is longer or shorter than 24h.

You can define the format specification with any semantics you want,
including alternatives like those suggested above.

> A defcustom that can either be a format string *or* a function might be
> an option.

What I'm saying is that this whole business of exposing formatting
functions to the user is somewhat superfluous.  Whether different formats
dispatch their work to different (internal) functions is another matter
— if it makes implementation easier, just do it.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf microQ V2.22R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Mon, Nov 05, 2012 at 12:13:25PM +0100, Achim Gratz wrote:
> Toby Cubitt writes:
> > A format string isn't sufficient. It requires the number of time
> > components (days, hours, minutes, etc.) to be fixed in advance. Whereas a
> > function can decide whether to display e.g. days+hours or hours+minutes
> > depending on whether the time is longer or shorter than 24h.
> 
> You can define the format specification with any semantics you want,
> including alternatives like those suggested above.

OK, I get it now. I was interpreting "format specification" to mean
format string, but that's not what you meant.

> > A defcustom that can either be a format string *or* a function might be
> > an option.
> 
> What I'm saying is that this whole business of exposing formatting
> functions to the user is somewhat superfluous.

But defining a custom format semantics just for this one rather simple
case also seems somewhat superfluous. We already have a clean,
well-known, well-understood semantics for expressing conditionals: Elisp.

I'm not sure we've hit upon the clean solution yet...

Looking through the code, it seems the clocksum format options are used
in two places: org-colview.el and org-clock.el. For some reason, only the
latter honours `org-time-clocksum-use-fractional'. In my patch, only the
former honoured the new `org-time-clocksum-days-format'. Some
rationalisation of all these options is clearly needed.

Most users are probably happy with the defaults. So the question is how
best to allow the small minority who want to tweak the clocksum format to
do so.

A couple of questions:

1. Is there any real need to allow the org-colview and org-clock formats
   to be customized independently? Making them the same would simplify
   things and probably be clearer for users.

2. What are the different formats that users are likely to want? The list
   can't be very long. I can think of: "hh:mm", "hh.mm" (fractional),
   "dd hh:mm" (separate days), "dd hh.mm", and possibly "MM dd hh:mm" and
   "YY MM dd hh:mm".

If the above covers everything we want, then what about getting rid of
every customization option except `org-time-clocksum-format', and parsing
the format string itself to decide how many and what arguments to pass to
it?

More precisely, if the format string contains ":", "." or "," then the
smallest time component is minutes; otherwise it's hours. Pass as many
time components as necessary to fill all the "%" expandos in the format
string, from largest to smallest, with either hours or minutes as the
smallest. If the format string contains "." or "," then pass the number
of minutes as a fraction ("," is used as the decimal separator in many
European languages).

This would simplify things, and make the format string just "do the right
thing" in all the cases I listed above. On the other hand, it won't allow
unusual formats that don't fit the above scheme (but they're not possible
now, anyway).

Thoughts?

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Hello,

Toby Cubitt <tsc25@cantab.net> writes:

> Looking through the code, it seems the clocksum format options are used
> in two places: org-colview.el and org-clock.el. For some reason, only the
> latter honours `org-time-clocksum-use-fractional'. In my patch, only the
> former honoured the new `org-time-clocksum-days-format'. Some
> rationalisation of all these options is clearly needed.

That's the purpose of the patch: only one function to rule them all.

> Most users are probably happy with the defaults. So the question is how
> best to allow the small minority who want to tweak the clocksum format to
> do so.

Allow a free function and provide default ones.

> A couple of questions:
>
> 1. Is there any real need to allow the org-colview and org-clock formats
>    to be customized independently? Making them the same would simplify
>    things and probably be clearer for users.

I think they should be formatted the same way. It's important to have
a consistent format for such things.

> 2. What are the different formats that users are likely to want? The list
>    can't be very long. I can think of: "hh:mm", "hh.mm" (fractional),
>    "dd hh:mm" (separate days), "dd hh.mm", and possibly "MM dd hh:mm" and
>    "YY MM dd hh:mm".

Just provide what is actually possible to have along with your day
count. It will make a good enough default offer.

> If the above covers everything we want, then what about getting rid of
> every customization option except `org-time-clocksum-format', and parsing
> the format string itself to decide how many and what arguments to pass to
> it?
>
> More precisely, if the format string contains ":", "." or "," then the
> smallest time component is minutes; otherwise it's hours. Pass as many
> time components as necessary to fill all the "%" expandos in the format
> string, from largest to smallest, with either hours or minutes as the
> smallest. If the format string contains "." or "," then pass the number
> of minutes as a fraction ("," is used as the decimal separator in many
> European languages).

That would be over-engineering it.

> This would simplify things, and make the format string just "do the right
> thing" in all the cases I listed above. On the other hand, it won't allow
> unusual formats that don't fit the above scheme (but they're not possible
> now, anyway).
>
> Thoughts?

I think it's too much complicated: it requires to know about strange
formatting rules. I suggest to keep it simple: just specify a function
with fixed arguments to do the job and provide default functions to
handle most common cases.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Mon, Nov 05, 2012 at 01:20:48PM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
[snip]
> > [...] what about getting rid of every customization option except
> > `org-time-clocksum-format', and parsing the format string itself to
> > decide how many and what arguments to pass to it?
> >
> > More precisely, if the format string contains ":", "." or "," then the
> > smallest time component is minutes; otherwise it's hours. Pass as many
> > time components as necessary to fill all the "%" expandos in the format
> > string, from largest to smallest, with either hours or minutes as the
> > smallest. If the format string contains "." or "," then pass the number
> > of minutes as a fraction ("," is used as the decimal separator in many
> > European languages).
> 
> That would be over-engineering it.
> 
> > This would simplify things, and make the format string just "do the right
> > thing" in all the cases I listed above. On the other hand, it won't allow
> > unusual formats that don't fit the above scheme (but they're not possible
> > now, anyway).
> >
> > Thoughts?
> 
> I think it's too much complicated: it requires to know about strange
> formatting rules. I suggest to keep it simple: just specify a function
> with fixed arguments to do the job and provide default functions to
> handle most common cases.

I'm fine with a function + sensible defaults, but Achim didn't like it
and proposed a custom format syntax instead.

org-time-clocksum-format is used all over the place in org-clock.el,
often concatenated with other bits of format string. So the changes
needed to change it into "one function to rule them all" are more
extensive, though fairly trivial.

I'd prefer to see some agreement before I waste time coding something
that won't get accepted.

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Hello,

Toby Cubitt <tsc25@cantab.net> writes:

> I'd prefer to see some agreement before I waste time coding something
> that won't get accepted.

Then let's wait for Achim (Cc'ed) to illustrate what he has in mind,
because his proposal is too vague yet to permit discussion about it.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Achim Gratz]

Nicolas Goaziou writes:
> Then let's wait for Achim (Cc'ed) to illustrate what he has in mind,
> because his proposal is too vague yet to permit discussion about it.

What I had in mind was to remove org-time-clocksum-format and replace
the associated format calls with a formatting function that has
customizable behaviour (how customizable is t.b.d.).

(format org-time-clocksum-format h m) => (org--format-time n fmt)

Even if nothing else changes, that removes a lot of unnecessary code
duplication, like the seven or so different ways to split the hours from
the minutes that may or may not agree on their results.  This is not far
from your own suggestion to provide different functions depending on
which output is desired, I just happen to think that these functions
would all be so similar that they should be rolled into a single
function that can produce different outputs.  I think there'd only be a
handful of possible values for fmt based on the current usage and that
suggests just another cond form would be needed in implementing this
function rather than a full-blown format string interpreter.  The fmt
argument might even be optional (use the custom value if nil) or
dynamically bound instead of being a function argument.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Mon, Nov 05, 2012 at 06:40:20PM +0100, Achim Gratz wrote:
> Nicolas Goaziou writes:
> > Then let's wait for Achim (Cc'ed) to illustrate what he has in mind,
> > because his proposal is too vague yet to permit discussion about it.
> 
> What I had in mind was to remove org-time-clocksum-format and replace
> the associated format calls with a formatting function that has
> customizable behaviour (how customizable is t.b.d.).
> 
> (format org-time-clocksum-format h m) => (org--format-time n fmt)
>
> Even if nothing else changes, that removes a lot of unnecessary code
> duplication, like the seven or so different ways to split the hours
> from the minutes that may or may not agree on their results.

I think whatever we end up doing, it's going simply things and remove
duplicate code. Which I agree is definitely a good thing.

> This is not far from your own suggestion to provide different functions
> depending on which output is desired, I just happen to think that these
> functions would all be so similar that they should be rolled into a
> single function that can produce different outputs.  I think there'd
> only be a handful of possible values for fmt based on the current usage
> and that suggests just another cond form would be needed in
> implementing this function rather than a full-blown format string
> interpreter.

It seems to me your `org--format-time' function would end up looking very
like what I sketched. A cond to switch between "hh:mm", "hh.mm",
"dd hh:mm" or "dd hh.mm" based only on the contents of the fmt argument
would have to check whether fmt contains 2 or 3 %-sequences, then check
if it contains "." or ":"

If these 4 options are the only ones that are ever going to be useful, we
could instead just have a single defcustom with a 4-way choice (between 4
different descriptive symbols). This would remove some flexibility from
the existing version (as well as adding some), but it's simpler than
partially parsing the fmt string.

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

>> This is not far from your own suggestion to provide different functions
>> depending on which output is desired, I just happen to think that these
>> functions would all be so similar that they should be rolled into a
>> single function that can produce different outputs.  I think there'd
>> only be a handful of possible values for fmt based on the current usage
>> and that suggests just another cond form would be needed in
>> implementing this function rather than a full-blown format string
>> interpreter.
>
> It seems to me your `org--format-time' function would end up looking very
> like what I sketched. A cond to switch between "hh:mm", "hh.mm",
> "dd hh:mm" or "dd hh.mm" based only on the contents of the fmt argument
> would have to check whether fmt contains 2 or 3 %-sequences, then check
> if it contains "." or ":"

Again, these are strange and very limiting rules. What if I want to have
"5 h 32 min"? And "5,3 days"?

Achim didn't specify how he conceives the FMT argument. One possibility
would be to have a placeholder-based template with, i.e. %d, %h, %m, %w
for respectively number of days, hours, minutes and weeks. But it's
still less flexible than functions because you need to have a fixed
number of placeholders in every template.

I still think functions are the way to go. Three options in the
defcustom:

  - One to provide regular time (i.e 14:40 or 3d 18:32)

  - One to provide decimal time with the highest unit available (i.e.
    18,75 h or 2,5 d).

  - One free slot for an user-defined function.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Mon, Nov 05, 2012 at 11:45:24PM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> >> This is not far from your own suggestion to provide different functions
> >> depending on which output is desired, I just happen to think that these
> >> functions would all be so similar that they should be rolled into a
> >> single function that can produce different outputs.  I think there'd
> >> only be a handful of possible values for fmt based on the current usage
> >> and that suggests just another cond form would be needed in
> >> implementing this function rather than a full-blown format string
> >> interpreter.
> >
> > It seems to me your `org--format-time' function would end up looking very
> > like what I sketched. A cond to switch between "hh:mm", "hh.mm",
> > "dd hh:mm" or "dd hh.mm" based only on the contents of the fmt argument
> > would have to check whether fmt contains 2 or 3 %-sequences, then check
> > if it contains "." or ":"
> 
> Again, these are strange and very limiting rules. What if I want to
> have "5 h 32 min"? And "5,3 days"?

Good point.

> Achim didn't specify how he conceives the FMT argument. One possibility
> would be to have a placeholder-based template with, i.e. %d, %h, %m, %w
> for respectively number of days, hours, minutes and weeks. But it's
> still less flexible than functions because you need to have a fixed
> number of placeholders in every template.

Indeed. None of the format-only proposals would let me reproduce the
setup I have currently (with my earlier patch): "5d 3h" for durations
longer than a day, "3:15" for durations shorter than a day.

> I still think functions are the way to go. Three options in the
> defcustom:
> 
>   - One to provide regular time (i.e 14:40 or 3d 18:32)
> 
>   - One to provide decimal time with the highest unit available (i.e.
>     18,75 h or 2,5 d).
> 
>   - One free slot for an user-defined function.

I like the flexibility of functions. But one drawback of this is that you
can't produce your "5 h 32 min" or "5,3 days" examples without defining a
new function. It would be nice if tweaking just the format (without
changing the numbers themselves) could be done by changing a simple
format string.

Because the number of placeholders in a format string is fixed, I don't
see how to avoid the need for multiple format strings. Perhaps we need a
second defcustom that holds a list of format strings, to be used by the
functions in your first two choices.

The first format string for durations < 1 day (or for all durations if
this is the only string in the list), the second for durations >= 1 day.
One nice thing is that this could easily be extended in the obvious way
if one wanted to allow different formats for durations >= 1 month or
>= 1 year.

It's slightly ugly that the defaults for the format-string defcustom
would have to change depending on the value of the function defcustom. I
guess one could either have the format-string defcustom default to nil,
and use hard-coded defaults in the functions (which are overridden by a
non-nil format string value). Or put both functions and format strings
into a single defcustom, e.g. as a list with the function in the first
element.

Best,
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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

>> I still think functions are the way to go. Three options in the
>> defcustom:
>> 
>>   - One to provide regular time (i.e 14:40 or 3d 18:32)
>> 
>>   - One to provide decimal time with the highest unit available (i.e.
>>     18,75 h or 2,5 d).
>> 
>>   - One free slot for an user-defined function.
>
> I like the flexibility of functions. But one drawback of this is that you
> can't produce your "5 h 32 min" or "5,3 days" examples without defining a
> new function.
>
> It would be nice if tweaking just the format (without
> changing the numbers themselves) could be done by changing a simple
> format string.
>
> Because the number of placeholders in a format string is fixed, I don't
> see how to avoid the need for multiple format strings. Perhaps we need a
> second defcustom that holds a list of format strings, to be used by the
> functions in your first two choices.
>
> The first format string for durations < 1 day (or for all durations if
> this is the only string in the list), the second for durations >= 1 day.
> One nice thing is that this could easily be extended in the obvious way
> if one wanted to allow different formats for durations >= 1 month or
>>= 1 year.
>
> It's slightly ugly that the defaults for the format-string defcustom
> would have to change depending on the value of the function defcustom. I
> guess one could either have the format-string defcustom default to nil,
> and use hard-coded defaults in the functions (which are overridden by a
> non-nil format string value). Or put both functions and format strings
> into a single defcustom, e.g. as a list with the function in the first
> element.

Actually the number of functions defined doesn't matter much. What
matters is the number of functions exposed to the end-user, which is
0 in this situation (or 1 if he decides to write his own). Here, all is
solved with one defcustom.

You don't even need to create multiple functions for that. The defcustom
can store `regular', `decimal' symbols or a function. Then you can write
a generic duration format function that will be used across code base
with the following template:

#+begin_src emacs-lisp
(defun org-build-format-duration (n)
  "Format duration N according to `org-duration-format' variable.
N is the duration to display, as a number, expressed in minutes.
Return formatted duration as a string."
  (cond ((functionp org-duration-format) (funcall org-duration-format))
        ((eq org-duration-format 'regular) ...)
        ((eq org-duration-format 'decimal) ...)
        (t (error "Invalid `org-duration-format' value"))))
#+end_src

One variable exposed to the user. One function exposed to the developer.
It's much simpler.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Tue, Nov 06, 2012 at 11:57:25AM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> 
> >> I still think functions are the way to go. Three options in the
> >> defcustom:
> >> 
> >>   - One to provide regular time (i.e 14:40 or 3d 18:32)
> >> 
> >>   - One to provide decimal time with the highest unit available (i.e.
> >>     18,75 h or 2,5 d).
> >> 
> >>   - One free slot for an user-defined function.
> >
> > I like the flexibility of functions. But one drawback of this is that you
> > can't produce your "5 h 32 min" or "5,3 days" examples without defining a
> > new function.
> >
> > It would be nice if tweaking just the format (without
> > changing the numbers themselves) could be done by changing a simple
> > format string.
> >
> > Because the number of placeholders in a format string is fixed, I don't
> > see how to avoid the need for multiple format strings. Perhaps we need a
> > second defcustom that holds a list of format strings, to be used by the
> > functions in your first two choices.
> >
> > The first format string for durations < 1 day (or for all durations if
> > this is the only string in the list), the second for durations >= 1 day.
> > One nice thing is that this could easily be extended in the obvious way
> > if one wanted to allow different formats for durations >= 1 month or
> >>= 1 year.
> >
> > It's slightly ugly that the defaults for the format-string defcustom
> > would have to change depending on the value of the function defcustom. I
> > guess one could either have the format-string defcustom default to nil,
> > and use hard-coded defaults in the functions (which are overridden by a
> > non-nil format string value). Or put both functions and format strings
> > into a single defcustom, e.g. as a list with the function in the first
> > element.
> 
> Actually the number of functions defined doesn't matter much. What
> matters is the number of functions exposed to the end-user, which is 0
> in this situation (or 1 if he decides to write his own). Here, all is
> solved with one defcustom.
>
> You don't even need to create multiple functions for that. The defcustom
> can store `regular', `decimal' symbols or a function.

Sure, I'm familiar with how defcustoms work.

> Then you can write a generic duration format function that will be used
> across code base with the following template:
> 
> #+begin_src emacs-lisp
> (defun org-build-format-duration (n)
>   "Format duration N according to `org-duration-format' variable.
> N is the duration to display, as a number, expressed in minutes.
> Return formatted duration as a string."
>   (cond ((functionp org-duration-format) (funcall org-duration-format))
>         ((eq org-duration-format 'regular) ...)
>         ((eq org-duration-format 'decimal) ...)
>         (t (error "Invalid `org-duration-format' value"))))
> #+end_src

The issue I pointed out has nothing to do with the internal
implementation.

> One variable exposed to the user. One function exposed to the developer.
> It's much simpler.

You're missing my point (which probably means I didn't explain it well).

How do I produce the format "5 h 32 min" with your defcustom, without
requiring the user to define their own function? (Assuming that 'regular
produces the current default "5:32" format.)

You still need a way to allow users to supply format strings, so they can
customize the appearance of the `regular' and `decimal' formats. Your own
"5 h 32 min" and "5,3 days" examples demonstrate this.

This either implies a second defcustom for the format strings, or it
implies storing both the format strings and choice of function choice in
the same defcustom. Furthermore, one format string isn't be enough
because you might want to different numbers of placeholders depending on
the duration (which is what my original patch allowed).

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> How do I produce the format "5 h 32 min" with your defcustom, without
> requiring the user to define their own function? (Assuming that 'regular
> produces the current default "5:32" format.)

Why "without defining their own function"? My proposal was to use
functions instead of format strings to customize output.

> You still need a way to allow users to supply format strings, so they can
> customize the appearance of the `regular' and `decimal' formats. Your own
> "5 h 32 min" and "5,3 days" examples demonstrate this.

In my proposal, customizing `regular' and `decimal' format wasn't
expected.

> This either implies a second defcustom for the format strings, or it
> implies storing both the format strings and choice of function choice in
> the same defcustom. Furthermore, one format string isn't be enough
> because you might want to different numbers of placeholders depending on
> the duration (which is what my original patch allowed).

But let's forget about it, it's a false good idea, anyway. I'm shooting
myself in the foot: custom formats mean parsing hell. So either:

1. We define a new format, non customizable, but possibly conditional,
   which can describe a duration, in order to include days.

2. We allow customization as overlays (much like timestamps).

3. We leave it as-is.

I'm not very fond of 2, so I think this whole customization problem is
moot anyway.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Tue, Nov 06, 2012 at 01:29:02PM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> 
> > How do I produce the format "5 h 32 min" with your defcustom, without
> > requiring the user to define their own function? (Assuming that 'regular
> > produces the current default "5:32" format.)
> 
> Why "without defining their own function"? My proposal was to use
> functions instead of format strings to customize output.

Because customizing a format string is easier than defining a new
function when you don't want to do anything more fancy than modifying the
textual formatting. (If you want to do something conditional on the
duration, then defining a new function is natural.)

> > You still need a way to allow users to supply format strings, so they can
> > customize the appearance of the `regular' and `decimal' formats. Your own
> > "5 h 32 min" and "5,3 days" examples demonstrate this.
> 
> In my proposal, customizing `regular' and `decimal' format wasn't
> expected.
> 
> > This either implies a second defcustom for the format strings, or it
> > implies storing both the format strings and choice of function choice in
> > the same defcustom. Furthermore, one format string isn't be enough
> > because you might want to different numbers of placeholders depending on
> > the duration (which is what my original patch allowed).
> 
> But let's forget about it, it's a false good idea, anyway. I'm shooting
> myself in the foot: custom formats mean parsing hell.

No. It's possible to do it with standard format strings. You just need to
(optionally) allow multiple format strings, one for each duration.


> So either:
> 
> 1. We define a new format, non customizable, but possibly conditional,
>    which can describe a duration, in order to include days.
> 
> 2. We allow customization as overlays (much like timestamps).
> 
> 3. We leave it as-is.
> 
> I'm not very fond of 2, so I think this whole customization problem is
> moot anyway.

Or

4. We replace the existing muddle with two defcustoms, one selecting
   regular or decimal (or user-defined function), and one specifying a
   list of standard format strings that the function can choose between,
   depending on the duration.

No custom format required, no format string parsing required.

Best,
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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> 4. We replace the existing muddle with two defcustoms, one selecting
>    regular or decimal (or user-defined function), and one specifying a
>    list of standard format strings that the function can choose between,
>    depending on the duration.
>
> No custom format required, no format string parsing required.

I'm talking about the other parsing: from the displayed duration to
a number of minutes. It cannot be done if we allow user-defined
functions.

Also, I'd rather have one defcustom than two for a simple thing like
duration display.

So, I still suggest to provide an unique, although conditional, display
for duration. If you still want to allow customization, make sure it can
be parsed back (or better, provide the parser). I'm not fundamentally
against format strings.


Regards,

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Tue, Nov 06, 2012 at 06:41:06PM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> 
> > 4. We replace the existing muddle with two defcustoms, one selecting
> >    regular or decimal (or user-defined function), and one specifying a
> >    list of standard format strings that the function can choose between,
> >    depending on the duration.
> >
> > No custom format required, no format string parsing required.
> 
> I'm talking about the other parsing: from the displayed duration to
> a number of minutes. It cannot be done if we allow user-defined
> functions.

I doubt it can be done now, since we already allow user-defined format
strings.

Where are displayed durations formatted with org-time-clocksum-format et
al. parsed back to a number of minutes in the current code? If there is
anywhere, it's surely broken because a user-supplied
`org-time-clocksum-format' or `org-time-clocksum-fractional-format' could
already format the duration in arbitrarily bizarre ways as things are
currently.

> Also, I'd rather have one defcustom than two for a simple thing like
> duration display.

There are currently three, four with my original patch. Reducing this to
two whilst simultaneously increasing customizability and ensuring a
uniform format doesn't seem so bad :)

We're trying to allow two fundamentally different things here:

1. Change the values that are computed and displayed (e.g. condition
   on < 1 day vs. >= 1 day; use fractional minutes or not).

2. Customize the way those values are formatted ("3:15" vs. "3h 15min").

It seems fairly natural to separate these into two defcustoms, rather
than try to somehow mash two rather different types of customization into
one. The same logic was presumably lurking behind the separation of
org-time-clocksum-use-fractional and
org-time-clocksum-[fractional-]format.

If we want to allow the kind of flexibility you were proposing, then
pre-canned options (or user-defined function) are a good fit to 1. and
format strings are the natural choice for 2.

If all we want to do is allow the existing regular or fractional formats
to be conditioned on the duration, a 100% backwards-compatible
alternative could be to allow org-time-clocksum-[fractional-]format to be
either a single format string (as currently) or a list of format
strings. In the latter case, the first element would be used for
durations < 1 day, the second for durations >= 1 day (extending to
months/years/decades in the obvious way if desired).

The benefits of this over my original patch are: (a) it avoids
introducing any new defcustoms or changing the existing ones in a
backwards-incompatibility way; (b) it could easily be generalised if
desired (now or in future) to condition the format on further duration
ranges. The disadvantages are: (i) it doesn't have the flexibility of
user-defined format functions; (ii) it doesn't simplify the existing
defcustoms.

> So, I still suggest to provide an unique, although conditional, display
> for duration. If you still want to allow customization, make sure it can
> be parsed back (or better, provide the parser). I'm not fundamentally
> against format strings.

You mean abandon any sort of customizable format string (since that
inherently can't be parsed back in general), and use a hard-coded
conditional "hh:mm" or "dd hh:mm" format? (Possibly retaining one
customisation option, org-time-clocksum-use-fractional, to switch this to
"hh.mm" or "dd hh.mm"?)

That would give me the format I want, but it's a feature regression.

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> I doubt it can be done now, since we already allow user-defined format
> strings.

I'm more focused on what we will be able to do.

> Where are displayed durations formatted with org-time-clocksum-format et
> al. parsed back to a number of minutes in the current code? If there is
> anywhere, it's surely broken because a user-supplied
> `org-time-clocksum-format' or `org-time-clocksum-fractional-format' could
> already format the duration in arbitrarily bizarre ways as things are
> currently.

In org-element.el.

> You mean abandon any sort of customizable format string (since that
> inherently can't be parsed back in general), and use a hard-coded
> conditional "hh:mm" or "dd hh:mm" format? (Possibly retaining one
> customisation option, org-time-clocksum-use-fractional, to switch this to
> "hh.mm" or "dd hh.mm"?)

We can allow a limited set of conses of format strings (with or without
days), possibly defined in the same defcustom (see
`org-table-number-regexp' customize interface). If we know the format
string used, we can parse it back.

> That would give me the format I want, but it's a feature regression.

There are features more honoured in the breach than in the observance.
I want to have a parseable Org syntax, for its own good.


Regards,

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Tue, Nov 06, 2012 at 08:55:12PM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> 
> > I doubt it can be done now, since we already allow user-defined format
> > strings.
> 
> I'm more focused on what we will be able to do.
> 
> > Where are displayed durations formatted with org-time-clocksum-format et
> > al. parsed back to a number of minutes in the current code? If there is
> > anywhere, it's surely broken because a user-supplied
> > `org-time-clocksum-format' or `org-time-clocksum-fractional-format' could
> > already format the duration in arbitrarily bizarre ways as things are
> > currently.
> 
> In org-element.el.

But that only needs to parse clock strings stored in properties/drawers,
not the ones displayed in overlays (column view) or in the mode-line.

Are the clock strings stored in properties/drawers formatted using the
existing org-time-clocksum-* defcustoms? I can't easily tell from the
org-clock.el code...

The only sane answer ought to be "no" (which doesn't mean that it is ;)
It would clearly be better if the clock strings stored in org buffers
used a single fixed format, which could be mangled as desired for display
in overlays and the mode-line.

> > You mean abandon any sort of customizable format string (since that
> > inherently can't be parsed back in general), and use a hard-coded
> > conditional "hh:mm" or "dd hh:mm" format? (Possibly retaining one
> > customisation option, org-time-clocksum-use-fractional, to switch this to
> > "hh.mm" or "dd hh.mm"?)
> 
> We can allow a limited set of conses of format strings (with or without
> days), possibly defined in the same defcustom (see
> `org-table-number-regexp' customize interface). If we know the format
> string used, we can parse it back.

Ugh. Wouldn't it be far better to make sure the customization options
only affect the visual display of clocksum durations (in
overlays/mode-line), and not the strings stored in the file? Then the
parser can be kept simple and reliable.

> > That would give me the format I want, but it's a feature regression.
> 
> There are features more honoured in the breach than in the observance.
> I want to have a parseable Org syntax, for its own good.

Best way to achieve this is to separate style from content!

That would allow the visual clocksum format can be customized to our
hearts content, whilst keeping the parser simple and therefore reliable.

Best,
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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> But that only needs to parse clock strings stored in properties/drawers,
> not the ones displayed in overlays (column view) or in the mode-line.

Correct.

> Are the clock strings stored in properties/drawers formatted using the
> existing org-time-clocksum-* defcustoms? I can't easily tell from the
> org-clock.el code...
>
> The only sane answer ought to be "no" (which doesn't mean that it is ;)
> It would clearly be better if the clock strings stored in org buffers
> used a single fixed format, which could be mangled as desired for display
> in overlays and the mode-line.

Format string for clock lines is hard-coded (see line 1493 in
org-clock.el), which means we don't have to limit ourselves to
parse-able format. Back to point 1.

Then, I'm fine with format strings. Following your suggestion, what
about the following variables:
- one to determine format of data: fractional or regular.
- one to determine display format. Its value would be a list of 3 format
  strings associated to days, hours and minutes.

Internally, the duration is computed as a list of three integers or nil
if data is regular, or a list of one float and two nil if data is
fractional. Format strings from the second variable will be concatenated
only when value is non-nil. If no format string is found for a given
unit, it's value will be converted into a lesser unit.

Examples:

| var1        | var2                        | internal representation | display     |
|-------------+-----------------------------+-------------------------+-------------|
| 'regular    | ("%dd " "%d h " "%d min")   | (nil 11 35)             | 11 h 35 min |
| 'fractional | ("%.2fd" "%.2fh" "%.2fmin") | (nil 11.3 nil)          | 11.30h      |
| 'regular    | ("%dd " "%d:" "%02d")       | (1 3 5)                 | 1d 3:05     |
| 'regular    | (nil "%d:" "%02d")          | (1 3 5)                 | 27:05       |

We can extend it to years if needed.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Thu, Nov 08, 2012 at 01:26:48AM +0100, Nicolas Goaziou wrote:
> Format string for clock lines is hard-coded (see line 1493 in
> org-clock.el), which means we don't have to limit ourselves to
> parse-able format. Back to point 1.

Good. That's what I thought.

> Then, I'm fine with format strings. Following your suggestion, what
> about the following variables:
> - one to determine format of data: fractional or regular.
> - one to determine display format. Its value would be a list of 3 format
>   strings associated to days, hours and minutes.

Sounds good to me. I think your idea of separating out the format strings
for days, hours and minutes is better.

Minor point: I would order the format strings minutes, hours, days. In
case we ever want to extend to months or years, that way we can simply
extend the list and it will be completely backwards compatible.

One other thing that needs some thought (which I already mentioned
previously). The minutes format string needs to change, depending on
whether the data format is fractional or regular. So if a user wants to
switch to fractional, it's not enough to set the first variable; they
have to *also* change the minute format string. If they don't the
durations will be garbled.

A customization setter function could be used to change the value of the
second variable when the first one is modified through customize. But
that quickly gets complex, e.g. we have to be careful about clobbering
any customizations the user has already made to the format strings.

The simpler solution would be to always have two format strings for the
minutes in the list: a fractional format, and a regular format. This
shouldn't be too confusing as long as it's documented in the variable
docstring and there are good descriptive :tags for each list element in
the customization type.

> Internally, the duration is computed as a list of three integers or nil
> if data is regular, or a list of one float and two nil if data is
> fractional. Format strings from the second variable will be concatenated
> only when value is non-nil. If no format string is found for a given
> unit, it's value will be converted into a lesser unit.
> 
> Examples:
> 
> | var1        | var2                        | internal representation | display     |
> |-------------+-----------------------------+-------------------------+-------------|
> | 'regular    | ("%dd " "%d h " "%d min")   | (nil 11 35)             | 11 h 35 min |
> | 'fractional | ("%.2fd" "%.2fh" "%.2fmin") | (nil 11.3 nil)          | 11.30h      |
> | 'regular    | ("%dd " "%d:" "%02d")       | (1 3 5)                 | 1d 3:05     |
> | 'regular    | (nil "%d:" "%02d")          | (1 3 5)                 | 27:05       |
> 
> We can extend it to years if needed.

Looks good to me, and lets me do what I wanted in my original patch.

Now I just need to find time to code it up...

Best,
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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> Minor point: I would order the format strings minutes, hours, days. In
> case we ever want to extend to months or years, that way we can simply
> extend the list and it will be completely backwards compatible.

Using a plist can circumvent the problem, too. I'm ok either way.

> One other thing that needs some thought (which I already mentioned
> previously). The minutes format string needs to change, depending on
> whether the data format is fractional or regular.

Not necessarily. The format string can use %s. We can also remind it in
the docstring of both variables and provide examples.

> So if a user wants to switch to fractional, it's not enough to set the
> first variable; they have to *also* change the minute format string.
> If they don't the durations will be garbled.

Sometimes, yes.

> A customization setter function could be used to change the value of the
> second variable when the first one is modified through customize. But
> that quickly gets complex, e.g. we have to be careful about clobbering
> any customizations the user has already made to the format strings.

That sounds too complicated.

> The simpler solution would be to always have two format strings for the
> minutes in the list: a fractional format, and a regular format. This
> shouldn't be too confusing as long as it's documented in the variable
> docstring and there are good descriptive :tags for each list element in
> the customization type.

I think this is not necessary. We can just document the fact that the
user must check both variables before applying some change. Anyway,
I let you judge this.

> Looks good to me, and lets me do what I wanted in my original patch.
>
> Now I just need to find time to code it up...

Good to hear we eventually settled on a solution !


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

[-- Attachment #1: Type: text/plain, Size: 2147 bytes --]

Attached is a new two-patch series implementing finer-grained control
over the format of clocksum durations (only in overlays and the
mode-line; the format of the CLOCK entries stored in org files remains
unchanged and hard-coded, as before).

The first patch:
- defines a new org-minutes-to-clocksum-string function (replacing
  org-minutes-to-hh:mm-string) for formatting time durations according to
  customization options;
- unifies the formatting of clocksum durations throughout org-colview.el
  and org-clock.el by always calling org-minutes-to-clocksum-string to do
  the formatting;
- extends the org-time-clocksum-format defcustom, allowing it to either
  be a single format string (as before), or a list of between 1 and 3
  format strings for the minutes, hours and days components of a time
  duration;
- retains the org-time-clocksum-use-fractional and
  org-time-clocksum-fractional-format defcustoms, whose meaning is
  unchanged.

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).

Both patches maintain backwards-compatibility with any existing
customizations users may have made to these variables. They just add an
additional type for org-time-clocksum-format.

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); (ii) it maintains backwards-compatibility; (iii) it side-steps
the issue of making users customize the format whenever they modify
org-time-clocksum-use-fractional.


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.

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

[-- Attachment #2: 0001-Allow-more-flexible-customization-of-clocksum-format.patch --]
[-- Type: text/x-patch, Size: 11849 bytes --]

From 812b6704191d96a8fc255e9c282eeef8deb091cf Mon Sep 17 00:00:00 2001
From: "Toby S. Cubitt" <tsc25@cantab.net>
Date: Sun, 11 Nov 2012 22:20:24 +0000
Subject: [PATCH 1/2] Allow more flexible customization of clocksum format

* lisp/org.el (org-time-clocksum-format): as well as a single format
string, can now be a list of up to three format strings which
determine how the minutes, hours and days compontents of a clocksum
duraction should be formated.

* lisp/org.el (org-minutes-to-clocksum-string): function replacing
org-minutes-to-hh:mm-string, which converts a number of minutes to a
string according to the customization options.

* lisp/org-colview.el (org-columns-number-to-string): use new
org-minutes-to-clocksum-string function to format clocksum durations.

* lisp/org-clock.el: instead of calling org-minutes-to-hh:mm-string or
passing org-time-clocksum-format directly to format in order to format
a clock duration as a string, always call the new
org-minutes-to-clocksum-string function to do this.
---
 lisp/org-clock.el   |   51 +++++++++++++------------------------
 lisp/org-colview.el |    3 +-
 lisp/org.el         |   69 +++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 81 insertions(+), 42 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 84eb2fd..c768491 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -556,28 +556,23 @@ pointing to it."
 If an effort estimate was defined for the current item, use
 01:30/01:50 format (clocked/estimated).
 If not, show simply the clocked time like 01:50."
-  (let* ((clocked-time (org-clock-get-clocked-time))
-	 (h (floor clocked-time 60))
-	 (m (- clocked-time (* 60 h))))
+  (let ((clocked-time (org-clock-get-clocked-time)))
     (if org-clock-effort
 	(let* ((effort-in-minutes
 		(org-duration-string-to-minutes org-clock-effort))
-	       (effort-h (floor effort-in-minutes 60))
-	       (effort-m (- effort-in-minutes (* effort-h 60)))
 	       (work-done-str
 		(org-propertize
-		 (format org-time-clocksum-format h m)
+		 (org-minutes-to-clocksum-string clocked-time)
 		 'face (if (and org-clock-task-overrun (not org-clock-task-overrun-text))
 			   'org-mode-line-clock-overrun 'org-mode-line-clock)))
-	       (effort-str (format org-time-clocksum-format effort-h effort-m))
+	       (effort-str (org-minutes-to-clocksum-string effort-in-minutes))
 	       (clockstr (org-propertize
 			  (concat  " [%s/" effort-str
 				   "] (" (replace-regexp-in-string "%" "%%" org-clock-heading) ")")
 			  'face 'org-mode-line-clock)))
 	  (format clockstr work-done-str))
-      (org-propertize (format
-		       (concat "[" org-time-clocksum-format " (%s)]")
-		       h m org-clock-heading)
+      (org-propertize (concat "[" (org-minutes-to-clocksum-string clocked-time)
+			      (format " (%s)" org-clock-heading) "]")
 		      'face 'org-mode-line-clock))))
 
 (defun org-clock-get-last-clock-out-time ()
@@ -650,7 +645,7 @@ the mode line."
 	      (setq value (- current value))
 	    (if (equal ?+ sign) (setq value (+ current value)))))
 	(setq value (max 0 value)
-	      org-clock-effort (org-minutes-to-hh:mm-string value))
+	      org-clock-effort (org-minutes-to-clocksum-string value))
 	(org-entry-put org-clock-marker "Effort" org-clock-effort)
 	(org-clock-update-mode-line)
 	(message "Effort is now %s" org-clock-effort))
@@ -1528,8 +1523,9 @@ to, overriding the existing value of `org-clock-out-switch-to-state'."
 						"\\>"))))
 		  (org-todo org-clock-out-switch-to-state))))))
 	  (force-mode-line-update)
-	  (message (concat "Clock stopped at %s after HH:MM = " org-time-clocksum-format "%s") te h m
-		   (if remove " => LINE REMOVED" ""))
+	  (message (concat "Clock stopped at %s after "
+			   (org-minutes-to-clocksum-string (+ (* 60 h) m)) "%s")
+		   te (if remove " => LINE REMOVED" ""))
           (run-hooks 'org-clock-out-hook)
 	  (unless (org-clocking-p)
 	    (org-clock-delete-current)))))))
@@ -1797,12 +1793,9 @@ Use \\[org-clock-remove-overlays] to remove the subtree times."
 	(when org-remove-highlights-with-change
 	  (org-add-hook 'before-change-functions 'org-clock-remove-overlays
 			nil 'local))))
-    (if org-time-clocksum-use-fractional
-	(message (concat "Total file time: " org-time-clocksum-fractional-format
-			 " (%d hours and %d minutes)")
-		 (/ (+ (* h 60.0) m) 60.0) h m)
-      (message (concat "Total file time: " org-time-clocksum-format
-		       " (%d hours and %d minutes)") h m h m))))
+      (message (concat "Total file time: "
+		       (org-minutes-to-clocksum-string org-clock-file-total-minutes)
+		       " (%d hours and %d minutes)") h m)))
 
 (defvar org-clock-overlays nil)
 (make-variable-buffer-local 'org-clock-overlays)
@@ -1814,9 +1807,6 @@ This creates a new overlay and stores it in `org-clock-overlays', so that it
 will be easy to remove."
   (let* ((c 60) (h (floor (/ time 60))) (m (- time (* 60 h)))
 	 (l (if level (org-get-valid-level level 0) 0))
-	 (fmt (concat "%s " (if org-time-clocksum-use-fractional
-				org-time-clocksum-fractional-format
-			      org-time-clocksum-format) "%s"))
 	 (off 0)
 	 ov tx)
     (org-move-to-column c)
@@ -1825,14 +1815,9 @@ will be easy to remove."
     (setq ov (make-overlay (point-at-bol) (point-at-eol))
     	  tx (concat (buffer-substring (point-at-bol) (point))
 		     (make-string (+ off (max 0 (- c (current-column)))) ?.)
-		     (org-add-props (if org-time-clocksum-use-fractional
-					(format fmt
-						(make-string l ?*)
-						(/ (+ (* h 60.0) m) 60.0)
-						(make-string (- 16 l) ?\ ))
-				      (format fmt
-					      (make-string l ?*) h m
-					      (make-string (- 16 l) ?\ )))
+		     (org-add-props (concat (format "%s " (make-string l ?*))
+					    (org-minutes-to-clocksum-string time)
+					    (format "%s" (make-string (- 16 l) ?\ )))
 			 (list 'face 'org-clock-overlay))
 		     ""))
     (if (not (featurep 'xemacs))
@@ -2392,7 +2377,7 @@ from the dynamic block definition."
      (if properties (make-string (length properties) ?|) "")  ; properties columns, maybe
      (concat (format org-clock-total-time-cell-format (nth 7 lwords))  "| ") ; instead of a headline
      (format org-clock-total-time-cell-format
-	     (org-minutes-to-hh:mm-string (or total-time 0))) ; the time
+	     (org-minutes-to-clocksum-string (or total-time 0))) ; the time
      "|\n")                          ; close line
 
     ;; Now iterate over the tables and insert the data
@@ -2416,7 +2401,7 @@ from the dynamic block definition."
 		     (if level-p   "| " "") ; level column, maybe
 		     (if timestamp "| " "") ; timestamp column, maybe
 		     (if properties (make-string (length properties) ?|) "")  ;properties columns, maybe
-		     (org-minutes-to-hh:mm-string (nth 1 tbl))))) ; the time
+		     (org-minutes-to-clocksum-string (nth 1 tbl))))) ; the time
 
 	  ;; Get the list of node entries and iterate over it
 	  (setq entries (nth 2 tbl))
@@ -2449,7 +2434,7 @@ from the dynamic block definition."
 	     hlc headline hlc "|"                                ; headline
 	     (make-string (min (1- ntcol) (or (- level 1))) ?|)
 					; empty fields for higher levels
-	     hlc (org-minutes-to-hh:mm-string (nth 3 entry)) hlc ; time
+	     hlc (org-minutes-to-clocksum-string (nth 3 entry)) hlc ; time
 	     "|\n"                                               ; close line
 	     )))))
     ;; When exporting subtrees or regions the region might be
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 9d58b5f..d9afbd1 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1058,8 +1058,7 @@ Don't set this, this is meant for dynamic scoping.")
    ((memq fmt '(estimate)) (org-estimate-print n printf))
    ((not (numberp n)) "")
    ((memq fmt '(add_times max_times min_times mean_times))
-    (let* ((h (floor n)) (m (floor (+ 0.5 (* 60 (- n h))))))
-      (format org-time-clocksum-format h m)))
+    (org-hours-to-clocksum-string n))
    ((eq fmt 'checkbox)
     (cond ((= n (floor n)) "[X]")
 	  ((> n 1.) "[-]")
diff --git a/lisp/org.el b/lisp/org.el
index 080b527..3ae1892 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2714,11 +2714,37 @@ commands, if custom time display is turned on at the time of export."
 	(concat "[" (substring f 1 -1) "]")
       f)))
 
-(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.
+
+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.
+
+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."
   :group 'org-time
-  :type 'string)
+  :type '(choice (string :tag "single format")
+		 (group :tag "mins" (string :tag "minutes format"))
+		 (group :tag "mins,hrs"
+			(string :tag "minutes format")
+			(string :tag "hours format"))
+		 (group :tag "mins,hrs,days"
+			(string :tag "minutes format")
+			(string :tag "hours format")
+			(string :tag "days format"))))
 
 (defcustom org-time-clocksum-use-fractional nil
   "If non-nil, \\[org-clock-display] uses fractional times.
@@ -16667,11 +16693,40 @@ If there is already a time stamp at the cursor position, update it."
       (org-insert-time-stamp
        (encode-time 0 0 0 (nth 1 cal-date) (car cal-date) (nth 2 cal-date))))))
 
-(defun org-minutes-to-hh:mm-string (m)
+(defun org-minutes-to-clocksum-string (m)
   "Compute H:MM from a number of minutes."
-  (let ((h (/ m 60)))
-    (setq m (- m (* 60 h)))
-    (format org-time-clocksum-format h m)))
+  (let* ((d (/ m 1400)) (h (/ (- m (* 1400 d)) 60)))
+    (setq m (- m (* 1400 d) (* 60 h)))
+    (if org-time-clocksum-use-fractional
+	(format org-time-clocksum-fractional-format
+		(/ (+ (* 60.0 (+ (* 1400 d) h)) m) 60.0))
+      (cond
+       ((stringp org-time-clocksum-format)
+	(format org-time-clocksum-format (+ (* 1400 d) h) m))
+       ((> d 0)
+	(if (>= (length org-time-clocksum-format) 3)
+	    (concat (format (nth 2 org-time-clocksum-format) d)
+		    (format (nth 1 org-time-clocksum-format) h)
+		    (format (nth 0 org-time-clocksum-format) m))
+	  (if (>= (length org-time-clocksum-format) 2)
+	      (concat (format (nth 1 org-time-clocksum-format)
+			      (+ (* 1400 d) h) m)
+		      (format (nth 0 org-time-clocksum-format) m))
+	    (format (nth 0 org-time-clocksum-format)
+		    (+ (* 60 (+ (* 1400 d) h)) m)))))
+       (t (if (>= (length org-time-clocksum-format) 2)
+	      (concat (format (nth 1 org-time-clocksum-format)
+			      (+ (* 1400 d) h) m)
+		      (format (nth 0 org-time-clocksum-format) m))
+	    (format (nth 0 org-time-clocksum-format)
+		    (+ (* 60 (+ (* 1400 d) h)) m))))))))
+
+(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")
+
+(defun org-hours-to-clocksum-string (n)
+  (org-minutes-to-clocksum-string (* n 60)))
 
 (defun org-hh:mm-string-to-minutes (s)
   "Convert a string H:MM to a number of minutes.
-- 
1.7.8.6


[-- Attachment #3: 0002-Optionally-allow-months-and-years-to-be-used-in-cloc.patch --]
[-- Type: text/x-patch, Size: 5805 bytes --]

From 3a6a227158507d2f6500488f1f87eac8a6d47463 Mon Sep 17 00:00:00 2001
From: "Toby S. Cubitt" <tsc25@cantab.net>
Date: Mon, 12 Nov 2012 11:45:47 +0000
Subject: [PATCH 2/2] Optionally allow months and years to be used in clocksum
 format

---
 lisp/org.el |   98 ++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 57 insertions(+), 41 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 3ae1892..fd05104 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2722,29 +2722,25 @@ 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.
 
-Alternatively, the value can be a list of up to three format
+Alternatively, the value can be a list of up to five 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."
+hours, the third for the number of days if the duration is longer
+than 1 day, the fourth for the number of months if the duration
+is longer than 1 month (a month is taken to be 30 days here), the
+fifth for the number of years if the duration is longer than 1
+year (a year is taken to be 365 days here). The complete
+formatted duration is obtained by concatenating these in the
+order: years, months, days, hours, minutes.
+
+If the list contains fewer than five elements, the largest time
+unit in the formatted duration is restricted to the largest time
+unit in the list. E.g. 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."
   :group 'org-time
-  :type '(choice (string :tag "single format")
-		 (group :tag "mins" (string :tag "minutes format"))
-		 (group :tag "mins,hrs"
-			(string :tag "minutes format")
-			(string :tag "hours format"))
-		 (group :tag "mins,hrs,days"
-			(string :tag "minutes format")
-			(string :tag "hours format")
-			(string :tag "days format"))))
+  :type '(choice (string :tag "format string")
+		 (repeat :tag "list" (string :tag "format string"))))
 
 (defcustom org-time-clocksum-use-fractional nil
   "If non-nil, \\[org-clock-display] uses fractional times.
@@ -16695,31 +16691,51 @@ If there is already a time stamp at the cursor position, update it."
 
 (defun org-minutes-to-clocksum-string (m)
   "Compute H:MM from a number of minutes."
-  (let* ((d (/ m 1400)) (h (/ (- m (* 1400 d)) 60)))
-    (setq m (- m (* 1400 d) (* 60 h)))
+  (let ((h (/ m 60)) d M Y)
+    (setq m (- m (* 60 h)))
     (if org-time-clocksum-use-fractional
 	(format org-time-clocksum-fractional-format
-		(/ (+ (* 60.0 (+ (* 1400 d) h)) m) 60.0))
+		(/ (+ (* 60.0 h) m) 60.0))
       (cond
+       ;; single hh:mm format
        ((stringp org-time-clocksum-format)
-	(format org-time-clocksum-format (+ (* 1400 d) h) m))
-       ((> d 0)
-	(if (>= (length org-time-clocksum-format) 3)
-	    (concat (format (nth 2 org-time-clocksum-format) d)
-		    (format (nth 1 org-time-clocksum-format) h)
-		    (format (nth 0 org-time-clocksum-format) m))
-	  (if (>= (length org-time-clocksum-format) 2)
-	      (concat (format (nth 1 org-time-clocksum-format)
-			      (+ (* 1400 d) h) m)
-		      (format (nth 0 org-time-clocksum-format) m))
-	    (format (nth 0 org-time-clocksum-format)
-		    (+ (* 60 (+ (* 1400 d) h)) m)))))
-       (t (if (>= (length org-time-clocksum-format) 2)
-	      (concat (format (nth 1 org-time-clocksum-format)
-			      (+ (* 1400 d) h) m)
-		      (format (nth 0 org-time-clocksum-format) m))
-	    (format (nth 0 org-time-clocksum-format)
-		    (+ (* 60 (+ (* 1400 d) h)) m))))))))
+	(format org-time-clocksum-format h m))
+       ;; years
+       ((and (>= (length org-time-clocksum-format) 5)
+	     (> (/ h 8760) 0))
+	(setq Y (/ h 8760) h (- h (* 8760 Y))
+	      M (/ h 720)  h (- h (* 720 M))
+	      d (/ h 24)   h (- h (* 24 d)))
+	(concat (format (nth 4 org-time-clocksum-format) Y)
+		(format (nth 3 org-time-clocksum-format) M)
+		(format (nth 2 org-time-clocksum-format) d)
+		(format (nth 1 org-time-clocksum-format) h)
+		(format (nth 0 org-time-clocksum-format) m)))
+       ;; months
+       ((and (>= (length org-time-clocksum-format) 4)
+	     (> (/ h 720) 0))
+	(setq M (/ h 720)  h (- h (* 720 M))
+	      d (/ h 24)   h (- h (* 24 d)))
+	(concat (format (nth 3 org-time-clocksum-format) M)
+		(format (nth 2 org-time-clocksum-format) d)
+		(format (nth 1 org-time-clocksum-format) h)
+		(format (nth 0 org-time-clocksum-format) m)))
+       ;; days
+       ((and (>= (length org-time-clocksum-format) 3)
+	     (> (/ h 24) 0))
+	(setq d (/ h 24) h (- h (* 24 d)))
+	(concat (format (nth 2 org-time-clocksum-format) d)
+		(format (nth 1 org-time-clocksum-format) h)
+		(format (nth 0 org-time-clocksum-format) m)))
+       ;; hours
+       ((>= (length org-time-clocksum-format) 2)
+	(concat (format (nth 1 org-time-clocksum-format) h)
+		(format (nth 0 org-time-clocksum-format) m)))
+       ;; minutes
+       ((>= (length org-time-clocksum-format) 1)
+	(setq m (+ m (* 60 h)))
+	(format (nth 0 org-time-clocksum-format) m))
+       (t "")))))
 
 (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
-- 
1.7.8.6

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> 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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Wed, Nov 14, 2012 at 04:04:05PM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> 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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> 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?

That's the idea, yes. Though, it will be the largest time unit _with
a format string_ which will give a non-zero integer part.

> 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.

If they don't want to do anything fancy, they use the default value,
whatever it may be. ;) I don't mind keeping the single format string
option anyway.

> I'll post an updated patch when I get time to make the changes.

Since it's for 8.0, there's no hurry. I'll wait for you to merge the two
patches and make subsequent changes.

Thank you.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Wed, Nov 14, 2012 at 05:09:38PM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> 
> > 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?
> 
> That's the idea, yes. Though, it will be the largest time unit _with
> a format string_ which will give a non-zero integer part.

Yes, that's what I meant.

> > 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.
> 
> If they don't want to do anything fancy, they use the default value,
> whatever it may be. ;) I don't mind keeping the single format string
> option anyway.

I'll leave it in my patch. If you want to remove it for 8.0, it'll be a
simple case of deleting some code.

> > I'll post an updated patch when I get time to make the changes.
> 
> Since it's for 8.0, there's no hurry. I'll wait for you to merge the two
> patches and make subsequent changes.

OK.

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

[-- Attachment #1: Type: text/plain, Size: 2477 bytes --]

On Wed, Nov 14, 2012 at 05:20:14PM +0100, Toby Cubitt wrote:
> On Wed, Nov 14, 2012 at 05:09:38PM +0100, Nicolas Goaziou wrote:
> > Toby Cubitt <tsc25@cantab.net> writes:
> > 
> > > 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?
> > 
> > That's the idea, yes. Though, it will be the largest time unit _with
> > a format string_ which will give a non-zero integer part.
> 
> Yes, that's what I meant.
> 
> > > 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.
> > 
> > If they don't want to do anything fancy, they use the default value,
> > whatever it may be. ;) I don't mind keeping the single format string
> > option anyway.
> 
> I'll leave it in my patch. If you want to remove it for 8.0, it'll be a
> simple case of deleting some code.
> 
> > > I'll post an updated patch when I get time to make the changes.
> > 
> > Since it's for 8.0, there's no hurry. I'll wait for you to merge the two
> > patches and make subsequent changes.

Here's an updated patch. Now both org-time-clocksum-format and
org-time-clocksum-fractional-format can be plists, as discussed.

In the org-time-clocksum-format case, I made the values cons cells which
specify both a format string and a boolean. The latter indicates whether
the time component should always be included in the formatted duration,
even if its value is 0. This is needed for the hours component to
reproduce the current default format, and I figured I might as well make
it general.

I used a somewhat complex customization type in the defcustoms, instead
of a straight plist, in order to produce a better ui for the
customization interface. I'm still not completely satisfied with it.
E.g. it would be nice to get rid of the "Cons cell" tag entirely, and use
a checkbox for the boolean. But I can't figure out how to do that
(without defining new customization types/widgets, which I don't have the
patience for).

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

[-- Attachment #2: 0001-Allow-more-flexible-customization-of-clocksum-format.patch --]
[-- Type: text/x-patch, Size: 16616 bytes --]

From 639baf9c942df97e7355f402a9df38e6c9b6ef88 Mon Sep 17 00:00:00 2001
From: "Toby S. Cubitt" <tsc25@cantab.net>
Date: Sun, 11 Nov 2012 22:20:24 +0000
Subject: [PATCH] Allow more flexible customization of clocksum format

* lisp/org.el (org-time-clocksum-format, org-time-clocksum-fractional-format):
in addition to a single format string, the clocksum formats can now be
plists specifying separate formats for different time units.

* lisp/org.el (org-minutes-to-clocksum-string): new function to
replace org-minutes-to-hh:mm-string, which converts a number of
minutes to a string according to the customization options.

* lisp/org-colview.el (org-columns-number-to-string): use new
org-minutes-to-clocksum-string function to format clocksum durations.

* lisp/org-clock.el: always call new org-minutes-to-clocksum-string
function when formatting time durations, instead of calling
org-minutes-to-hh:mm-string or passing org-time-clocksum-format
directly to format.
---
 lisp/org-clock.el   |   51 +++++----------
 lisp/org-colview.el |    3 +-
 lisp/org.el         |  175 +++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 183 insertions(+), 46 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 84eb2fd..c768491 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -556,28 +556,23 @@ pointing to it."
 If an effort estimate was defined for the current item, use
 01:30/01:50 format (clocked/estimated).
 If not, show simply the clocked time like 01:50."
-  (let* ((clocked-time (org-clock-get-clocked-time))
-	 (h (floor clocked-time 60))
-	 (m (- clocked-time (* 60 h))))
+  (let ((clocked-time (org-clock-get-clocked-time)))
     (if org-clock-effort
 	(let* ((effort-in-minutes
 		(org-duration-string-to-minutes org-clock-effort))
-	       (effort-h (floor effort-in-minutes 60))
-	       (effort-m (- effort-in-minutes (* effort-h 60)))
 	       (work-done-str
 		(org-propertize
-		 (format org-time-clocksum-format h m)
+		 (org-minutes-to-clocksum-string clocked-time)
 		 'face (if (and org-clock-task-overrun (not org-clock-task-overrun-text))
 			   'org-mode-line-clock-overrun 'org-mode-line-clock)))
-	       (effort-str (format org-time-clocksum-format effort-h effort-m))
+	       (effort-str (org-minutes-to-clocksum-string effort-in-minutes))
 	       (clockstr (org-propertize
 			  (concat  " [%s/" effort-str
 				   "] (" (replace-regexp-in-string "%" "%%" org-clock-heading) ")")
 			  'face 'org-mode-line-clock)))
 	  (format clockstr work-done-str))
-      (org-propertize (format
-		       (concat "[" org-time-clocksum-format " (%s)]")
-		       h m org-clock-heading)
+      (org-propertize (concat "[" (org-minutes-to-clocksum-string clocked-time)
+			      (format " (%s)" org-clock-heading) "]")
 		      'face 'org-mode-line-clock))))
 
 (defun org-clock-get-last-clock-out-time ()
@@ -650,7 +645,7 @@ the mode line."
 	      (setq value (- current value))
 	    (if (equal ?+ sign) (setq value (+ current value)))))
 	(setq value (max 0 value)
-	      org-clock-effort (org-minutes-to-hh:mm-string value))
+	      org-clock-effort (org-minutes-to-clocksum-string value))
 	(org-entry-put org-clock-marker "Effort" org-clock-effort)
 	(org-clock-update-mode-line)
 	(message "Effort is now %s" org-clock-effort))
@@ -1528,8 +1523,9 @@ to, overriding the existing value of `org-clock-out-switch-to-state'."
 						"\\>"))))
 		  (org-todo org-clock-out-switch-to-state))))))
 	  (force-mode-line-update)
-	  (message (concat "Clock stopped at %s after HH:MM = " org-time-clocksum-format "%s") te h m
-		   (if remove " => LINE REMOVED" ""))
+	  (message (concat "Clock stopped at %s after "
+			   (org-minutes-to-clocksum-string (+ (* 60 h) m)) "%s")
+		   te (if remove " => LINE REMOVED" ""))
           (run-hooks 'org-clock-out-hook)
 	  (unless (org-clocking-p)
 	    (org-clock-delete-current)))))))
@@ -1797,12 +1793,9 @@ Use \\[org-clock-remove-overlays] to remove the subtree times."
 	(when org-remove-highlights-with-change
 	  (org-add-hook 'before-change-functions 'org-clock-remove-overlays
 			nil 'local))))
-    (if org-time-clocksum-use-fractional
-	(message (concat "Total file time: " org-time-clocksum-fractional-format
-			 " (%d hours and %d minutes)")
-		 (/ (+ (* h 60.0) m) 60.0) h m)
-      (message (concat "Total file time: " org-time-clocksum-format
-		       " (%d hours and %d minutes)") h m h m))))
+      (message (concat "Total file time: "
+		       (org-minutes-to-clocksum-string org-clock-file-total-minutes)
+		       " (%d hours and %d minutes)") h m)))
 
 (defvar org-clock-overlays nil)
 (make-variable-buffer-local 'org-clock-overlays)
@@ -1814,9 +1807,6 @@ This creates a new overlay and stores it in `org-clock-overlays', so that it
 will be easy to remove."
   (let* ((c 60) (h (floor (/ time 60))) (m (- time (* 60 h)))
 	 (l (if level (org-get-valid-level level 0) 0))
-	 (fmt (concat "%s " (if org-time-clocksum-use-fractional
-				org-time-clocksum-fractional-format
-			      org-time-clocksum-format) "%s"))
 	 (off 0)
 	 ov tx)
     (org-move-to-column c)
@@ -1825,14 +1815,9 @@ will be easy to remove."
     (setq ov (make-overlay (point-at-bol) (point-at-eol))
     	  tx (concat (buffer-substring (point-at-bol) (point))
 		     (make-string (+ off (max 0 (- c (current-column)))) ?.)
-		     (org-add-props (if org-time-clocksum-use-fractional
-					(format fmt
-						(make-string l ?*)
-						(/ (+ (* h 60.0) m) 60.0)
-						(make-string (- 16 l) ?\ ))
-				      (format fmt
-					      (make-string l ?*) h m
-					      (make-string (- 16 l) ?\ )))
+		     (org-add-props (concat (format "%s " (make-string l ?*))
+					    (org-minutes-to-clocksum-string time)
+					    (format "%s" (make-string (- 16 l) ?\ )))
 			 (list 'face 'org-clock-overlay))
 		     ""))
     (if (not (featurep 'xemacs))
@@ -2392,7 +2377,7 @@ from the dynamic block definition."
      (if properties (make-string (length properties) ?|) "")  ; properties columns, maybe
      (concat (format org-clock-total-time-cell-format (nth 7 lwords))  "| ") ; instead of a headline
      (format org-clock-total-time-cell-format
-	     (org-minutes-to-hh:mm-string (or total-time 0))) ; the time
+	     (org-minutes-to-clocksum-string (or total-time 0))) ; the time
      "|\n")                          ; close line
 
     ;; Now iterate over the tables and insert the data
@@ -2416,7 +2401,7 @@ from the dynamic block definition."
 		     (if level-p   "| " "") ; level column, maybe
 		     (if timestamp "| " "") ; timestamp column, maybe
 		     (if properties (make-string (length properties) ?|) "")  ;properties columns, maybe
-		     (org-minutes-to-hh:mm-string (nth 1 tbl))))) ; the time
+		     (org-minutes-to-clocksum-string (nth 1 tbl))))) ; the time
 
 	  ;; Get the list of node entries and iterate over it
 	  (setq entries (nth 2 tbl))
@@ -2449,7 +2434,7 @@ from the dynamic block definition."
 	     hlc headline hlc "|"                                ; headline
 	     (make-string (min (1- ntcol) (or (- level 1))) ?|)
 					; empty fields for higher levels
-	     hlc (org-minutes-to-hh:mm-string (nth 3 entry)) hlc ; time
+	     hlc (org-minutes-to-clocksum-string (nth 3 entry)) hlc ; time
 	     "|\n"                                               ; close line
 	     )))))
     ;; When exporting subtrees or regions the region might be
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 9d58b5f..d9afbd1 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1058,8 +1058,7 @@ Don't set this, this is meant for dynamic scoping.")
    ((memq fmt '(estimate)) (org-estimate-print n printf))
    ((not (numberp n)) "")
    ((memq fmt '(add_times max_times min_times mean_times))
-    (let* ((h (floor n)) (m (floor (+ 0.5 (* 60 (- n h))))))
-      (format org-time-clocksum-format h m)))
+    (org-hours-to-clocksum-string n))
    ((eq fmt 'checkbox)
     (cond ((= n (floor n)) "[X]")
 	  ((> n 1.) "[-]")
diff --git a/lisp/org.el b/lisp/org.el
index 080b527..d347b95 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2714,11 +2714,67 @@ commands, if custom time display is turned on at the time of export."
 	(concat "[" (substring f 1 -1) "]")
       f)))
 
-(defcustom org-time-clocksum-format "%d:%02d"
+(defcustom org-time-clocksum-format
+  '(:days ("%dd " . nil) :hours ("%d" . t) :minutes (":%02d" . t))
   "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.
+
+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.
+
+Alternatively, the value can be a plist associating any of the
+keys :years, :months, :weeks, :days, :hours or :minutes with a
+format. The time duration is formatted using only the time
+components that are needed and concatenating the results. If a
+time unit in absent, it falls back to the next smallest unit.
+
+The format values in the plist must be cons cells, containing a
+format string in the car and either t or nil in the cdr. To
+format a time component, the value is passed to the corresponding
+format string. A non-nil value in the cdr indicates that the
+corresponding time component should always be included, even if
+its value is 0.
+
+
+For example,
+
+  (:days (\"%dd\" . nil) :hours (\"%d\" . t)
+   :minutes (\":%02d\" . t))
+
+means durations longer than a day will be expressed in days,
+hours and minutes, and durations less than a day will always be
+expressed in hours and minutes (even for durations less than an
+hour).
+
+The value
+
+  (:days (\"%dd\" . nil) :minutes (\"%dm\" . nil))
+
+means durations longer than a day will be expressed in days and
+minutes, and durations less than a day will be expressed entirely
+in minutes (even for durations longer than an hour)."
   :group 'org-time
-  :type 'string)
+  :type '(choice (string :tag "Format string")
+		 (set :tag "Plist"
+		      (group :inline t (const :tag "Years" :years)
+			    (cons (string :tag "Format string")
+				  (boolean :tag "Required")))
+		      (group :inline t (const :tag "Months" :months)
+			    (cons (string :tag "Format string")
+				  (boolean :tag "Required" t)))
+		      (group :inline t (const :tag "Weeks" :weeks)
+			    (cons (string :tag "Format string")
+				  (boolean :tag "Required" t)))
+		      (group :inline t (const :tag "Days" :days)
+			    (cons (string :tag "Format string")
+				  (boolean :tag "Required" t)))
+		      (group :inline t (const :tag "Hours" :hours)
+			    (cons (string :tag "Format string")
+				  (boolean :tag "Required" t)))
+		      (group :inline t (const :tag "Minutes" :minutes)
+			    (cons (string :tag "Format string")
+				  (boolean :tag "Required" t))))))
 
 (defcustom org-time-clocksum-use-fractional nil
   "If non-nil, \\[org-clock-display] uses fractional times.
@@ -2727,10 +2783,33 @@ org-mode generates a time duration."
   :type 'boolean)
 
 (defcustom org-time-clocksum-fractional-format "%.2f"
-  "The format string used when creating CLOCKSUM lines, or when
-org-mode generates a time duration."
+  "The format string used when creating CLOCKSUM lines,
+or when Org mode generates a time duration, if
+`org-time-clocksum-use-fractional' is enabled.
+
+The value can be a single format string containing one
+%-sequence, which will be filled with the number of hours as a
+float.
+
+Alternatively, the value can be a plist associating any of the
+keys :years, :months, :weeks, :days, :hours or :minutes with a
+format string. The time duration is formatted using the largest
+time unit which gives a non-zero integer part. If all specified
+formats have zero integer part, the smallest time unit is used."
   :group 'org-time
-  :type 'string)
+  :type '(choice (string :tag "Format string")
+		 (set (group :inline t (const :tag "Years" :years)
+			    (string :tag "Format string"))
+		      (group :inline t (const :tag "Months" :months)
+			    (string :tag "Format string"))
+		      (group :inline t (const :tag "Weeks" :weeks)
+			     (string :tag "Format string"))
+		      (group :inline t (const :tag "Days" :days)
+			     (string :tag "Format string"))
+		      (group :inline t (const :tag "Hours" :hours)
+			     (string :tag "Format string"))
+		      (group :inline t (const :tag "Minutes" :minutes)
+			     (string :tag "Format string")))))
 
 (defcustom org-deadline-warning-days 14
   "No. of days before expiration during which a deadline becomes active.
@@ -16667,11 +16746,85 @@ If there is already a time stamp at the cursor position, update it."
       (org-insert-time-stamp
        (encode-time 0 0 0 (nth 1 cal-date) (car cal-date) (nth 2 cal-date))))))
 
-(defun org-minutes-to-hh:mm-string (m)
-  "Compute H:MM from a number of minutes."
-  (let ((h (/ m 60)))
-    (setq m (- m (* 60 h)))
-    (format org-time-clocksum-format h m)))
+(defun org-minutes-to-clocksum-string (m)
+  "Format number of minutes as a clocksum string.
+The format is determined by `org-time-clocksum-format',
+`org-time-clocksum-use-fractional' and
+`org-time-clocksum-fractional-format'."
+  (let ((clocksum "") fmt n)
+    ;; fractional format
+    (if org-time-clocksum-use-fractional
+	(cond
+	 ;; single format string
+	 ((stringp org-time-clocksum-fractional-format)
+	  (format org-time-clocksum-fractional-format (/ m 60.0)))
+	 ;; choice of formats for different time units
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :years))
+	       (> (/ m (* 365 24 60)) 0))
+	  (format fmt (/ m (* 365 24 60.0))))
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :months))
+	       (> (/ m (* 30 24 60)) 0))
+	  (format fmt (/ m (* 30 24 60.0))))
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :weeks))
+	       (> (/ m (* 7 24 60)) 0))
+	  (format fmt (/ m (* 7 24 60.0))))
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :days))
+	       (> (/ m (* 24 60)) 0))
+	  (format fmt (/ m (* 24 60.0))))
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :hours))
+	       (> (/ m 60) 0))
+	  (format fmt (/ m 60.0)))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :minutes))
+	  (format fmt m))
+	 ;; fall back to smallest time unit with a format
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :hours))
+	  (format fmt (/ m 60.0)))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :days))
+	  (format fmt (/ m (* 24 60.0))))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :weeks))
+	  (format fmt (/ m (* 7 24 60.0))))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :months))
+	  (format fmt (/ m (* 30 24 60.0))))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :years))
+	  (format fmt (/ m (* 365 24 60.0)))))
+      ;; standard (non-fractional) format
+      (and (setq fmt (plist-get org-time-clocksum-format :years))
+	   (or (> (setq n (/ m (* 365 24 60))) 0)
+	       (eq (cdr fmt) t))
+	   (setq clocksum (concat clocksum (format (car fmt) n))
+		 m (- m (* n 365 24 60))))
+      (and (setq fmt (plist-get org-time-clocksum-format :months))
+	   (or (> (setq n (/ m (* 30 24 60))) 0)
+	       (eq (cdr fmt) t))
+	   (setq clocksum (concat clocksum (format (car fmt) n))
+		 m (- m (* n 30 24 60))))
+      (and (setq fmt (plist-get org-time-clocksum-format :weeks))
+	   (or (> (setq n (/ m (* 7 24 60))) 0)
+	       (eq (cdr fmt) t))
+	   (setq clocksum (concat clocksum (format (car fmt) n))
+		 m (- m (* n 7 24 60))))
+      (and (setq fmt (plist-get org-time-clocksum-format :days))
+	   (or (> (setq n (/ m (* 24 60))) 0)
+	       (eq (cdr fmt) t))
+	   (setq clocksum (concat clocksum (format (car fmt) n))
+		 m (- m (* n 24 60))))
+      (and (setq fmt (plist-get org-time-clocksum-format :hours))
+	   (or (> (setq n (/ m 60)) 0)
+	       (eq (cdr fmt) t))
+	   (setq clocksum (concat clocksum (format (car fmt) n))
+		 m (- m (* n 60))))
+      (and (setq fmt (plist-get org-time-clocksum-format :minutes))
+	   (or (> m 0) (eq (cdr fmt) t))
+	   (setq clocksum (concat clocksum (format (car fmt) m))))
+      ;; return formatted time duration
+      clocksum)))
+
+(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")
+
+(defun org-hours-to-clocksum-string (n)
+  (org-minutes-to-clocksum-string (* n 60)))
 
 (defun org-hh:mm-string-to-minutes (s)
   "Convert a string H:MM to a number of minutes.
-- 
1.7.8.6

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> Here's an updated patch. Now both org-time-clocksum-format and
> org-time-clocksum-fractional-format can be plists, as discussed.

That was quick. Thank you.

> In the org-time-clocksum-format case, I made the values cons cells which
> specify both a format string and a boolean. The latter indicates whether
> the time component should always be included in the formatted duration,
> even if its value is 0. This is needed for the hours component to
> reproduce the current default format, and I figured I might as well make
> it general.

I understand. It is a necessary evil. Though, instead of asking for cons
cells, maybe the boolean could be provided as another property. I.e.

    '(:hour "..." :persistent-hour t)

would be a replacement for:

    '(:hour ("..." . t))

And, better,

    '(:hour "...")

would the become a replacement for

    '(:hour ("..." . nil))

What do you think about it? The name of the property is only
a suggestion.

> I used a somewhat complex customization type in the defcustoms, instead
> of a straight plist, in order to produce a better ui for the
> customization interface. I'm still not completely satisfied with it.
> E.g. it would be nice to get rid of the "Cons cell" tag entirely, and use
> a checkbox for the boolean. But I can't figure out how to do that
> (without defining new customization types/widgets, which I don't have the
> patience for).

The advantage of the method above it that it would /de facto/ get rid of
the "Cons cell" tag.

> +		     (org-add-props (concat (format "%s " (make-string l ?*))
> +					    (org-minutes-to-clocksum-string time)
> +					    (format "%s" (make-string (- 16 l) ?\ )))

You forgot to change that.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

[-- Attachment #1: Type: text/plain, Size: 2640 bytes --]

On Sat, Nov 17, 2012 at 09:48:09AM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> 
> > Here's an updated patch. Now both org-time-clocksum-format and
> > org-time-clocksum-fractional-format can be plists, as discussed.
> 
> That was quick. Thank you.
> 
> > In the org-time-clocksum-format case, I made the values cons cells which
> > specify both a format string and a boolean. The latter indicates whether
> > the time component should always be included in the formatted duration,
> > even if its value is 0. This is needed for the hours component to
> > reproduce the current default format, and I figured I might as well make
> > it general.
> 
> I understand. It is a necessary evil. Though, instead of asking for cons
> cells, maybe the boolean could be provided as another property. I.e.
> 
>     '(:hour "..." :persistent-hour t)
> 
> would be a replacement for:
> 
>     '(:hour ("..." . t))
> 
> And, better,
> 
>     '(:hour "...")
> 
> would the become a replacement for
> 
>     '(:hour ("..." . nil))
> 
> What do you think about it? The name of the property is only a
> suggestion.

Good idea. I agree, additional keys are cleaner than cons cells.

> > I used a somewhat complex customization type in the defcustoms,
> > instead of a straight plist, in order to produce a better ui for the
> > customization interface. I'm still not completely satisfied with it.
> > E.g. it would be nice to get rid of the "Cons cell" tag entirely, and
> > use a checkbox for the boolean. But I can't figure out how to do that
> > (without defining new customization types/widgets, which I don't have
> > the patience for).
> 
> The advantage of the method above it that it would /de facto/ get rid of
> the "Cons cell" tag.

I've replaced the cons cells with additional plist properties, as you
suggested. The resulting customization ui still isn't wonderful in my
opinion. But it does the job, and I'm not sure how much scope there is
for improving it further. If you see a way, by all means feel free to
make the changes yourself. I really don't mind what format you go with
for org-time-clocksum-format, as long as it supports the new formatting
features implemented in the patch.

> > +		     (org-add-props (concat (format "%s " (make-string l ?*))
> > +					    (org-minutes-to-clocksum-string time)
> > +					    (format "%s" (make-string (- 16 l) ?\ )))
> 
> You forgot to change that.

Ooops. Fixed in the attached version.

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

[-- Attachment #2: 0001-Allow-more-flexible-customization-of-clocksum-format.patch --]
[-- Type: text/x-patch, Size: 17260 bytes --]

From 6e87864c125676093d8072111519c37cf9dd126c Mon Sep 17 00:00:00 2001
From: "Toby S. Cubitt" <tsc25@cantab.net>
Date: Sun, 11 Nov 2012 22:20:24 +0000
Subject: [PATCH] Allow more flexible customization of clocksum format

* lisp/org.el (org-time-clocksum-format, org-time-clocksum-fractional-format):
in addition to a single format string, the clocksum formats can now be
plists specifying separate formats for different time units.

* lisp/org.el (org-minutes-to-clocksum-string): new function to
replace org-minutes-to-hh:mm-string, which converts a number of
minutes to a string according to the customization options.

* lisp/org-colview.el (org-columns-number-to-string): use new
org-minutes-to-clocksum-string function to format clocksum durations.

* lisp/org-clock.el: always call new org-minutes-to-clocksum-string
function when formatting time durations, instead of calling
org-minutes-to-hh:mm-string or passing org-time-clocksum-format
directly to format.
---
 lisp/org-clock.el   |   51 +++++---------
 lisp/org-colview.el |    3 +-
 lisp/org.el         |  190 ++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 198 insertions(+), 46 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 84eb2fd..54e4018 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -556,28 +556,23 @@ pointing to it."
 If an effort estimate was defined for the current item, use
 01:30/01:50 format (clocked/estimated).
 If not, show simply the clocked time like 01:50."
-  (let* ((clocked-time (org-clock-get-clocked-time))
-	 (h (floor clocked-time 60))
-	 (m (- clocked-time (* 60 h))))
+  (let ((clocked-time (org-clock-get-clocked-time)))
     (if org-clock-effort
 	(let* ((effort-in-minutes
 		(org-duration-string-to-minutes org-clock-effort))
-	       (effort-h (floor effort-in-minutes 60))
-	       (effort-m (- effort-in-minutes (* effort-h 60)))
 	       (work-done-str
 		(org-propertize
-		 (format org-time-clocksum-format h m)
+		 (org-minutes-to-clocksum-string clocked-time)
 		 'face (if (and org-clock-task-overrun (not org-clock-task-overrun-text))
 			   'org-mode-line-clock-overrun 'org-mode-line-clock)))
-	       (effort-str (format org-time-clocksum-format effort-h effort-m))
+	       (effort-str (org-minutes-to-clocksum-string effort-in-minutes))
 	       (clockstr (org-propertize
 			  (concat  " [%s/" effort-str
 				   "] (" (replace-regexp-in-string "%" "%%" org-clock-heading) ")")
 			  'face 'org-mode-line-clock)))
 	  (format clockstr work-done-str))
-      (org-propertize (format
-		       (concat "[" org-time-clocksum-format " (%s)]")
-		       h m org-clock-heading)
+      (org-propertize (concat "[" (org-minutes-to-clocksum-string clocked-time)
+			      (format " (%s)" org-clock-heading) "]")
 		      'face 'org-mode-line-clock))))
 
 (defun org-clock-get-last-clock-out-time ()
@@ -650,7 +645,7 @@ the mode line."
 	      (setq value (- current value))
 	    (if (equal ?+ sign) (setq value (+ current value)))))
 	(setq value (max 0 value)
-	      org-clock-effort (org-minutes-to-hh:mm-string value))
+	      org-clock-effort (org-minutes-to-clocksum-string value))
 	(org-entry-put org-clock-marker "Effort" org-clock-effort)
 	(org-clock-update-mode-line)
 	(message "Effort is now %s" org-clock-effort))
@@ -1528,8 +1523,9 @@ to, overriding the existing value of `org-clock-out-switch-to-state'."
 						"\\>"))))
 		  (org-todo org-clock-out-switch-to-state))))))
 	  (force-mode-line-update)
-	  (message (concat "Clock stopped at %s after HH:MM = " org-time-clocksum-format "%s") te h m
-		   (if remove " => LINE REMOVED" ""))
+	  (message (concat "Clock stopped at %s after "
+			   (org-minutes-to-clocksum-string (+ (* 60 h) m)) "%s")
+		   te (if remove " => LINE REMOVED" ""))
           (run-hooks 'org-clock-out-hook)
 	  (unless (org-clocking-p)
 	    (org-clock-delete-current)))))))
@@ -1797,12 +1793,9 @@ Use \\[org-clock-remove-overlays] to remove the subtree times."
 	(when org-remove-highlights-with-change
 	  (org-add-hook 'before-change-functions 'org-clock-remove-overlays
 			nil 'local))))
-    (if org-time-clocksum-use-fractional
-	(message (concat "Total file time: " org-time-clocksum-fractional-format
-			 " (%d hours and %d minutes)")
-		 (/ (+ (* h 60.0) m) 60.0) h m)
-      (message (concat "Total file time: " org-time-clocksum-format
-		       " (%d hours and %d minutes)") h m h m))))
+      (message (concat "Total file time: "
+		       (org-minutes-to-clocksum-string org-clock-file-total-minutes)
+		       " (%d hours and %d minutes)") h m)))
 
 (defvar org-clock-overlays nil)
 (make-variable-buffer-local 'org-clock-overlays)
@@ -1814,9 +1807,6 @@ This creates a new overlay and stores it in `org-clock-overlays', so that it
 will be easy to remove."
   (let* ((c 60) (h (floor (/ time 60))) (m (- time (* 60 h)))
 	 (l (if level (org-get-valid-level level 0) 0))
-	 (fmt (concat "%s " (if org-time-clocksum-use-fractional
-				org-time-clocksum-fractional-format
-			      org-time-clocksum-format) "%s"))
 	 (off 0)
 	 ov tx)
     (org-move-to-column c)
@@ -1825,14 +1815,9 @@ will be easy to remove."
     (setq ov (make-overlay (point-at-bol) (point-at-eol))
     	  tx (concat (buffer-substring (point-at-bol) (point))
 		     (make-string (+ off (max 0 (- c (current-column)))) ?.)
-		     (org-add-props (if org-time-clocksum-use-fractional
-					(format fmt
-						(make-string l ?*)
-						(/ (+ (* h 60.0) m) 60.0)
-						(make-string (- 16 l) ?\ ))
-				      (format fmt
-					      (make-string l ?*) h m
-					      (make-string (- 16 l) ?\ )))
+		     (org-add-props (concat (make-string l ?*) " "
+					    (org-minutes-to-clocksum-string time)
+					    (make-string (- 16 l) ?\ ))
 			 (list 'face 'org-clock-overlay))
 		     ""))
     (if (not (featurep 'xemacs))
@@ -2392,7 +2377,7 @@ from the dynamic block definition."
      (if properties (make-string (length properties) ?|) "")  ; properties columns, maybe
      (concat (format org-clock-total-time-cell-format (nth 7 lwords))  "| ") ; instead of a headline
      (format org-clock-total-time-cell-format
-	     (org-minutes-to-hh:mm-string (or total-time 0))) ; the time
+	     (org-minutes-to-clocksum-string (or total-time 0))) ; the time
      "|\n")                          ; close line
 
     ;; Now iterate over the tables and insert the data
@@ -2416,7 +2401,7 @@ from the dynamic block definition."
 		     (if level-p   "| " "") ; level column, maybe
 		     (if timestamp "| " "") ; timestamp column, maybe
 		     (if properties (make-string (length properties) ?|) "")  ;properties columns, maybe
-		     (org-minutes-to-hh:mm-string (nth 1 tbl))))) ; the time
+		     (org-minutes-to-clocksum-string (nth 1 tbl))))) ; the time
 
 	  ;; Get the list of node entries and iterate over it
 	  (setq entries (nth 2 tbl))
@@ -2449,7 +2434,7 @@ from the dynamic block definition."
 	     hlc headline hlc "|"                                ; headline
 	     (make-string (min (1- ntcol) (or (- level 1))) ?|)
 					; empty fields for higher levels
-	     hlc (org-minutes-to-hh:mm-string (nth 3 entry)) hlc ; time
+	     hlc (org-minutes-to-clocksum-string (nth 3 entry)) hlc ; time
 	     "|\n"                                               ; close line
 	     )))))
     ;; When exporting subtrees or regions the region might be
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 9d58b5f..d9afbd1 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1058,8 +1058,7 @@ Don't set this, this is meant for dynamic scoping.")
    ((memq fmt '(estimate)) (org-estimate-print n printf))
    ((not (numberp n)) "")
    ((memq fmt '(add_times max_times min_times mean_times))
-    (let* ((h (floor n)) (m (floor (+ 0.5 (* 60 (- n h))))))
-      (format org-time-clocksum-format h m)))
+    (org-hours-to-clocksum-string n))
    ((eq fmt 'checkbox)
     (cond ((= n (floor n)) "[X]")
 	  ((> n 1.) "[-]")
diff --git a/lisp/org.el b/lisp/org.el
index 080b527..b43de9d 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2714,11 +2714,79 @@ commands, if custom time display is turned on at the time of export."
 	(concat "[" (substring f 1 -1) "]")
       f)))
 
-(defcustom org-time-clocksum-format "%d:%02d"
+(defcustom org-time-clocksum-format
+  '(:days "%dd " :hours "%d" :require-hours t :minutes ":%02d" :require-minutes t)
   "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.
+
+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.
+
+Alternatively, the value can be a plist associating any of the
+keys :years, :months, :weeks, :days, :hours or :minutes with
+format strings. The time duration is formatted using only the
+time components that are needed and concatenating the results. If
+a time unit in absent, it falls back to the next smallest unit.
+
+The keys :require-years, :require-months, :require-days,
+:require-weeks, :require-hours, minutes-required are also
+meaningful. A non-nil value for these keys indicates that the
+corresponding time component should always be included, even if
+its value is 0.
+
+
+For example,
+
+  (:days (\"%dd\" . nil) :hours (\"%d\" . t)
+   :minutes (\":%02d\" . t))
+
+means durations longer than a day will be expressed in days,
+hours and minutes, and durations less than a day will always be
+expressed in hours and minutes (even for durations less than an
+hour).
+
+The value
+
+  (:days (\"%dd\" . nil) :minutes (\"%dm\" . nil))
+
+means durations longer than a day will be expressed in days and
+minutes, and durations less than a day will be expressed entirely
+in minutes (even for durations longer than an hour)."
   :group 'org-time
-  :type 'string)
+  :type '(choice (string :tag "Format string")
+		 (set :tag "Plist"
+		      (group :inline t (const :tag "Years" :years)
+			     (string :tag "Format string"))
+		      (group :inline t
+			     (const :tag "Always show years" :require-years)
+			     (const t))
+		      (group :inline t (const :tag "Months" :months)
+			     (string :tag "Format string"))
+		      (group :inline t
+			     (const :tag "Always show months" :require-months)
+			     (const t))
+		      (group :inline t (const :tag "Weeks" :weeks)
+			     (string :tag "Format string"))
+		      (group :inline t
+			     (const :tag "Always show weeks" :require-weeks)
+			     (const t))
+		      (group :inline t (const :tag "Days" :days)
+			     (string :tag "Format string"))
+		      (group :inline t
+			     (const :tag "Always show days" :require-days)
+			     (const t))
+		      (group :inline t (const :tag "Hours" :hours)
+			     (string :tag "Format string"))
+		      (group :inline t
+			     (const :tag "Always show hours" :require-hours)
+			     (const t))
+		      (group :inline t (const :tag "Minutes" :minutes)
+			     (string :tag "Format string"))
+		      (group :inline t
+			     (const :tag "Always show minutes" :require-minutes)
+			     (const t))
+		      )))
 
 (defcustom org-time-clocksum-use-fractional nil
   "If non-nil, \\[org-clock-display] uses fractional times.
@@ -2727,10 +2795,33 @@ org-mode generates a time duration."
   :type 'boolean)
 
 (defcustom org-time-clocksum-fractional-format "%.2f"
-  "The format string used when creating CLOCKSUM lines, or when
-org-mode generates a time duration."
+  "The format string used when creating CLOCKSUM lines,
+or when Org mode generates a time duration, if
+`org-time-clocksum-use-fractional' is enabled.
+
+The value can be a single format string containing one
+%-sequence, which will be filled with the number of hours as a
+float.
+
+Alternatively, the value can be a plist associating any of the
+keys :years, :months, :weeks, :days, :hours or :minutes with a
+format string. The time duration is formatted using the largest
+time unit which gives a non-zero integer part. If all specified
+formats have zero integer part, the smallest time unit is used."
   :group 'org-time
-  :type 'string)
+  :type '(choice (string :tag "Format string")
+		 (set (group :inline t (const :tag "Years" :years)
+			    (string :tag "Format string"))
+		      (group :inline t (const :tag "Months" :months)
+			    (string :tag "Format string"))
+		      (group :inline t (const :tag "Weeks" :weeks)
+			     (string :tag "Format string"))
+		      (group :inline t (const :tag "Days" :days)
+			     (string :tag "Format string"))
+		      (group :inline t (const :tag "Hours" :hours)
+			     (string :tag "Format string"))
+		      (group :inline t (const :tag "Minutes" :minutes)
+			     (string :tag "Format string")))))
 
 (defcustom org-deadline-warning-days 14
   "No. of days before expiration during which a deadline becomes active.
@@ -16667,11 +16758,88 @@ If there is already a time stamp at the cursor position, update it."
       (org-insert-time-stamp
        (encode-time 0 0 0 (nth 1 cal-date) (car cal-date) (nth 2 cal-date))))))
 
-(defun org-minutes-to-hh:mm-string (m)
-  "Compute H:MM from a number of minutes."
-  (let ((h (/ m 60)))
-    (setq m (- m (* 60 h)))
-    (format org-time-clocksum-format h m)))
+(defun org-minutes-to-clocksum-string (m)
+  "Format number of minutes as a clocksum string.
+The format is determined by `org-time-clocksum-format',
+`org-time-clocksum-use-fractional' and
+`org-time-clocksum-fractional-format'."
+  (let ((clocksum "") fmt n)
+    ;; fractional format
+    (if org-time-clocksum-use-fractional
+	(cond
+	 ;; single format string
+	 ((stringp org-time-clocksum-fractional-format)
+	  (format org-time-clocksum-fractional-format (/ m 60.0)))
+	 ;; choice of fractional formats for different time units
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :years))
+	       (> (/ m (* 365 24 60)) 0))
+	  (format fmt (/ m (* 365 24 60.0))))
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :months))
+	       (> (/ m (* 30 24 60)) 0))
+	  (format fmt (/ m (* 30 24 60.0))))
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :weeks))
+	       (> (/ m (* 7 24 60)) 0))
+	  (format fmt (/ m (* 7 24 60.0))))
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :days))
+	       (> (/ m (* 24 60)) 0))
+	  (format fmt (/ m (* 24 60.0))))
+	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :hours))
+	       (> (/ m 60) 0))
+	  (format fmt (/ m 60.0)))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :minutes))
+	  (format fmt m))
+	 ;; fall back to smallest time unit with a format
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :hours))
+	  (format fmt (/ m 60.0)))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :days))
+	  (format fmt (/ m (* 24 60.0))))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :weeks))
+	  (format fmt (/ m (* 7 24 60.0))))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :months))
+	  (format fmt (/ m (* 30 24 60.0))))
+	 ((setq fmt (plist-get org-time-clocksum-fractional-format :years))
+	  (format fmt (/ m (* 365 24 60.0)))))
+      ;; standard (non-fractional) format, with single format string
+      (if (stringp org-time-clocksum-format)
+	  (format org-time-clocksum-format (setq n (/ m 60)) (- m (* 60 n)))
+	;; separate formats components
+	(and (setq fmt (plist-get org-time-clocksum-format :years))
+	     (or (> (setq n (/ m (* 365 24 60))) 0)
+		 (plist-get org-time-clocksum-format :require-years))
+	     (setq clocksum (concat clocksum (format fmt n))
+		   m (- m (* n 365 24 60))))
+	(and (setq fmt (plist-get org-time-clocksum-format :months))
+	     (or (> (setq n (/ m (* 30 24 60))) 0)
+		 (plist-get org-time-clocksum-format :require-months))
+	     (setq clocksum (concat clocksum (format fmt n))
+		   m (- m (* n 30 24 60))))
+	(and (setq fmt (plist-get org-time-clocksum-format :weeks))
+	     (or (> (setq n (/ m (* 7 24 60))) 0)
+		 (plist-get org-time-clocksum-format :require-weeks))
+	     (setq clocksum (concat clocksum (format fmt n))
+		   m (- m (* n 7 24 60))))
+	(and (setq fmt (plist-get org-time-clocksum-format :days))
+	     (or (> (setq n (/ m (* 24 60))) 0)
+		 (plist-get org-time-clocksum-format :require-days))
+	     (setq clocksum (concat clocksum (format fmt n))
+		   m (- m (* n 24 60))))
+	(and (setq fmt (plist-get org-time-clocksum-format :hours))
+	     (or (> (setq n (/ m 60)) 0)
+		 (plist-get org-time-clocksum-format :require-hours))
+	     (setq clocksum (concat clocksum (format fmt n))
+		   m (- m (* n 60))))
+	(and (setq fmt (plist-get org-time-clocksum-format :minutes))
+	     (or (> m 0) (plist-get org-time-clocksum-format :require-minutes))
+	     (setq clocksum (concat clocksum (format fmt m))))
+	;; return formatted time duration
+	clocksum))))
+
+(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")
+
+(defun org-hours-to-clocksum-string (n)
+  (org-minutes-to-clocksum-string (* n 60)))
 
 (defun org-hh:mm-string-to-minutes (s)
   "Convert a string H:MM to a number of minutes.
-- 
1.7.8.6

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> I've replaced the cons cells with additional plist properties, as you
> suggested. The resulting customization ui still isn't wonderful in my
> opinion. But it does the job, and I'm not sure how much scope there is
> for improving it further. If you see a way, by all means feel free to
> make the changes yourself. I really don't mind what format you go with
> for org-time-clocksum-format, as long as it supports the new formatting
> features implemented in the patch.

Considering I'm not an expert in customize ui, it's good enough as it
is.

I've applied your patch (with some small changes in a docstring). Thank
you again for all that work.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Sat, Nov 17, 2012 at 03:42:24PM +0100, Nicolas Goaziou wrote:
> Toby Cubitt <tsc25@cantab.net> writes:
> 
> > I've replaced the cons cells with additional plist properties, as you
> > suggested. The resulting customization ui still isn't wonderful in my
> > opinion. But it does the job, and I'm not sure how much scope there is
> > for improving it further. If you see a way, by all means feel free to
> > make the changes yourself. I really don't mind what format you go with
> > for org-time-clocksum-format, as long as it supports the new formatting
> > features implemented in the patch.
> 
> Considering I'm not an expert in customize ui, it's good enough as it
> is.

OK. If someone thinks of a way to improve the customization ui (keeping
the same data type), it's easy to change it later without breaking
anything.

> I've applied your patch (with some small changes in a docstring). Thank
> you again for all that work.

Glad we finally found a good implementation. Thanks to you for all your
helpful feedback.

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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Mike McLean]

[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]

On Sat, Nov 17, 2012 at 11:02 AM, Toby Cubitt <tsc25@cantab.net> wrote:

> On Sat, Nov 17, 2012 at 03:42:24PM +0100, Nicolas Goaziou wrote:
> > Toby Cubitt <tsc25@cantab.net> writes:
> >
> > > I've replaced the cons cells with additional plist properties, as you
> > > suggested. The resulting customization ui still isn't wonderful in my
> > > opinion. But it does the job, and I'm not sure how much scope there is
> > > for improving it further. If you see a way, by all means feel free to
> > > make the changes yourself. I really don't mind what format you go with
> > > for org-time-clocksum-format, as long as it supports the new formatting
> > > features implemented in the patch.
> >
> > Considering I'm not an expert in customize ui, it's good enough as it
> > is.
>
> OK. If someone thinks of a way to improve the customization ui (keeping
> the same data type), it's easy to change it later without breaking
> anything.
>
> > I've applied your patch (with some small changes in a docstring). Thank
> > you again for all that work.
>
> Glad we finally found a good implementation. Thanks to you for all your
> helpful feedback.
>
>
I like the new implementation and customization options, but now when I do
~C-c C-x C-c~ to get a sub-tree time, I get nothing but zeros (~0d 0:00~)
for every subtree.

Mike

[-- Attachment #2: Type: text/html, Size: 1954 bytes --]

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

[-- Attachment #1: Type: text/plain, Size: 1983 bytes --]

On Tue, Nov 20, 2012 at 11:12:10AM -0500, Mike McLean wrote:
> On Sat, Nov 17, 2012 at 11:02 AM, Toby Cubitt <tsc25@cantab.net> wrote:
> 
> > On Sat, Nov 17, 2012 at 03:42:24PM +0100, Nicolas Goaziou wrote:
> > > Toby Cubitt <tsc25@cantab.net> writes:
> > >
> > > > I've replaced the cons cells with additional plist properties, as you
> > > > suggested. The resulting customization ui still isn't wonderful in my
> > > > opinion. But it does the job, and I'm not sure how much scope there is
> > > > for improving it further. If you see a way, by all means feel free to
> > > > make the changes yourself. I really don't mind what format you go with
> > > > for org-time-clocksum-format, as long as it supports the new formatting
> > > > features implemented in the patch.
> > >
> > > Considering I'm not an expert in customize ui, it's good enough as it
> > > is.
> >
> > OK. If someone thinks of a way to improve the customization ui (keeping
> > the same data type), it's easy to change it later without breaking
> > anything.
> >
> > > I've applied your patch (with some small changes in a docstring). Thank
> > > you again for all that work.
> >
> > Glad we finally found a good implementation. Thanks to you for all your
> > helpful feedback.
> >
> >
> I like the new implementation and customization options, but now when I do
> ~C-c C-x C-c~ to get a sub-tree time, I get nothing but zeros (~0d 0:00~)
> for every subtree.

Argh. This is a bug in org-hours-to-clocksum-string, which doesn't
truncate the computed number of minutes to an integer.

Instead of changing org-hours-to-clocksum-string, it's probably
preferable to make org-minutes-to-clocksum-string more robust, so it
copes correctly with floating point arguments.

The attached patch does this. Sorry for letting this slip through.

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

[-- Attachment #2: 0001-Fix-org-minutes-to-clocksum-string-to-cope-with-floa.patch --]
[-- Type: text/x-patch, Size: 3372 bytes --]

From 24c646916a1195b3291067ef6e54d9e99a1201da Mon Sep 17 00:00:00 2001
From: "Toby S. Cubitt" <tsc25@cantab.net>
Date: Tue, 20 Nov 2012 18:15:21 +0100
Subject: [PATCH] Fix org-minutes-to-clocksum-string to cope with floating
 point arguments.

---
 lisp/org.el |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index dc411b8..e3354c6 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16773,19 +16773,19 @@ The format is determined by `org-time-clocksum-format',
 	  (format org-time-clocksum-fractional-format (/ m 60.0)))
 	 ;; choice of fractional formats for different time units
 	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :years))
-	       (> (/ m (* 365 24 60)) 0))
+	       (> (/ (truncate m) (* 365 24 60)) 0))
 	  (format fmt (/ m (* 365 24 60.0))))
 	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :months))
-	       (> (/ m (* 30 24 60)) 0))
+	       (> (/ (truncate m) (* 30 24 60)) 0))
 	  (format fmt (/ m (* 30 24 60.0))))
 	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :weeks))
-	       (> (/ m (* 7 24 60)) 0))
+	       (> (/ (truncate m) (* 7 24 60)) 0))
 	  (format fmt (/ m (* 7 24 60.0))))
 	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :days))
-	       (> (/ m (* 24 60)) 0))
+	       (> (/ (truncate m) (* 24 60)) 0))
 	  (format fmt (/ m (* 24 60.0))))
 	 ((and (setq fmt (plist-get org-time-clocksum-fractional-format :hours))
-	       (> (/ m 60) 0))
+	       (> (/ (truncate m) 60) 0))
 	  (format fmt (/ m 60.0)))
 	 ((setq fmt (plist-get org-time-clocksum-fractional-format :minutes))
 	  (format fmt m))
@@ -16805,27 +16805,27 @@ The format is determined by `org-time-clocksum-format',
 	  (format org-time-clocksum-format (setq n (/ m 60)) (- m (* 60 n)))
 	;; separate formats components
 	(and (setq fmt (plist-get org-time-clocksum-format :years))
-	     (or (> (setq n (/ m (* 365 24 60))) 0)
+	     (or (> (setq n (/ (truncate m) (* 365 24 60))) 0)
 		 (plist-get org-time-clocksum-format :require-years))
 	     (setq clocksum (concat clocksum (format fmt n))
 		   m (- m (* n 365 24 60))))
 	(and (setq fmt (plist-get org-time-clocksum-format :months))
-	     (or (> (setq n (/ m (* 30 24 60))) 0)
+	     (or (> (setq n (/ (truncate m) (* 30 24 60))) 0)
 		 (plist-get org-time-clocksum-format :require-months))
 	     (setq clocksum (concat clocksum (format fmt n))
 		   m (- m (* n 30 24 60))))
 	(and (setq fmt (plist-get org-time-clocksum-format :weeks))
-	     (or (> (setq n (/ m (* 7 24 60))) 0)
+	     (or (> (setq n (/ (truncate m) (* 7 24 60))) 0)
 		 (plist-get org-time-clocksum-format :require-weeks))
 	     (setq clocksum (concat clocksum (format fmt n))
 		   m (- m (* n 7 24 60))))
 	(and (setq fmt (plist-get org-time-clocksum-format :days))
-	     (or (> (setq n (/ m (* 24 60))) 0)
+	     (or (> (setq n (/ (truncate m) (* 24 60))) 0)
 		 (plist-get org-time-clocksum-format :require-days))
 	     (setq clocksum (concat clocksum (format fmt n))
 		   m (- m (* n 24 60))))
 	(and (setq fmt (plist-get org-time-clocksum-format :hours))
-	     (or (> (setq n (/ m 60)) 0)
+	     (or (> (setq n (/ (truncate m) 60)) 0)
 		 (plist-get org-time-clocksum-format :require-hours))
 	     (setq clocksum (concat clocksum (format fmt n))
 		   m (- m (* n 60))))
-- 
1.7.8.6

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Nicolas Goaziou]

Toby Cubitt <tsc25@cantab.net> writes:

> Instead of changing org-hours-to-clocksum-string, it's probably
> preferable to make org-minutes-to-clocksum-string more robust, so it
> copes correctly with floating point arguments.
>
> The attached patch does this. Sorry for letting this slip through.

Applied. Thank you.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Mike McLean]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

On Tuesday, November 20, 2012, Nicolas Goaziou wrote:

> Toby Cubitt <tsc25@cantab.net <javascript:;>> writes:
>
> > Instead of changing org-hours-to-clocksum-string, it's probably
> > preferable to make org-minutes-to-clocksum-string more robust, so it
> > copes correctly with floating point arguments.
> >
> > The attached patch does this. Sorry for letting this slip through.
>
> Applied. Thank you.


Yes, thank you!

[-- Attachment #2: Type: text/html, Size: 714 bytes --]

[bug] Separate clocksum format for durations >= 1 day

[Sebastien Vauban]

Nicolas, Toby,

Nicolas Goaziou wrote:
> I've applied your patch (with some small changes in a docstring). Thank
> you again for all that work.

Here an ECM which gave correct results before commit a00a7b2, and which does
not anymore:

--8<---------------cut here---------------start------------->8---
#+TITLE:     ECM Prestas
#+Time-stamp: <2012-11-30 Fri 12:12>
#+LANGUAGE:  en
#+OPTIONS:   H:4 num:t toc:t

* Tasks

Some tasks with CLOCK lines.

** Design

*** TODO Do this
    :LOGBOOK:
    CLOCK: [2012-11-19 Mon 09:00]--[2012-11-19 Mon 10:11] =>  1:11
    :END:
    :PROPERTIES:
    :Effort:   2:00
    :END:

** Develop

*** TODO Do that
    :LOGBOOK:
    CLOCK: [2012-11-19 Mon 10:00]--[2012-11-19 Mon 12:22] =>  2:22
    CLOCK: [2012-11-20 Tue 10:01]--[2012-11-20 Tue 12:35] =>  2:34
    :END:

* Reporting

** Summary

#+TBLNAME: prestasTotal
#+BEGIN: clocktable :maxlevel 4 :scope file :block 2012-11 :lang "en" :narrow 80! :indent t
#+CAPTION: Clock summary at [2012-11-30 Fri 12:12], for November 2012.
| Headline            | Time |      |      |
|---------------------+------+------+------|
| Total time          | 6:07 |      |      |
|---------------------+------+------+------|
| Tasks               | 6:07 |      |      |
| \__ Design          |      | 1:11 |      |
| \_____ TODO Do this |      |      | 1:11 |
| \__ Develop         |      | 4:56 |      |
| \_____ TODO Do that |      |      | 4:56 |
#+END:

** Bill

| Total of worked hours                  | 6.12 | hours |
| Total of worked days (in days of 7:36) | 0.81 | days  |
#+TBLFM: @1$2=remote(prestasTotal,@2$2);t::@2$2=@1$2/7.6;%.2f
--8<---------------cut here---------------end--------------->8---

Now, with current master, the "bill" table returns:

--8<---------------cut here---------------start------------->8---
| Total of worked hours                  | 0.00 | hours |
| Total of worked days (in days of 7:36) | 0.00 | days  |
#+TBLFM: @1$2=remote(prestasTotal,@2$2);t::@2$2=@1$2/7.6;%.2f
--8<---------------cut here---------------end--------------->8---

I don't know (yet -- lack of time) if it's possible that, by setting some vars
to non-default values, the above problem would disappear; but I would expect
the defaults to conserve the old behavior anyway, wouldn't I?

Best regards,
  Seb

--
Sebastien Vauban

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Achim Gratz]

Nicolas Goaziou writes:
> Again, these are strange and very limiting rules. What if I want to have
> "5 h 32 min"? And "5,3 days"?

You provide a format string like you do now.

> Achim didn't specify how he conceives the FMT argument.

I thought that's obvious: the canned formats (the ones you'd want to use
functions for) will by symbols that the cond compares against and
anything else must be a string that can be given to format, like the
custom we have now.  That also takes care of staying backwards
compatible (some folks might actually have customized that variable).
If that turns out to be too limiting we can still decide to parse
fancier format strings and feed the correct arguments into them.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Toby Cubitt]

On Tue, Nov 06, 2012 at 07:42:54PM +0100, Achim Gratz wrote:
> Nicolas Goaziou writes:
> > Again, these are strange and very limiting rules. What if I want to have
> > "5 h 32 min"? And "5,3 days"?
> 
> You provide a format string like you do now.

That means if you want a format that's conditional on the duration, then
you have to use whatever hard-coded formats are provided by the canned
options. Conversely, if you want to customize the format you can't make
it conditional on the duration.

Same applies to regular vs. fractional if we also decide do that via
canned formats, to reduce the ridiculous number of defcustoms (3) to
customize this simple feature (which has also taken a ridiculous number
of emails to discuss ;-)

> > Achim didn't specify how he conceives the FMT argument.
> 
> I thought that's obvious: the canned formats (the ones you'd want to use
> functions for) will by symbols that the cond compares against and
> anything else must be a string that can be given to format, like the
> custom we have now.  That also takes care of staying backwards
> compatible (some folks might actually have customized that variable).
> If that turns out to be too limiting we can still decide to parse
> fancier format strings and feed the correct arguments into them.

Including the changes in my patch, we're trying to customize three
orthogonal things:

1. Conditioning what we display on the duration.

2. Regular vs. fractional minutes.

3. Customizing the formatting.

Any combination of choices for these three factors is valid. 1 affects
which values we compute. 2 affects how we compute one of the values. 3
affects how we display those values.

Your proposal doesn't allow 3 to be customized independently of 1 and 2.

But it seems Nicolas wants to get rid of customizable format strings
anyway, to allow clocksum durations to be parsed back into a number of
minutes (see other part of this thread). For this, a single defcustom for
choosing pre-canned formats with no user-defined functions and no
customizable format strings is the only (sane) option. In which case this
discussion is somewhat moot.

Best,
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

Re: [PATCH] Separate clocksum format for durations >= 1 day

[Achim Gratz]

Toby Cubitt writes:
> On Tue, Nov 06, 2012 at 07:42:54PM +0100, Achim Gratz wrote:
>> Nicolas Goaziou writes:
>> > Again, these are strange and very limiting rules. What if I want to have
>> > "5 h 32 min"? And "5,3 days"?
>> 
>> You provide a format string like you do now.
>
> That means if you want a format that's conditional on the duration, then
> you have to use whatever hard-coded formats are provided by the canned
> options.

Yes.  The idea is to not open that can of worms, to speak figuratively.

> Conversely, if you want to customize the format you can't make
> it conditional on the duration.

Just as it is right now.  I'm having no vested interest, but I'd
hesitate to take that option away, somebody else might rely on it.

[...]
> Including the changes in my patch, we're trying to customize three
> orthogonal things:
>
> 1. Conditioning what we display on the duration.
>
> 2. Regular vs. fractional minutes.
>
> 3. Customizing the formatting.
>
> Any combination of choices for these three factors is valid. 1 affects
> which values we compute. 2 affects how we compute one of the values. 3
> affects how we display those values.
>
> Your proposal doesn't allow 3 to be customized independently of 1 and 2.

There's nothing to stop us from looking at (and maybe modifying) the
format string and decide whether integer or fractional is requested, but
let's start simple.  The canned recipes are easily extended to include
both possibilities.

> But it seems Nicolas wants to get rid of customizable format strings
> anyway, to allow clocksum durations to be parsed back into a number of
> minutes (see other part of this thread).

I can see how this would help some things, but then you'd surely need to
clamp down on customization for good and just offer a selection from
which to chose or require that a user-supplied format string meets
certain criteria or be accompanied with a parser that undoes the
formatting.  But that gets ugly fast and doesn't help much with legacy
documents…

> For this, a single defcustom for choosing pre-canned formats with no
> user-defined functions and no customizable format strings is the only
> (sane) option. In which case this discussion is somewhat moot.

Indeed.  But let's wait for Nicolas.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada