Re: [O] [PATCH 3/9] org-clock: fix `org-clock-time%'

[Aaron Ecay]

Hi Jan,

A couple stylistic comments.

2015ko azaroak 3an, Jan Malakhovski-ek idatzi zuen:
> 
> * lisp/org-clock.el (org-clock-time%): Respect org-effort-durations.
> 
> This also fixes a bug with time percents looking pretty much random and adding
> to a number that is less than 100% when a clock report has long intervals
> (e.g. days).
> ---
>  lisp/org-clock.el | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/lisp/org-clock.el b/lisp/org-clock.el
> index ad423f1..4563a8a 100644
> --- a/lisp/org-clock.el
> +++ b/lisp/org-clock.el
> @@ -2867,27 +2867,20 @@ TIME:      The sum of all time spend in this tree, in 
> minutes.  This time
>  
>  (defun org-clock-time% (total &rest strings)
>    "Compute a time fraction in percent.
> -TOTAL s a time string like 10:21 specifying the total times.
> +TOTAL s a total time string.
>  STRINGS is a list of strings that should be checked for a time.
> -The first string that does have a time will be used.
> -This function is made for clock tables."
> -  (let ((re "\\([0-9]+\\):\\([0-9]+\\)")
> -     tot s)
> -    (save-match-data
> +Strings are parsed using `org-duration-string-to-minutes`.
> +The first string that does have a time will be used. This
> +function is made for clock tables."
> +  (save-match-data
> +    (let (tot s cur)
>        (catch 'exit
> -     (if (not (string-match re total))
> -         (throw 'exit 0.)
> -       (setq tot (+ (string-to-number (match-string 2 total))
> -                    (* 60 (string-to-number (match-string 1 total)))))
> -       (if (= tot 0.) (throw 'exit 0.)))
> +     (setq tot (org-duration-string-to-minutes total))
> +     (if (= tot 0.) (throw 'exit 0.))
>       (while (setq s (pop strings))

This could be converted to dolist while you’re here (I realize you
didn’t touch this line).

> -       (if (string-match "\\([0-9]+\\):\\([0-9]+\\)" s)
> -           (throw 'exit
> -                  (/ (* 100.0 (+ (string-to-number (match-string 2 s))
> -                                 (* 60 (string-to-number
> -                                        (match-string 1 s)))))
> -                     tot))))
> -     0))))
> +       (setq cur (org-clocksum-string-to-minutes s))
> +       (if (not (equal cur nil)) (throw 'exit (/ (* 100.0 cur) tot))))

(when cur (throw 'exit ...))

-- 
Aaron Ecay