From 57ca9ba80a8527e2ecbcf17c057d9eea52edf2e3 Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Thu, 29 Dec 2016 01:19:00 +0100
Subject: [PATCH] org-clock: Fix ":formula %" with disparate duration formats

* lisp/org-clock.el (org-clock-time%): Handle various duration
  formats.  Refactor code.

* testing/lisp/test-org-clock.el (test-org-clock/clocktable): Add
  tests.

Reported-by: Andreas Mueller <lists@0x7.ch>
<http://permalink.gmane.org/gmane.emacs.orgmode/111067>
---
 lisp/org-clock.el              | 27 ++++++++------------
 testing/lisp/test-org-clock.el | 46 +++++++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 6e58ce91a..b1d35b3aa 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -2852,23 +2852,16 @@ TOTAL s a time string like 10:21 specifying the total times.
 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
-      (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.)))
-	(while (setq s (pop strings))
-	  (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))))
+  (save-match-data
+    (let ((total (org-duration-string-to-minutes total)))
+      (if (= total 0) 0
+	(cl-some (lambda (s)
+		   ;; Any number can express a duration.  See
+		   ;; `org-hh:mm-string-to-minutes' for details.
+		   (and (string-match-p "[0-9]" s)
+			(/ (* 100.0 (org-duration-string-to-minutes s))
+			   total)))
+		 strings)))))
 
 ;; Saving and loading the clock
 
diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index f99affc12..27fd00ba8 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -359,7 +359,51 @@ CLOCK: [2012-03-29 Thu 16:40]--[2014-03-04 Thu 00:41] => 16905:01
       (forward-line 2)
       (buffer-substring-no-properties
        (point) (progn (goto-char (point-max))
-		      (line-beginning-position -1)))))))
+		      (line-beginning-position -1))))))
+  ;; Test ":formula %".  Handle various duration formats.
+  (should
+   (equal
+    "| Headline     |   Time |     % |
+|--------------+--------+-------|
+| *Total time* | *6:00* | 100.0 |
+|--------------+--------+-------|
+| Foo          |   4:00 |  66.7 |
+| Bar          |   2:00 |  33.3 |
+"
+    (org-test-with-temp-text
+	"* Foo
+  CLOCK: [2016-12-28 Wed 11:09]--[2016-12-28 Wed 15:09] =>  4:00
+* Bar
+  CLOCK: [2016-12-28 Wed 13:09]--[2016-12-28 Wed 15:09] =>  2:00
+
+* Report
+<point>#+BEGIN: clocktable :maxlevel 1 :formula %
+#+END:
+"
+      (org-update-dblock)
+      (buffer-substring-no-properties (line-beginning-position 3)
+				      (line-beginning-position 9)))))
+  (should
+   (equal
+    "| Headline     | Time      |     % |
+|--------------+-----------+-------|
+| *Total time* | *1d 4:00* | 100.0 |
+|--------------+-----------+-------|
+| Foo          | 1d 2:00   |  83.3 |
+| Bar          | 2:00      |  16.7 |
+"
+    (org-test-with-temp-text
+	"
+* Foo
+  CLOCK: [2016-12-27 Wed 13:09]--[2016-12-28 Wed 15:09] => 26:00
+* Bar
+  CLOCK: [2016-12-28 Wed 13:09]--[2016-12-28 Wed 15:09] =>  2:00
+* Report
+<point>#+BEGIN: clocktable :maxlevel 1 :formula %
+#+END:"
+      (org-update-dblock)
+      (buffer-substring-no-properties (line-beginning-position 3)
+				      (line-beginning-position 9))))))
 
 (provide 'test-org-clock)
 ;;; test-org-clock.el end here