emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH v2 0/9] mail, clock and calc changes
@ 2015-11-03 20:15 Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 1/9] org-clock: fix a typo Jan Malakhovski
                   ` (9 more replies)
  0 siblings, 10 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

Hello.

While my assignment is snail-mail delivered to and processed by FSF
I'd like to request comments on the following set of patches, which
contains all the patches I sent to this list before and some new ones.

I'm mainly worried by ChangeLog format and possibly unorthodox elisp
in ob-calc.

The first two are TINYCHANGE.

Cheers,
  Jan

Jan Malakhovski (9):
  org-clock: fix a typo
  org-colview: add a FIXME
  org-clock: fix `org-clock-time%'
  org: move `org-duration-string-to-minutes' to a better place
  rename `org-duration-string-to-minutes' to
    `org-clocksum-string-to-minutes' everywhere
  factor out date-timestamp* calculations to org-store-link-props
  org-notmuch: add date support to org-notmuch-store-link
  ob-calc: add more API, documentation and examples so that it can be
    used in tables
  ob-calc: don't leave garbage on the stack

 contrib/lisp/org-depend.el     |   2 +-
 contrib/lisp/org-mew.el        |  11 +-
 contrib/lisp/org-notmuch.el    |   7 +-
 contrib/lisp/org-vm.el         |  11 +-
 contrib/lisp/org-wl.el         |  10 +-
 contrib/lisp/ox-taskjuggler.el |   2 +-
 doc/org.texi                   |   4 +-
 lisp/ob-calc.el                | 236 ++++++++++++++++++++++++++++++++---------
 lisp/org-agenda.el             |   2 +-
 lisp/org-clock.el              |  41 +++----
 lisp/org-colview.el            |   5 +-
 lisp/org-gnus.el               |  15 +--
 lisp/org-mhe.el                |  10 +-
 lisp/org-rmail.el              |  11 +-
 lisp/org.el                    |  67 +++++++-----
 15 files changed, 263 insertions(+), 171 deletions(-)

-- 
2.6.2

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

* [PATCH 1/9] org-clock: fix a typo
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 2/9] org-colview: add a FIXME Jan Malakhovski
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

TINYCHANGE
---
 lisp/org-clock.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 09f8391..ad423f1 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -2774,7 +2774,7 @@ following structure:
   (LEVEL HEADLINE TIMESTAMP TIME)
 
 LEVEL:     The level of the headline, as an integer.  This will be
-           the reduced leve, so 1,2,3,... even if only odd levels
+           the reduced level, so 1,2,3,... even if only odd levels
            are being used.
 HEADLINE:  The text of the headline.  Depending on PARAMS, this may
            already be formatted like a link.
-- 
2.6.2

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

* [PATCH 2/9] org-colview: add a FIXME
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 1/9] org-clock: fix a typo Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 3/9] org-clock: fix `org-clock-time%' Jan Malakhovski
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

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

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index b698801..d27abc3 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.6.2

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

* [PATCH 3/9] org-clock: fix `org-clock-time%'
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 1/9] org-clock: fix a typo Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 2/9] org-colview: add a FIXME Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-04 11:18   ` Aaron Ecay
  2015-11-03 20:15 ` [PATCH 4/9] org: move `org-duration-string-to-minutes' to a better place Jan Malakhovski
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

* 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))
-	  (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.6.2

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

* [PATCH 4/9] org: move `org-duration-string-to-minutes' to a better place
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
                   ` (2 preceding siblings ...)
  2015-11-03 20:15 ` [PATCH 3/9] org-clock: fix `org-clock-time%' Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 5/9] rename `org-duration-string-to-minutes' to `org-clocksum-string-to-minutes' everywhere Jan Malakhovski
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 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 6218a3a..a0fe644 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -18328,6 +18328,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?
 
@@ -18386,26 +18406,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.6.2

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

* [PATCH 5/9] rename `org-duration-string-to-minutes' to `org-clocksum-string-to-minutes' everywhere
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
                   ` (3 preceding siblings ...)
  2015-11-03 20:15 ` [PATCH 4/9] org: move `org-duration-string-to-minutes' to a better place Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-04 11:21   ` Aaron Ecay
  2015-11-03 20:15 ` [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props Jan Malakhovski
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

* lisp/org-agenda.el:
* lisp/org-clock.el:
* lisp/org-colview.el:
* lisp/org.el:
* contrib/lisp/org-depend.el:
* contrib/lisp/ox-taskjuggler.el: Rename (org-duration-string-to-minutes)
  to (org-clocksum-string-to-minutes).
---
 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 6313f52..ab4595b 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7665,7 +7665,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 4563a8a..ab65d3b 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 d27abc3..bbbeea9 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1119,7 +1119,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 a0fe644..c466870 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
 
@@ -15673,7 +15673,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))
@@ -16534,7 +16534,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)
@@ -18328,7 +18328,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
@@ -18348,6 +18348,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.6.2

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

* [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
                   ` (4 preceding siblings ...)
  2015-11-03 20:15 ` [PATCH 5/9] rename `org-duration-string-to-minutes' to `org-clocksum-string-to-minutes' everywhere Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-04 11:26   ` Aaron Ecay
  2015-11-03 20:15 ` [PATCH 7/9] org-notmuch: add date support to org-notmuch-store-link Jan Malakhovski
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

* lisp/org.el (org-store-link-props): Rewrite function to get date-timestamp*
  calculations.
* lisp/org-gnus.el:
* lisp/org-mhe.el:
* lisp/org-rmail.el:
* contrib/lisp/org-mew.el:
* contrib/lisp/org-vm.el:
* contrib/lisp/org-wl.el: Remove date-timestamp* copy-paste.
* doc/org.texi: Fix `org-capture-templates' documentation.
---
 contrib/lisp/org-mew.el | 11 +----------
 contrib/lisp/org-vm.el  | 11 +----------
 contrib/lisp/org-wl.el  | 10 +---------
 doc/org.texi            |  4 ++--
 lisp/org-gnus.el        | 15 +--------------
 lisp/org-mhe.el         | 10 +---------
 lisp/org-rmail.el       | 11 +----------
 lisp/org.el             | 15 +++++++++++++--
 8 files changed, 21 insertions(+), 66 deletions(-)

diff --git a/contrib/lisp/org-mew.el b/contrib/lisp/org-mew.el
index eb0afc0..35fdd8b 100644
--- a/contrib/lisp/org-mew.el
+++ b/contrib/lisp/org-mew.el
@@ -167,19 +167,10 @@ with \"t\" key."
 	       (from (mew-header-get-value "From:"))
 	       (to (mew-header-get-value "To:"))
 	       (date (mew-header-get-value "Date:"))
-	       (date-ts (and date (format-time-string
-				   (org-time-stamp-format t)
-				   (date-to-time date))))
-	       (date-ts-ia (and date (format-time-string
-				      (org-time-stamp-format t t)
-				      (date-to-time date))))
 	       (subject (mew-header-get-value "Subject:"))
 	       desc link)
-	  (org-store-link-props :type "mew" :from from :to to
+	  (org-store-link-props :type "mew" :from from :to to :date date
 				:subject subject :message-id message-id)
-	  (when date
-	    (org-add-link-props :date date :date-timestamp date-ts
-				:date-timestamp-inactive date-ts-ia))
 	  (setq message-id (org-remove-angle-brackets message-id))
 	  (setq desc (org-email-link-description))
 	  (setq link (concat "mew:" folder-name "#" message-id))
diff --git a/contrib/lisp/org-vm.el b/contrib/lisp/org-vm.el
index 5d30f64..da242cb 100644
--- a/contrib/lisp/org-vm.el
+++ b/contrib/lisp/org-vm.el
@@ -77,12 +77,6 @@
              (message-id (vm-su-message-id message))
              (link-type (if (vm-imap-folder-p) "vm-imap" "vm"))
 	     (date (vm-get-header-contents message "Date"))
-	     (date-ts (and date (format-time-string
-				 (org-time-stamp-format t)
-				 (date-to-time date))))
-	     (date-ts-ia (and date (format-time-string
-				    (org-time-stamp-format t t)
-				    (date-to-time date))))
 	     folder desc link)
         (if (vm-imap-folder-p)
 	    (let ((spec (vm-imap-find-spec-for-buffer (current-buffer))))
@@ -95,10 +89,7 @@
                 (setq folder (replace-match "" t t folder)))))
         (setq message-id (org-remove-angle-brackets message-id))
 	(org-store-link-props :type link-type :from from :to to :subject subject
-			      :message-id message-id)
-	(when date
-	  (org-add-link-props :date date :date-timestamp date-ts
-			      :date-timestamp-inactive date-ts-ia))
+			      :message-id message-id :date date)
 	(setq desc (org-email-link-description))
 	(setq link (concat (concat link-type ":") folder "#" message-id))
 	(org-add-link-props :link link :description desc)
diff --git a/contrib/lisp/org-wl.el b/contrib/lisp/org-wl.el
index 632c9e3..2cc333c 100644
--- a/contrib/lisp/org-wl.el
+++ b/contrib/lisp/org-wl.el
@@ -198,12 +198,6 @@ ENTITY is a message entity."
 		 (xref (org-wl-message-field 'xref wl-message-entity))
 		 (subject (org-wl-message-field 'subject wl-message-entity))
 		 (date (org-wl-message-field 'date wl-message-entity))
-		 (date-ts (and date (format-time-string
-				     (org-time-stamp-format t)
-				     (date-to-time date))))
-		 (date-ts-ia (and date (format-time-string
-					(org-time-stamp-format t t)
-					(date-to-time date))))
 		 desc link)
 
 	    ;; remove text properties of subject string to avoid possible bug
@@ -243,9 +237,7 @@ ENTITY is a message entity."
 	      (setq desc (org-email-link-description))
 	      (setq link (concat "wl:" folder-name "#" message-id-no-brackets))
 	      (org-add-link-props :link link :description desc)))
-	    (when date
-	      (org-add-link-props :date date :date-timestamp date-ts
-				  :date-timestamp-inactive date-ts-ia))
+	    (org-add-link-props :date date)
 	    (or link xref)))))))
 
 (defun org-wl-open-nntp (path)
diff --git a/doc/org.texi b/doc/org.texi
index ba402bf..2bf2b24 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -7314,8 +7314,8 @@ Link type                        |  Available keywords
 ---------------------------------+----------------------------------------------
 bbdb                             |  %:name %:company
 irc                              |  %:server %:port %:nick
-vm, vm-imap, wl, mh, mew, rmail  |  %:type %:subject %:message-id
-                                 |  %:from %:fromname %:fromaddress
+vm, vm-imap, wl, mh, mew, rmail, |  %:type %:subject %:message-id
+gnus                             |  %:from %:fromname %:fromaddress
                                  |  %:to   %:toname   %:toaddress
                                  |  %:date @r{(message date header field)}
                                  |  %:date-timestamp @r{(date as active timestamp)}
diff --git a/lisp/org-gnus.el b/lisp/org-gnus.el
index c7b46af..c1eb6c6 100644
--- a/lisp/org-gnus.el
+++ b/lisp/org-gnus.el
@@ -159,16 +159,6 @@ If `org-store-link' was called with a prefix arg the meaning of
 	   (from (mail-header-from header))
 	   (message-id (org-remove-angle-brackets (mail-header-id header)))
 	   (date (org-trim (mail-header-date header)))
-	   (date-ts (and date
-			 (ignore-errors
-			   (format-time-string
-			    (org-time-stamp-format t)
-			    (date-to-time date)))))
-	   (date-ts-ia (and date
-			    (ignore-errors
-			      (format-time-string
-			       (org-time-stamp-format t t)
-			       (date-to-time date)))))
 	   (subject (copy-sequence (mail-header-subject header)))
 	   (to (cdr (assq 'To (mail-header-extra header))))
 	   newsgroups x-no-archive desc link)
@@ -188,11 +178,8 @@ If `org-store-link' was called with a prefix arg the meaning of
 	(setq to (or to (gnus-fetch-original-field "To"))
 	      newsgroups (gnus-fetch-original-field "Newsgroups")
 	      x-no-archive (gnus-fetch-original-field "x-no-archive")))
-      (org-store-link-props :type "gnus" :from from :subject subject
+      (org-store-link-props :type "gnus" :from from :date date :subject subject
 			    :message-id message-id :group group :to to)
-      (when date
-	(org-add-link-props :date date :date-timestamp date-ts
-			    :date-timestamp-inactive date-ts-ia))
       (setq desc (org-email-link-description)
 	    link (org-gnus-article-link
 		  group	newsgroups message-id x-no-archive))
diff --git a/lisp/org-mhe.el b/lisp/org-mhe.el
index e184440..3fcbc46 100644
--- a/lisp/org-mhe.el
+++ b/lisp/org-mhe.el
@@ -88,17 +88,9 @@ supported by MH-E."
 	     (message-id (org-mhe-get-header "Message-Id:"))
 	     (subject (org-mhe-get-header "Subject:"))
 	     (date (org-mhe-get-header "Date:"))
-	     (date-ts (and date (format-time-string
-				 (org-time-stamp-format t) (date-to-time date))))
-	     (date-ts-ia (and date (format-time-string
-				    (org-time-stamp-format t t)
-				    (date-to-time date))))
 	     link desc)
-	(org-store-link-props :type "mh" :from from :to to
+	(org-store-link-props :type "mh" :from from :to to :date date
 			      :subject subject :message-id message-id)
-	(when date
-	  (org-add-link-props :date date :date-timestamp date-ts
-			      :date-timestamp-inactive date-ts-ia))
 	(setq desc (org-email-link-description))
 	(setq link (concat "mhe:" (org-mhe-get-message-real-folder) "#"
 			   (org-remove-angle-brackets message-id)))
diff --git a/lisp/org-rmail.el b/lisp/org-rmail.el
index af47e0f..4732f51 100644
--- a/lisp/org-rmail.el
+++ b/lisp/org-rmail.el
@@ -65,19 +65,10 @@
 	       (to (mail-fetch-field "to"))
 	       (subject (mail-fetch-field "subject"))
 	       (date (mail-fetch-field "date"))
-	       (date-ts (and date (format-time-string
-				   (org-time-stamp-format t)
-				   (date-to-time date))))
-	       (date-ts-ia (and date (format-time-string
-				      (org-time-stamp-format t t)
-				      (date-to-time date))))
 	       desc link)
 	  (org-store-link-props
-	   :type "rmail" :from from :to to
+	   :type "rmail" :from from :to to :date date
 	   :subject subject :message-id message-id)
-	  (when date
-	    (org-add-link-props :date date :date-timestamp date-ts
-				:date-timestamp-inactive date-ts-ia))
 	  (setq message-id (org-remove-angle-brackets message-id))
 	  (setq desc (org-email-link-description))
 	  (setq link (concat "rmail:" folder "#" message-id))
diff --git a/lisp/org.el b/lisp/org.el
index c466870..cf0ef1f 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -9960,7 +9960,7 @@ active region."
 	 (car org-stored-links))))))
 
 (defun org-store-link-props (&rest plist)
-  "Store link properties, extract names and addresses."
+  "Store link properties, extract names, addresses and dates."
   (let (x adr)
     (when (setq x (plist-get plist :from))
       (setq adr (mail-extract-address-components x))
@@ -9969,7 +9969,18 @@ active region."
     (when (setq x (plist-get plist :to))
       (setq adr (mail-extract-address-components x))
       (setq plist (plist-put plist :toname (car adr)))
-      (setq plist (plist-put plist :toaddress (nth 1 adr)))))
+      (setq plist (plist-put plist :toaddress (nth 1 adr))))
+    (when (setq x (plist-get plist :date))
+      (setq plist (plist-put plist :date-timestamp
+			(ignore-errors
+			  (format-time-string
+			    (org-time-stamp-format t)
+			    (date-to-time x)))))
+      (setq plist (plist-put plist :date-timestamp-inactive
+			(ignore-errors
+			  (format-time-string
+			    (org-time-stamp-format t t)
+			    (date-to-time x)))))))
   (let ((from (plist-get plist :from))
 	(to (plist-get plist :to)))
     (when (and from to org-from-is-user-regexp)
-- 
2.6.2

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

* [PATCH 7/9] org-notmuch: add date support to org-notmuch-store-link
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
                   ` (5 preceding siblings ...)
  2015-11-03 20:15 ` [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 8/9] ob-calc: add more API, documentation and examples so that it can be used in tables Jan Malakhovski
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

* contrib/lisp/org-notmuch.el (org-notmuch-store-link): Add date support.
* doc/org.texi: Fix `org-capture-templates' documentation.
---
 contrib/lisp/org-notmuch.el | 7 ++++---
 doc/org.texi                | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/contrib/lisp/org-notmuch.el b/contrib/lisp/org-notmuch.el
index 712ec5a..265742e 100644
--- a/contrib/lisp/org-notmuch.el
+++ b/contrib/lisp/org-notmuch.el
@@ -71,15 +71,16 @@ Should accept a notmuch search string as the sole argument."
 (defun org-notmuch-store-link ()
   "Store a link to a notmuch search or message."
   (when (eq major-mode 'notmuch-show-mode)
-    (let* ((message-id (notmuch-show-get-prop :id))
+    (let* ((message-id (notmuch-show-get-message-id t))
 	   (subject (notmuch-show-get-subject))
 	   (to (notmuch-show-get-to))
 	   (from (notmuch-show-get-from))
+	   (date (org-trim (notmuch-show-get-date)))
 	   desc link)
-      (org-store-link-props :type "notmuch" :from from :to to
+      (org-store-link-props :type "notmuch" :from from :to to :date date
        			    :subject subject :message-id message-id)
       (setq desc (org-email-link-description))
-      (setq link (concat "notmuch:"  "id:" message-id))
+      (setq link (concat "notmuch:id:" message-id))
       (org-add-link-props :link link :description desc)
       link)))
 
diff --git a/doc/org.texi b/doc/org.texi
index 2bf2b24..106bdac 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -7315,7 +7315,7 @@ Link type                        |  Available keywords
 bbdb                             |  %:name %:company
 irc                              |  %:server %:port %:nick
 vm, vm-imap, wl, mh, mew, rmail, |  %:type %:subject %:message-id
-gnus                             |  %:from %:fromname %:fromaddress
+gnus, notmuch                    |  %:from %:fromname %:fromaddress
                                  |  %:to   %:toname   %:toaddress
                                  |  %:date @r{(message date header field)}
                                  |  %:date-timestamp @r{(date as active timestamp)}
-- 
2.6.2

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

* [PATCH 8/9] ob-calc: add more API, documentation and examples so that it can be used in tables
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
                   ` (6 preceding siblings ...)
  2015-11-03 20:15 ` [PATCH 7/9] org-notmuch: add date support to org-notmuch-store-link Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-03 20:15 ` [PATCH 9/9] ob-calc: don't leave garbage on the stack Jan Malakhovski
  2015-11-04 11:36 ` [PATCH v2 0/9] mail, clock and calc changes Aaron Ecay
  9 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

* lisp/ob-calc.el (org-babel-calc-eval):
(org-babel-calc-set-env):
(org-babel-calc-reset-env):
(org-babel-calc-store-env):
(org-babel-calc-eval-string):
(org-babel-calc-eval-line): New funcion.
(org-babel-execute:calc): Rewrite to use new functions.

This also makes ob-calc useful for computing complicated stuff in org-tables. See
`org-babel-calc-eval` docstring for more info.
---
 lisp/ob-calc.el | 232 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 183 insertions(+), 49 deletions(-)

diff --git a/lisp/ob-calc.el b/lisp/ob-calc.el
index a8c50da..e8b43e7 100644
--- a/lisp/ob-calc.el
+++ b/lisp/ob-calc.el
@@ -1,4 +1,4 @@
-;;; ob-calc.el --- Babel Functions for Calc          -*- lexical-binding: t; -*-
+;;; ob-calc.el --- Babel Functions for Calc
 
 ;; Copyright (C) 2010-2015 Free Software Foundation, Inc.
 
@@ -23,7 +23,8 @@
 
 ;;; Commentary:
 
-;; Org-Babel support for evaluating calc code
+;; Org-Babel and Org-Table support for evaluating calc code.
+;; See `org-babel-calc-eval' for documentation.
 
 ;;; Code:
 (require 'ob)
@@ -42,67 +43,200 @@
 (defun org-babel-expand-body:calc (body _params)
   "Expand BODY according to PARAMS, return the expanded body." body)
 
-(defvar org--var-syms) ; Dynamically scoped from org-babel-execute:calc
-
 (defun org-babel-execute:calc (body params)
   "Execute a block of calc code with Babel."
+  (org-babel-calc-eval (org-babel-expand-body:calc body params)
+		       (org-babel--get-vars params)))
+
+(defvar org--ob-calc-env-symbol nil) ; For org-babel-calc-eval
+(defvar org--ob-calc-var-names nil)
+
+(defun org-babel-calc-eval (text &optional environment env-symbol setup env-setup)
+  "Evaluate TEXT as set of calc expressions (one per line) and return the top of the stack.
+
+Optional argument ENVIRONMENT is a user-defined variables
+environment which is an alist of (SYMBOL . VALUE).
+
+Optional argument ENV-SYMBOL is a symbol of a user-defined
+variables environment which is an alist of (SYMBOL . VALUE).
+
+Setting your environment using either of ENVIRONMENT or
+ENV-SYMBOL has the same effect. The difference is that this
+function caches the value of ENV-SYMBOL internally between
+succesive evaluations with ENV-SYMBOL arguments of equal symbol
+names and reevaluates the value of ENV-SYMBOL only when the
+symbol name of ENV-SYMBOL changes.
+
+Additionally, setting ENV-SYMBOL to nil will forget any
+internal environment before applying ENVIRONMENT, i.e. with
+ENV-SYMBOL set to nil this function is pure.
+
+You can also use `org-babel-calc-set-env',
+`org-babel-calc-reset-env' and `org-babel-calc-store-env' to set,
+reset and update the internal environment between evaluations.
+
+Optional argument SETUP allows additional calc setup on every
+evaluation.
+
+Optional argument ENV-SETUP allows additional calc setup on every
+ENV-SYMBOL change.
+
+This function is useful if you want to evaluate complicated
+formulas in a table, e.g. after evaluating
+
+  (setq an-env '((foo . \"2 day\")
+                 (bar . \"6 hr\")))
+
+you can use this in the following table
+
+  | Expr      | Result       |
+  |-----------+--------------|
+  | foo + bar | 2 day + 6 hr |
+  | foo - bar | 2 day - 6 hr |
+  |-----------+--------------|
+  #+TBLFM: $2='(org-babel-calc-eval $1 an-env)
+
+which would become slow to recompute with a lot of rows, but then
+you can change the TBLFM line to
+
+  #+TBLFM: $2='(org-babel-calc-eval $1 nil 'an-env)
+
+and it would become fast again.
+
+SETUP argument can be used like this:
+
+  | Expr      | Result   |
+  |-----------+----------|
+  | foo + bar | 2.25 day |
+  | foo - bar | 1.75 day |
+  |-----------+----------|
+  #+TBLFM: $2='(org-babel-calc-eval $1 nil 'an-env nil (lambda () (calc-units-simplify-mode t)))
+
+In case that is not fast or complicated enough, you can combine
+this with `org-babel-calc-store-env' to produce some clever stuff
+like, e.g. computing environment on the fly (an-env variable is
+not actually used here, it is being generated just in case you
+want to use it elsewhere):
+
+  (setq an-env nil)
+  (defun compute-and-remember (name expr)
+    (let* ((v (org-babel-calc-eval expr nil 'an-env nil (lambda () (calc-units-simplify-mode t))))
+           (c `(,(intern name) . ,v)))
+        (org-babel-calc-store-env (list c))
+        (push c an-env)
+        v))
+
+and then
+
+  | Name | Expr       | Value    |
+  |------+------------+----------|
+  | foo  | 2 day      | 2 day    |
+  | bar  | foo + 6 hr | 2.25 day |
+  |------+------------+----------|
+  #+TBLFM: $3='(compute-and-remember $1 $2)
+
+Note that you can set ENV-SYMBOL to 'nil to get ENV-SETUP
+without.
+
+The subsequent results might become somewhat surprising in case
+ENVIRONMENT overrides variables set with ENV-SYMBOL."
+  (org-babel-calc-init)
+  (cond
+    ((equal env-symbol nil) (org-babel-calc-reset-env))
+    ((not (equal (symbol-name env-symbol) org--ob-calc-env-symbol))
+	(org-babel-calc-set-env env-symbol)
+	(unless (null env-setup)
+	  (funcall env-setup))))
+  (org-babel-calc-store-env environment)
+  (unless (null setup)
+    (funcall setup))
+  (org-babel-calc-eval-string text))
+
+(defun org-babel-calc-init ()
+  "Initialize calc.
+
+You probably don't want to call this function explicitly."
   (unless (get-buffer "*Calculator*")
-    (save-window-excursion (calc) (calc-quit)))
-  (let* ((vars (org-babel--get-vars params))
-	 (org--var-syms (mapcar #'car vars))
-	 (var-names (mapcar #'symbol-name org--var-syms)))
-    (mapc
-     (lambda (pair)
-       (calc-push-list (list (cdr pair)))
-       (calc-store-into (car pair)))
-     vars)
-    (mapc
-     (lambda (line)
-       (when (> (length line) 0)
-	 (cond
-	  ;; simple variable name
-	  ((member line var-names) (calc-recall (intern line)))
-	  ;; stack operation
-	  ((string= "'" (substring line 0 1))
-	   (funcall (lookup-key calc-mode-map (substring line 1)) nil))
-	  ;; complex expression
-	  (t
-	   (calc-push-list
-	    (list (let ((res (calc-eval line)))
-                    (cond
-                     ((numberp res) res)
-                     ((math-read-number res) (math-read-number res))
-                     ((listp res) (error "Calc error \"%s\" on input \"%s\""
-                                         (cadr res) line))
-                     (t (replace-regexp-in-string
-                         "'" ""
-                         (calc-eval
-                          (math-evaluate-expr
-                           ;; resolve user variables, calc built in
-                           ;; variables are handled automatically
-                           ;; upstream by calc
-                           (mapcar #'org-babel-calc-maybe-resolve-var
-                                   ;; parse line into calc objects
-                                   (car (math-read-exprs line)))))))))
-                  ))))))
-     (mapcar #'org-babel-trim
-	     (split-string (org-babel-expand-body:calc body params) "[\n\r]"))))
+    (save-window-excursion (calc) (calc-quit))))
+
+(defun org-babel-calc-set-env (env-symbol)
+  "Force update current environment with the value of ENV-SYMBOL.
+
+See `org-babel-calc-eval' for more info."
+  (org-babel-calc-reset-env)
+  (org-babel-calc-store-env (eval env-symbol))
+  (setq org--ob-calc-env-symbol (symbol-name env-symbol)))
+
+(defun org-babel-calc-reset-env ()
+  "Forget current environment and the value of the last
+ENV-SYMBOL.
+
+See `org-babel-calc-eval' for more info."
+  (setq org--ob-calc-var-names nil
+	org--ob-calc-env-symbol nil))
+
+(defun org-babel-calc-store-env (vars)
+  "Store an environment (alist of (SYMBOL . VALUE) pairs) into calc.
+
+See `org-babel-calc-eval' for more info."
+  (mapc
+    (lambda (pair)
+	(let ((name (symbol-name (car pair)))
+	    (value (cdr pair)))
+	;; Using symbol-name and then intern here may seem a little
+	;; crazy, but without it calc may not recall some of variables
+	;; that got non-canonical symbols, which will be very surprising
+	;; for users that produce their environments with '(...) syntax.
+	;; Better safe than sorry.
+	  (calc-store-value (intern name) value "" 0)
+	  (push name org--ob-calc-var-names)))
+    vars))
+
+(defun org-babel-calc-eval-string (text)
+  (mapc #'org-babel-calc-eval-line (split-string text "[\n\r]"))
   (save-excursion
     (with-current-buffer (get-buffer "*Calculator*")
       (calc-eval (calc-top 1)))))
 
+(defun org-babel-calc-eval-line (line)
+  (let ((line (org-babel-trim line)))
+    (when (> (length line) 0)
+	(cond
+	 ;; simple variable name
+	 ((member line org--ob-calc-var-names) (calc-recall (intern line)))
+	 ;; stack operation
+	 ((string= "'" (substring line 0 1))
+	(funcall (lookup-key calc-mode-map (substring line 1)) nil))
+	 ;; complex expression
+	 (t (calc-push-list
+	 (list (let ((res (calc-eval line)))
+		 (cond
+		   ((numberp res) res)
+		   ((math-read-number res) (math-read-number res))
+		   ((listp res) (error "Calc error \"%s\" on input \"%s\""
+					 (cadr res) line))
+		   (t (replace-regexp-in-string "'" ""
+			(calc-eval
+			  (math-evaluate-expr
+				;; resolve user variables, calc built in
+				;; variables are handled automatically
+				;; upstream by calc
+				(mapcar #'org-babel-calc-maybe-resolve-var
+				;; parse line into calc objects
+				(car (math-read-exprs line))))))))))))))))
+
 (defun org-babel-calc-maybe-resolve-var (el)
   (if (consp el)
-      (if (and (equal 'var (car el)) (member (cadr el) org--var-syms))
+	(if (and (equal 'var (car el))
+		 (member (symbol-name (cadr el)) org--ob-calc-var-names))
 	  (progn
 	    (calc-recall (cadr el))
-	    (prog1 (calc-top 1)
+	    (prog1
+	      (calc-top 1)
 	      (calc-pop 1)))
-	(mapcar #'org-babel-calc-maybe-resolve-var el))
+	  (mapcar #'org-babel-calc-maybe-resolve-var el))
     el))
 
 (provide 'ob-calc)
 
-
-
 ;;; ob-calc.el ends here
-- 
2.6.2

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

* [PATCH 9/9] ob-calc: don't leave garbage on the stack
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
                   ` (7 preceding siblings ...)
  2015-11-03 20:15 ` [PATCH 8/9] ob-calc: add more API, documentation and examples so that it can be used in tables Jan Malakhovski
@ 2015-11-03 20:15 ` Jan Malakhovski
  2015-11-04 11:31   ` Aaron Ecay
  2015-11-04 11:36 ` [PATCH v2 0/9] mail, clock and calc changes Aaron Ecay
  9 siblings, 1 reply; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-03 20:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

* lisp/ob-calc.el (org-babel-calc-eval-string): Clean up the stack after expression
  evaluation.
---
 lisp/ob-calc.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-calc.el b/lisp/ob-calc.el
index e8b43e7..2656f27 100644
--- a/lisp/ob-calc.el
+++ b/lisp/ob-calc.el
@@ -196,7 +196,9 @@ See `org-babel-calc-eval' for more info."
   (mapc #'org-babel-calc-eval-line (split-string text "[\n\r]"))
   (save-excursion
     (with-current-buffer (get-buffer "*Calculator*")
-      (calc-eval (calc-top 1)))))
+      (prog1
+        (calc-eval (calc-top 1)
+        (calc-pop 1))))))
 
 (defun org-babel-calc-eval-line (line)
   (let ((line (org-babel-trim line)))
-- 
2.6.2

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

* Re: [PATCH 3/9] org-clock: fix `org-clock-time%'
  2015-11-03 20:15 ` [PATCH 3/9] org-clock: fix `org-clock-time%' Jan Malakhovski
@ 2015-11-04 11:18   ` Aaron Ecay
  2015-11-04 11:46     ` Jan Malakhovski
  0 siblings, 1 reply; 24+ messages in thread
From: Aaron Ecay @ 2015-11-04 11:18 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

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

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

* Re: [PATCH 5/9] rename `org-duration-string-to-minutes' to `org-clocksum-string-to-minutes' everywhere
  2015-11-03 20:15 ` [PATCH 5/9] rename `org-duration-string-to-minutes' to `org-clocksum-string-to-minutes' everywhere Jan Malakhovski
@ 2015-11-04 11:21   ` Aaron Ecay
  2015-11-04 11:47     ` Jan Malakhovski
  0 siblings, 1 reply; 24+ messages in thread
From: Aaron Ecay @ 2015-11-04 11:21 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

Hi Jan,

2015ko azaroak 3an, Jan Malakhovski-ek idatzi zuen:
> 
> * lisp/org-agenda.el:
> * lisp/org-clock.el:
> * lisp/org-colview.el:
> * lisp/org.el:
> * contrib/lisp/org-depend.el:
> * contrib/lisp/ox-taskjuggler.el: Rename (org-duration-string-to-minutes)
>   to (org-clocksum-string-to-minutes).
> ---
>  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 6313f52..ab4595b 100644
> --- a/lisp/org-agenda.el
> +++ b/lisp/org-agenda.el
> @@ -7665,7 +7665,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 4563a8a..ab65d3b 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 d27abc3..bbbeea9 100644
> --- a/lisp/org-colview.el
> +++ b/lisp/org-colview.el
> @@ -1119,7 +1119,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 a0fe644..c466870 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
>  
> @@ -15673,7 +15673,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))
> @@ -16534,7 +16534,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)
> @@ -18328,7 +18328,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
> @@ -18348,6 +18348,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")
> +

You can use ‘define-obsolete-function-alias’.

-- 
Aaron Ecay

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

* Re: [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props
  2015-11-03 20:15 ` [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props Jan Malakhovski
@ 2015-11-04 11:26   ` Aaron Ecay
  2015-11-04 11:45     ` Jan Malakhovski
  0 siblings, 1 reply; 24+ messages in thread
From: Aaron Ecay @ 2015-11-04 11:26 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

Hi Jan,

2015ko azaroak 3an, Jan Malakhovski-ek idatzi zuen:

[...]

> diff --git a/lisp/org.el b/lisp/org.el
> index c466870..cf0ef1f 100755
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -9960,7 +9960,7 @@ active region."
>  	 (car org-stored-links))))))
>  
>  (defun org-store-link-props (&rest plist)
> -  "Store link properties, extract names and addresses."
> +  "Store link properties, extract names, addresses and dates."
>    (let (x adr)
>      (when (setq x (plist-get plist :from))
>        (setq adr (mail-extract-address-components x))
> @@ -9969,7 +9969,18 @@ active region."
>      (when (setq x (plist-get plist :to))
>        (setq adr (mail-extract-address-components x))
>        (setq plist (plist-put plist :toname (car adr)))
> -      (setq plist (plist-put plist :toaddress (nth 1 adr)))))
> +      (setq plist (plist-put plist :toaddress (nth 1 adr))))
> +    (when (setq x (plist-get plist :date))
> +      (setq plist (plist-put plist :date-timestamp
> +			(ignore-errors
> +			  (format-time-string
> +			    (org-time-stamp-format t)
> +			    (date-to-time x)))))
> +      (setq plist (plist-put plist :date-timestamp-inactive
> +			(ignore-errors
> +			  (format-time-string
> +			    (org-time-stamp-format t t)
> +			    (date-to-time x)))))))

Can you introduce a let binding so that (date-to-time x) is only called
once?  Also, what is ignore-errors protecting?  The call to date-to-time,
or format-time-string?  I think the scope of ignore-errors should be as
narrow as it can be.

-- 
Aaron Ecay

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

* Re: [PATCH 9/9] ob-calc: don't leave garbage on the stack
  2015-11-03 20:15 ` [PATCH 9/9] ob-calc: don't leave garbage on the stack Jan Malakhovski
@ 2015-11-04 11:31   ` Aaron Ecay
  2015-11-04 11:53     ` Jan Malakhovski
  0 siblings, 1 reply; 24+ messages in thread
From: Aaron Ecay @ 2015-11-04 11:31 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

Hi Jan,

2015ko azaroak 3an, Jan Malakhovski-ek idatzi zuen:
> 
> * lisp/ob-calc.el (org-babel-calc-eval-string): Clean up the stack after expression
>   evaluation.
> ---
>  lisp/ob-calc.el | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lisp/ob-calc.el b/lisp/ob-calc.el
> index e8b43e7..2656f27 100644
> --- a/lisp/ob-calc.el
> +++ b/lisp/ob-calc.el
> @@ -196,7 +196,9 @@ See `org-babel-calc-eval' for more info."
>    (mapc #'org-babel-calc-eval-line (split-string text "[\n\r]"))
>    (save-excursion
>      (with-current-buffer (get-buffer "*Calculator*")
> -      (calc-eval (calc-top 1)))))
> +      (prog1
> +        (calc-eval (calc-top 1)

Are you missing a close paren at the end of the above line?  Also,
shouldn’t calc-eval take a string as an argument, not a lisp form?  (I’m
asking based on the docstring, I don’t know the calc API at all).

-- 
Aaron Ecay

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

* Re: [PATCH v2 0/9] mail, clock and calc changes
  2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
                   ` (8 preceding siblings ...)
  2015-11-03 20:15 ` [PATCH 9/9] ob-calc: don't leave garbage on the stack Jan Malakhovski
@ 2015-11-04 11:36 ` Aaron Ecay
  2015-11-04 11:59   ` Jan Malakhovski
  9 siblings, 1 reply; 24+ messages in thread
From: Aaron Ecay @ 2015-11-04 11:36 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Jan Malakhovski

Hi Jan,

Thanks for the patches!  They look good to me.  I sent a few minor
comments about code style.  I didn’t review the 8th patch (ob-calc: add
more API, documentation and examples so that it can be used in tables)
because I didn’t feel familiar enough with the calc API to be useful.

Do you want somebody to apply the tinychange patches already?  Or do you
want to wait until your assignment is processed and apply them yourself?
(IMO patch 4 could also be a TINYCHANGE, since it is only code movement
with no actual changes).

-- 
Aaron Ecay

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

* Re: [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props
  2015-11-04 11:26   ` Aaron Ecay
@ 2015-11-04 11:45     ` Jan Malakhovski
  2015-11-04 14:39       ` Aaron Ecay
  0 siblings, 1 reply; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-04 11:45 UTC (permalink / raw)
  To: Aaron Ecay, emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> Can you introduce a let binding so that (date-to-time x) is only called
> once?

Ok.

> Also, what is ignore-errors protecting? The call to date-to-time, or
> format-time-string? I think the scope of ignore-errors should be as
> narrow as it can be.

I have no idea. I moved these lines from org-gnus, equivalent lines in
org-*other-mail-reader*s don't use ignore-errors at all. I don't use
gnus so went for the safest change.

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

* Re: [PATCH 3/9] org-clock: fix `org-clock-time%'
  2015-11-04 11:18   ` Aaron Ecay
@ 2015-11-04 11:46     ` Jan Malakhovski
  0 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-04 11:46 UTC (permalink / raw)
  To: Aaron Ecay, emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

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

Ok.

>> -	  (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))))

Ok.

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

* Re: [PATCH 5/9] rename `org-duration-string-to-minutes' to `org-clocksum-string-to-minutes' everywhere
  2015-11-04 11:21   ` Aaron Ecay
@ 2015-11-04 11:47     ` Jan Malakhovski
  0 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-04 11:47 UTC (permalink / raw)
  To: Aaron Ecay, emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> You can use ‘define-obsolete-function-alias’.

Ok.

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

* Re: [PATCH 9/9] ob-calc: don't leave garbage on the stack
  2015-11-04 11:31   ` Aaron Ecay
@ 2015-11-04 11:53     ` Jan Malakhovski
  2015-11-04 14:41       ` Aaron Ecay
  0 siblings, 1 reply; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-04 11:53 UTC (permalink / raw)
  To: Aaron Ecay, emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> Are you missing a close paren at the end of the above line?

It evaluates and works ok, so I think it's ok.

> Also, shouldn’t calc-eval take a string as an argument, not a lisp
> form? (I’m asking based on the docstring, I don’t know the calc API at
> all).

1) It was like this before.
2) It works.
3) calc-eval calls calc-do-calc-eval on this argument and the code
   there does a dispatch on expression, so I think it's safe to assume
   it's not actually a string (calc is a mess, yep).

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

* Re: [PATCH v2 0/9] mail, clock and calc changes
  2015-11-04 11:36 ` [PATCH v2 0/9] mail, clock and calc changes Aaron Ecay
@ 2015-11-04 11:59   ` Jan Malakhovski
  0 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-04 11:59 UTC (permalink / raw)
  To: Aaron Ecay, emacs-orgmode

Hi.

Aaron Ecay <aaronecay@gmail.com> writes:

> Thanks for the patches!  They look good to me.  I sent a few minor
> comments about code style.  I didn’t review the 8th patch (ob-calc: add
> more API, documentation and examples so that it can be used in tables)
> because I didn’t feel familiar enough with the calc API to be useful.

Thanks for the comments!

> Do you want somebody to apply the tinychange patches already?  Or do you
> want to wait until your assignment is processed and apply them yourself?
> (IMO patch 4 could also be a TINYCHANGE, since it is only code movement
> with no actual changes).

I'm fine either way, including patch 4. If somebody merges those, I'll
just resend the rest when I fix the pieces you pointed to.

Cheers,
  Jan

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

* Re: [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props
  2015-11-04 11:45     ` Jan Malakhovski
@ 2015-11-04 14:39       ` Aaron Ecay
  2015-11-04 15:21         ` Jan Malakhovski
  0 siblings, 1 reply; 24+ messages in thread
From: Aaron Ecay @ 2015-11-04 14:39 UTC (permalink / raw)
  To: Jan Malakhovski, emacs-orgmode

Hi Jan,

2015ko azaroak 4an, Jan Malakhovski-ek idatzi zuen:

[...]

>> Also, what is ignore-errors protecting? The call to date-to-time, or
>> format-time-string? I think the scope of ignore-errors should be as
>> narrow as it can be.
> 
> I have no idea. I moved these lines from org-gnus, equivalent lines in
> org-*other-mail-reader*s don't use ignore-errors at all. I don't use
> gnus so went for the safest change.

Checking the source, date-to-time can raise an error (invalid date).
format-time-string is a C function, so it’s less easy for me to
understand.  But it looks like if it raises any errors, these are bugs
that we want to know about and not suppress.

The ignore-error call was introduced to org-gnus in commit 0dfde2da.
Based on the commit message, it looks like the problem being solved was
invalid dates getting passed to date-to-time.

So I think the ignore-error can just wrap the date-to-time call.

-- 
Aaron Ecay

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

* Re: [PATCH 9/9] ob-calc: don't leave garbage on the stack
  2015-11-04 11:53     ` Jan Malakhovski
@ 2015-11-04 14:41       ` Aaron Ecay
  2015-11-04 15:24         ` Jan Malakhovski
  0 siblings, 1 reply; 24+ messages in thread
From: Aaron Ecay @ 2015-11-04 14:41 UTC (permalink / raw)
  To: Jan Malakhovski, emacs-orgmode

2015ko azaroak 4an, Jan Malakhovski-ek idatzi zuen:
> 
> Aaron Ecay <aaronecay@gmail.com> writes:
> 
>> Are you missing a close paren at the end of the above line?
> 
> It evaluates and works ok, so I think it's ok.

Ok.  Then:
1. The indentation is wrong, because (calc-pop 1) is the second argument
   to calc-eval.
2. The prog1 form is not needed, because it only wraps a single form
   (the calc-eval call).

This looks wrong to me, so please double-check.

-- 
Aaron Ecay

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

* Re: [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props
  2015-11-04 14:39       ` Aaron Ecay
@ 2015-11-04 15:21         ` Jan Malakhovski
  0 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-04 15:21 UTC (permalink / raw)
  To: Aaron Ecay, emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> Checking the source, date-to-time can raise an error (invalid date).
> format-time-string is a C function, so it’s less easy for me to
> understand.  But it looks like if it raises any errors, these are bugs
> that we want to know about and not suppress.
>
> The ignore-error call was introduced to org-gnus in commit 0dfde2da.
> Based on the commit message, it looks like the problem being solved was
> invalid dates getting passed to date-to-time.
>
> So I think the ignore-error can just wrap the date-to-time call.

Thanks. Ok.

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

* Re: [PATCH 9/9] ob-calc: don't leave garbage on the stack
  2015-11-04 14:41       ` Aaron Ecay
@ 2015-11-04 15:24         ` Jan Malakhovski
  0 siblings, 0 replies; 24+ messages in thread
From: Jan Malakhovski @ 2015-11-04 15:24 UTC (permalink / raw)
  To: Aaron Ecay, emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> Ok.  Then:
> 1. The indentation is wrong, because (calc-pop 1) is the second argument
>    to calc-eval.
> 2. The prog1 form is not needed, because it only wraps a single form
>    (the calc-eval call).
>
> This looks wrong to me, so please double-check.

Ah, I misunderstood. You are correct about parens indeed. Fixed, ok. Why
it wored as expected I have no idea.

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

end of thread, other threads:[~2015-11-04 15:24 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-03 20:15 [PATCH v2 0/9] mail, clock and calc changes Jan Malakhovski
2015-11-03 20:15 ` [PATCH 1/9] org-clock: fix a typo Jan Malakhovski
2015-11-03 20:15 ` [PATCH 2/9] org-colview: add a FIXME Jan Malakhovski
2015-11-03 20:15 ` [PATCH 3/9] org-clock: fix `org-clock-time%' Jan Malakhovski
2015-11-04 11:18   ` Aaron Ecay
2015-11-04 11:46     ` Jan Malakhovski
2015-11-03 20:15 ` [PATCH 4/9] org: move `org-duration-string-to-minutes' to a better place Jan Malakhovski
2015-11-03 20:15 ` [PATCH 5/9] rename `org-duration-string-to-minutes' to `org-clocksum-string-to-minutes' everywhere Jan Malakhovski
2015-11-04 11:21   ` Aaron Ecay
2015-11-04 11:47     ` Jan Malakhovski
2015-11-03 20:15 ` [PATCH 6/9] factor out date-timestamp* calculations to org-store-link-props Jan Malakhovski
2015-11-04 11:26   ` Aaron Ecay
2015-11-04 11:45     ` Jan Malakhovski
2015-11-04 14:39       ` Aaron Ecay
2015-11-04 15:21         ` Jan Malakhovski
2015-11-03 20:15 ` [PATCH 7/9] org-notmuch: add date support to org-notmuch-store-link Jan Malakhovski
2015-11-03 20:15 ` [PATCH 8/9] ob-calc: add more API, documentation and examples so that it can be used in tables Jan Malakhovski
2015-11-03 20:15 ` [PATCH 9/9] ob-calc: don't leave garbage on the stack Jan Malakhovski
2015-11-04 11:31   ` Aaron Ecay
2015-11-04 11:53     ` Jan Malakhovski
2015-11-04 14:41       ` Aaron Ecay
2015-11-04 15:24         ` Jan Malakhovski
2015-11-04 11:36 ` [PATCH v2 0/9] mail, clock and calc changes Aaron Ecay
2015-11-04 11:59   ` Jan Malakhovski

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).