Hello, Aaron Ecay <aarone...@gmail.com> writes:
> I had a few questions/comments though: > > - Given that the smallest unit of duration is the minute, The base unit is the minute, but nobody prevents you from adding a smaller unit: (let ((org-duration-units (cons `("s" . ,(/ 1 60.0)) org-duration-units))) (org-duration-from-minutes 1.5 '(("min") ("s")))) => "1min 30s" > why are seconds a choice in h:mm:ss? Wonʼt they always be zero? Internally, durations are floats because of seconds. (org-duration-from-minutes 1.5 'h:mm:ss) => "0:01:30" > Maybe it is > better not to offer this choice; I think it is potentially confusing > (giving the impression that durations might be accurate to the > second). Durations _can_ be accurate to the second. This library can be used outside clocksum computations, which are, indeed, accurate only to the minute. > - I would remove the h:mm symbol shorthand. It can still be offered as > a convenient option in customize using (const :tag "Use H:MM" (special > . h:mm)), but making it a special value with its own semantics makes > the system harder to understand for those who write their config in > lisp (rather than using customize). Using a list means you want to use special units to display the duration. However when the value is '((special . h:mm)), there is no unit to display the duration with, so '((special . h:mm)) is the degenerate case, not `h:mm'. Mind you, I'm not opposed to removing `h:mm'. I'm just pointing out the rational behind the initial choice. Moreover, if we remove `h:mm', we need to replace calls like (org-duration-from-minutes 120 'h:mm) with (org-duration-from-minutes 120 '((special . h:mm))) which is slightly uglier. > - The fact that the special options are grouped under the key “special” > is a bit confusing. If it isnʼt too much work, I would recommend > restructuring the options slightly to be (use-h:mm . t) and (precision > . INT). This more closely matches my intuition about how alists like > this are used. I chose `special' for a reason: these options are mutually exclusive. Using the same key, the structure (i.e., the alist) makes them mutually exclusive, too. With your suggestion, however, nothing prevents an user to have '((use-h:mm . t) (precision . 2)) and complain if strange things happen. So, I'd rather keep the same car. I'm not married to `special' though. > - Must the list be in decreasing order of unit size, or does everything > work if itʼs in arbitrary order? The documentation doesnʼt make it > clear one way or the other. There is no restriction about the order. > The value should be a list of entries each following this pattern: > > (UNIT . REQUIRED) I'd favor REQUIRED? over REQUIRED because it is a boolean. > UNIT is one of the unit strings defined in `org-duration-units'. A > duration is formatted using only the time components that are specified > here. If a time unit in missing, formatting falls back to the next > smallest unit. The algorithm works the other way: it consider biggest units first (greedy algorithm). > At the end of the list, there can be an entry indicating special formatting "The end of the list" is not accurate. The entry can be anywhere. Also, I reworded that part already in master. You may want to have a look at it. Regards, -- Nicolas Goaziou 0x80A93738