emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] make org-clock-time% respect org-effort-durations and related
@ 2015-10-25 12:48 Jan Malakhovski
  2015-10-25 12:48 ` [PATCH 1/4] org-clock: make org-clock-time% respect org-effort-durations Jan Malakhovski
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Jan Malakhovski @ 2015-10-25 12:48 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

Hi.

The first patch here is a must, because it fixes a bug I stumble upoon
daily. Middle two are just to make the names consistent. While doing
all those changes I read quite a lot of code and the last patch adds a
FIXME for a particularly ugly place I'm not sure how to fix (I think
don't ever consciously use that code path and I'm not sure what it
supposed to do).

Cheers,
  Jan

Jan Malakhovski (4):
  org-clock: make org-clock-time% respect org-effort-durations
  org: move org-duration-string-to-minutes to a better place
  rename org-duration-string-to-minutes to org-clocksum-string-to-minutes everywhere
  org-colview: add a FIXME

 contrib/lisp/org-depend.el     |  2 +-
 contrib/lisp/ox-taskjuggler.el |  2 +-
 lisp/org-agenda.el             |  2 +-
 lisp/org-clock.el              | 39 +++++++++++++------------------
 lisp/org-colview.el            |  5 +++-
 lisp/org.el                    | 52 +++++++++++++++++++++++-------------------
 6 files changed, 51 insertions(+), 51 deletions(-)

-- 
2.5.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] org-clock: make org-clock-time% respect org-effort-durations
  2015-10-25 12:48 [PATCH] make org-clock-time% respect org-effort-durations and related Jan Malakhovski
@ 2015-10-25 12:48 ` Jan Malakhovski
  2015-10-25 12:48 ` [PATCH 2/4] org: move org-duration-string-to-minutes to a better place Jan Malakhovski
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jan Malakhovski @ 2015-10-25 12:48 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

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 5d7c6b4..6cbd132 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))
-	  (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))))
+	nil))))
 
 ;; Saving and loading the clock
 
-- 
2.5.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] org: move org-duration-string-to-minutes to a better place
  2015-10-25 12:48 [PATCH] make org-clock-time% respect org-effort-durations and related Jan Malakhovski
  2015-10-25 12:48 ` [PATCH 1/4] org-clock: make org-clock-time% respect org-effort-durations Jan Malakhovski
@ 2015-10-25 12:48 ` Jan Malakhovski
  2015-10-25 12:48 ` [PATCH 3/4] rename org-duration-string-to-minutes to org-clocksum-string-to-minutes everywhere Jan Malakhovski
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jan Malakhovski @ 2015-10-25 12:48 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

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

diff --git a/lisp/org.el b/lisp/org.el
index 088913c..453ed37 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -18329,6 +18329,26 @@ If no number is found, the return value is 0."
     (string-to-number (match-string 1 s)))
    (t 0)))
 
+(defun org-duration-string-to-minutes (s &optional output-to-string)
+  "Convert a duration string S to minutes.
+
+A bare number is interpreted as minutes, modifiers can be set by
+customizing `org-effort-durations' (which see).
+
+Entries containing a colon are interpreted as H:MM by
+`org-hh:mm-string-to-minutes'."
+  (let ((result 0)
+	(re (concat "\\([0-9.]+\\) *\\("
+		    (regexp-opt (mapcar 'car org-effort-durations))
+		    "\\)")))
+    (while (string-match re s)
+      (incf result (* (cdr (assoc (match-string 2 s) org-effort-durations))
+		      (string-to-number (match-string 1 s))))
+      (setq s (replace-match "" nil t s)))
+    (setq result (floor result))
+    (incf result (org-hh:mm-string-to-minutes s))
+    (if output-to-string (number-to-string result) result)))
+
 (defcustom org-image-actual-width t
   "Should we use the actual width of images when inlining them?
 
@@ -18387,26 +18407,6 @@ The value is a list, with zero or more of the symbols `effort', `appt',
   :package-version '(Org . "8.3")
   :group 'org-agenda)
 
-(defun org-duration-string-to-minutes (s &optional output-to-string)
-  "Convert a duration string S to minutes.
-
-A bare number is interpreted as minutes, modifiers can be set by
-customizing `org-effort-durations' (which see).
-
-Entries containing a colon are interpreted as H:MM by
-`org-hh:mm-string-to-minutes'."
-  (let ((result 0)
-	(re (concat "\\([0-9.]+\\) *\\("
-		    (regexp-opt (mapcar 'car org-effort-durations))
-		    "\\)")))
-    (while (string-match re s)
-      (incf result (* (cdr (assoc (match-string 2 s) org-effort-durations))
-		      (string-to-number (match-string 1 s))))
-      (setq s (replace-match "" nil t s)))
-    (setq result (floor result))
-    (incf result (org-hh:mm-string-to-minutes s))
-    (if output-to-string (number-to-string result) result)))
-
 ;;;; Files
 
 (defun org-save-all-org-buffers ()
-- 
2.5.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] rename org-duration-string-to-minutes to org-clocksum-string-to-minutes everywhere
  2015-10-25 12:48 [PATCH] make org-clock-time% respect org-effort-durations and related Jan Malakhovski
  2015-10-25 12:48 ` [PATCH 1/4] org-clock: make org-clock-time% respect org-effort-durations Jan Malakhovski
  2015-10-25 12:48 ` [PATCH 2/4] org: move org-duration-string-to-minutes to a better place Jan Malakhovski
@ 2015-10-25 12:48 ` Jan Malakhovski
  2015-10-25 12:48 ` [PATCH 4/4] org-colview: add a FIXME Jan Malakhovski
  2015-10-25 17:23 ` [PATCH] make org-clock-time% respect org-effort-durations and related Kyle Meyer
  4 siblings, 0 replies; 6+ messages in thread
From: Jan Malakhovski @ 2015-10-25 12:48 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

---
 contrib/lisp/org-depend.el     |  2 +-
 contrib/lisp/ox-taskjuggler.el |  2 +-
 lisp/org-agenda.el             |  2 +-
 lisp/org-clock.el              | 14 +++++++-------
 lisp/org-colview.el            |  2 +-
 lisp/org.el                    | 14 +++++++++-----
 6 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/contrib/lisp/org-depend.el b/contrib/lisp/org-depend.el
index 1cd4130..7b263bc 100644
--- a/contrib/lisp/org-depend.el
+++ b/contrib/lisp/org-depend.el
@@ -270,7 +270,7 @@ This does two different kinds of triggers:
 			    (effort (when (or effort-up effort-down)
 				      (let ((effort (get-text-property (point) 'org-effort)))
 					(when effort
-					  (org-duration-string-to-minutes effort))))))
+					  (org-clocksum-string-to-minutes effort))))))
 			(push (list (point) todo-kwd priority tags effort)
 			      items))
 		      (unless (org-goto-sibling)
diff --git a/contrib/lisp/ox-taskjuggler.el b/contrib/lisp/ox-taskjuggler.el
index 2bd47e6..b425b1b 100644
--- a/contrib/lisp/ox-taskjuggler.el
+++ b/contrib/lisp/ox-taskjuggler.el
@@ -861,7 +861,7 @@ a unique id will be associated to it."
      (and complete (format "  complete %s\n" complete))
      (and effort
           (format "  effort %s\n"
-                  (let* ((minutes (org-duration-string-to-minutes effort))
+                  (let* ((minutes (org-clocksum-string-to-minutes effort))
                          (hours (/ minutes 60.0)))
                     (format "%.1fh" hours))))
      (and priority (format "  priority %s\n" priority))
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index e7a3776..bdb69c5 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7655,7 +7655,7 @@ E looks like \"+<2:25\"."
 		   ((equal op ??) op)
 		   (t '=)))
     (list 'org-agenda-compare-effort (list 'quote op)
-	  (org-duration-string-to-minutes e))))
+	  (org-clocksum-string-to-minutes e))))
 
 (defun org-agenda-compare-effort (op value)
   "Compare the effort of the current line with VALUE, using OP.
diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 6cbd132..2f254e1 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -668,7 +668,7 @@ If not, show simply the clocked time like 01:50."
   (let ((clocked-time (org-clock-get-clocked-time)))
     (if org-clock-effort
 	(let* ((effort-in-minutes
-		(org-duration-string-to-minutes org-clock-effort))
+		(org-clocksum-string-to-minutes org-clock-effort))
 	       (work-done-str
 		(org-propertize
 		 (org-minutes-to-clocksum-string clocked-time)
@@ -749,10 +749,10 @@ clocked item, and the value displayed in the mode line."
 	  ;; A string.  See if it is a delta
 	  (setq sign (string-to-char value))
 	  (if (member sign '(?- ?+))
-	      (setq current (org-duration-string-to-minutes current)
+	      (setq current (org-clocksum-string-to-minutes current)
 		    value (substring value 1))
 	    (setq current 0))
-	  (setq value (org-duration-string-to-minutes value))
+	  (setq value (org-clocksum-string-to-minutes value))
 	  (if (equal ?- sign)
 	      (setq value (- current value))
 	    (if (equal ?+ sign) (setq value (+ current value)))))
@@ -770,7 +770,7 @@ clocked item, and the value displayed in the mode line."
   "Show notification if we spent more time than we estimated before.
 Notification is shown only once."
   (when (org-clocking-p)
-    (let ((effort-in-minutes (org-duration-string-to-minutes org-clock-effort))
+    (let ((effort-in-minutes (org-clocksum-string-to-minutes org-clock-effort))
 	  (clocked-time (org-clock-get-clocked-time)))
       (if (setq org-clock-task-overrun
 		(if (or (null effort-in-minutes) (zerop effort-in-minutes))
@@ -1193,7 +1193,7 @@ make this the default behavior.)"
   (setq org-clock-notification-was-shown nil)
   (org-refresh-properties
    org-effort-property '((effort . identity)
-			 (effort-minutes . org-duration-string-to-minutes)))
+			 (effort-minutes . org-clocksum-string-to-minutes)))
   (catch 'abort
     (let ((interrupting (and (not org-clock-resolving-clocks-due-to-idleness)
 			     (org-clocking-p)))
@@ -2869,13 +2869,13 @@ TIME:      The sum of all time spend in this tree, in minutes.  This time
   "Compute a time fraction in percent.
 TOTAL s a total time string.
 STRINGS is a list of strings that should be checked for a time.
-Strings are parsed using `org-duration-string-to-minutes`.
+Strings are parsed using `org-clocksum-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
-	(setq tot (org-duration-string-to-minutes total))
+	(setq tot (org-clocksum-string-to-minutes total))
 	(if (= tot 0.) (throw 'exit 0.))
 	(while (setq s (pop strings))
 	  (setq cur (org-clocksum-string-to-minutes s))
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 14c8f49..1113adc 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1116,7 +1116,7 @@ display, or in the #+COLUMNS line of the current buffer."
        ((string-match (concat "\\([0-9.]+\\) *\\("
 			      (regexp-opt (mapcar 'car org-effort-durations))
 			      "\\)") s)
-	(setq s (concat "0:" (org-duration-string-to-minutes s t)))
+	(setq s (concat "0:" (org-clocksum-string-to-minutes s t)))
         (let ((l (nreverse (org-split-string s ":"))) (sum 0.0))
           (while l
             (setq sum (+ (string-to-number (pop l)) (/ sum 60))))
diff --git a/lisp/org.el b/lisp/org.el
index 453ed37..4e4f001 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -3699,7 +3699,7 @@ and the clock summary:
 
  ((\"Remaining\" (lambda(value)
                    (let ((clocksum (org-clock-sum-current-item))
-                         (effort (org-duration-string-to-minutes
+                         (effort (org-clocksum-string-to-minutes
                                    (org-entry-get (point) \"Effort\"))))
                      (org-minutes-to-clocksum-string (- effort clocksum))))))"
   :group 'org-properties
@@ -9612,7 +9612,7 @@ The refresh happens only for the current tree (not subtree)."
   (org-refresh-properties
    org-effort-property
    '((effort . identity)
-     (effort-minutes . org-duration-string-to-minutes))))
+     (effort-minutes . org-clocksum-string-to-minutes))))
 
 ;;;; Link Stuff
 
@@ -15683,7 +15683,7 @@ When INCREMENT is non-nil, set the property to the next allowed value."
       (org-entry-put nil prop val))
     (org-refresh-property
      '((effort . identity)
-       (effort-minutes . org-duration-string-to-minutes))
+       (effort-minutes . org-clocksum-string-to-minutes))
      val)
     (when (string= heading org-clock-current-task)
       (setq org-clock-effort (get-text-property (point-at-bol) 'effort))
@@ -16544,7 +16544,7 @@ completion."
     (when (equal prop org-effort-property)
       (org-refresh-property
        '((effort . identity)
-	 (effort-minutes . org-duration-string-to-minutes))
+	 (effort-minutes . org-clocksum-string-to-minutes))
        nval)
       (when (string= org-clock-current-task heading)
 	(setq org-clock-effort nval)
@@ -18329,7 +18329,7 @@ If no number is found, the return value is 0."
     (string-to-number (match-string 1 s)))
    (t 0)))
 
-(defun org-duration-string-to-minutes (s &optional output-to-string)
+(defun org-clocksum-string-to-minutes (s &optional output-to-string)
   "Convert a duration string S to minutes.
 
 A bare number is interpreted as minutes, modifiers can be set by
@@ -18349,6 +18349,10 @@ Entries containing a colon are interpreted as H:MM by
     (incf result (org-hh:mm-string-to-minutes s))
     (if output-to-string (number-to-string result) result)))
 
+(defalias 'org-duration-string-to-minutes 'org-clocksum-string-to-minutes)
+(make-obsolete 'org-duration-string-to-minutes 'org-clocksum-string-to-minutes
+	       "Org mode version 8.3")
+
 (defcustom org-image-actual-width t
   "Should we use the actual width of images when inlining them?
 
-- 
2.5.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] org-colview: add a FIXME
  2015-10-25 12:48 [PATCH] make org-clock-time% respect org-effort-durations and related Jan Malakhovski
                   ` (2 preceding siblings ...)
  2015-10-25 12:48 ` [PATCH 3/4] rename org-duration-string-to-minutes to org-clocksum-string-to-minutes everywhere Jan Malakhovski
@ 2015-10-25 12:48 ` Jan Malakhovski
  2015-10-25 17:23 ` [PATCH] make org-clock-time% respect org-effort-durations and related Kyle Meyer
  4 siblings, 0 replies; 6+ messages in thread
From: Jan Malakhovski @ 2015-10-25 12:48 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

---
 lisp/org-colview.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 1113adc..4f09766 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1113,6 +1113,9 @@ display, or in the #+COLUMNS line of the current buffer."
        ((memq fmt '(checkbox checkbox-n-of-m checkbox-percent))
         (if (equal s "[X]") 1. 0.000001))
        ((memq fmt '(estimate)) (org-string-to-estimate s))
+       ;; FIXME: does this duplicate org-clocksum-string-to-minutes in a most peculiar way?
+       ;; I can guess that long time ago this used org-hh:mm-string-to-minutes
+       ;; and all this ugliness derives from that
        ((string-match (concat "\\([0-9.]+\\) *\\("
 			      (regexp-opt (mapcar 'car org-effort-durations))
 			      "\\)") s)
-- 
2.5.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] make org-clock-time% respect org-effort-durations and related
  2015-10-25 12:48 [PATCH] make org-clock-time% respect org-effort-durations and related Jan Malakhovski
                   ` (3 preceding siblings ...)
  2015-10-25 12:48 ` [PATCH 4/4] org-colview: add a FIXME Jan Malakhovski
@ 2015-10-25 17:23 ` Kyle Meyer
  4 siblings, 0 replies; 6+ messages in thread
From: Kyle Meyer @ 2015-10-25 17:23 UTC (permalink / raw)
  To: Jan Malakhovski; +Cc: emacs-orgmode

Hello,

Jan Malakhovski <oxij@oxij.org> writes:

> Hi.
>
> The first patch here is a must, because it fixes a bug I stumble upoon
> daily. Middle two are just to make the names consistent. While doing
> all those changes I read quite a lot of code and the last patch adds a
> FIXME for a particularly ugly place I'm not sure how to fix (I think
> don't ever consciously use that code path and I'm not sure what it
> supposed to do).

Thanks for these patches, as well as the other series you sent [1].  I
haven't looked at the content in detail, but based on the diffstats,
these changes are touching enough lines to require copyright assignment
[2].  Also, please include a ChangeLog entry in the commit message [3].

Thanks again.

[1] http://thread.gmane.org/gmane.emacs.orgmode/102168
[2] http://orgmode.org/worg/org-contribute.html#orgheadline2
[3] http://orgmode.org/worg/org-contribute.html#orgheadline6

--
Kyle

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-10-25 17:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-25 12:48 [PATCH] make org-clock-time% respect org-effort-durations and related Jan Malakhovski
2015-10-25 12:48 ` [PATCH 1/4] org-clock: make org-clock-time% respect org-effort-durations Jan Malakhovski
2015-10-25 12:48 ` [PATCH 2/4] org: move org-duration-string-to-minutes to a better place Jan Malakhovski
2015-10-25 12:48 ` [PATCH 3/4] rename org-duration-string-to-minutes to org-clocksum-string-to-minutes everywhere Jan Malakhovski
2015-10-25 12:48 ` [PATCH 4/4] org-colview: add a FIXME Jan Malakhovski
2015-10-25 17:23 ` [PATCH] make org-clock-time% respect org-effort-durations and related Kyle Meyer

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).