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

[Aaron Ecay <aaronecay@gmail.com>]

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