emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH 00/10] babel cleanups
@ 2013-04-01  5:42 Aaron Ecay
  2013-04-01  5:42 ` [PATCH 01/10] Fix org-babel-R-initiate-session Aaron Ecay
                   ` (10 more replies)
  0 siblings, 11 replies; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

Here are several patches to fix things in and around org-babel.
They're each independent of the others (and hopefully all apply
cleanly, without depending on other members of the series).  Here's a
little summary of each:

Aaron Ecay (10):
  Fix org-babel-R-initiate-session
  -> An obvious bugfix
  Clean up org-babel-expand-body: functions for awk and picolisp
  -> Makes these functions consistent with other babel languages,
     though I don't use these languages so can't test
  Clean up various org-babel-*-maybe commands
  -> Code simplification and avoids an expensive operation under some circumstances 
  Add 'light argument to some uses of org-babel-get-src-block-info
  -> Avoids an expensive operation
  Remove info arg from several org-babel functions
  -> Code cleanup.  Could break third-party code.
  Use prefix arg in org-edit-special
  -> Makes the function consistent with its docstring, although the new behavior is
     somewhat odd (C-u C-c ' becomes basically the same as C-c C-v C-z, AFAICT)
  Simplify org-babel-execute-src-block
  -> Makes data flow cleaner through this function
  Fix testing/lisp/test-ob-emacs-lisp.el
  -> Obvious bugfix
  Remove org-babel-check-confirm-evaluate macro
  -> Refactoring.  Of all the patches, I am least sure of this one.
     It is a complicated operation however you slice it, but I find
     the approach where the complexity is local easier to understand.
     Deserves careful review, since it touches code which decides whether
     to evaluate source blocks.  Read: has security implications.
  Document how :var introduces code block dependencies.
  -> Documentation.

 doc/org.texi                       |   9 +++
 lisp/ob-R.el                       |  20 +++--
 lisp/ob-awk.el                     |   2 +-
 lisp/ob-core.el                    | 156 +++++++++++++++++--------------------
 lisp/ob-picolisp.el                |   2 +-
 lisp/ob-tangle.el                  |   2 +-
 lisp/org.el                        |   7 +-
 testing/lisp/test-ob-emacs-lisp.el |  19 +++--
 8 files changed, 112 insertions(+), 105 deletions(-)

-- 
1.8.2

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

* [PATCH 01/10] Fix org-babel-R-initiate-session
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-03 13:46   ` Eric Schulte
  2013-04-01  5:42 ` [PATCH 02/10] Clean up org-babel-expand-body: functions for awk and picolisp Aaron Ecay
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-R.el (org-babel-R-initiate-session): handle case where the
  session buffer exists, but does not have a live process

If the session buffer exists, but the user has exited the R process
manually, then the (R) command will create a new buffer, then try to
rename it over the old buffer, causing an error.  The right thing to do
is to start R within the existing buffer.
---
 lisp/ob-R.el | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 9875f81..de9ec5b 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -209,14 +209,18 @@ This function is called by `org-babel-execute-src-block'."
       (if (org-babel-comint-buffer-livep session)
 	  session
 	(save-window-excursion
-	  (require 'ess) (R)
-	  (rename-buffer
-	   (if (bufferp session)
-	       (buffer-name session)
-	     (if (stringp session)
-		 session
-	       (buffer-name))))
-	  (current-buffer))))))
+	  (save-excursion
+	    (when (get-buffer session)
+	      ;; Session buffer exists, but with dead process
+	      (set-buffer session))
+	    (require 'ess) (R)
+	    (rename-buffer
+	     (if (bufferp session)
+		 (buffer-name session)
+	       (if (stringp session)
+		   session
+		 (buffer-name))))
+	    (current-buffer)))))))
 
 (defun org-babel-R-associate-session (session)
   "Associate R code buffer with an R session.
-- 
1.8.2

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

* [PATCH 02/10] Clean up org-babel-expand-body: functions for awk and picolisp
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
  2013-04-01  5:42 ` [PATCH 01/10] Fix org-babel-R-initiate-session Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-03 13:56   ` Eric Schulte
  2013-04-01  5:42 ` [PATCH 03/10] Clean up various org-babel-*-maybe commands Aaron Ecay
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-awk.el (org-babel-expand-body:awk),
  lisp/ob-picolisp.el (org-babel-expand-body:picolisp): remove optional
  arg from these functions

The optional argument is apparently never passed by org-babel code.
Maybe this is a relic of an earlier calling convention?
---
 lisp/ob-awk.el      | 2 +-
 lisp/ob-picolisp.el | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/ob-awk.el b/lisp/ob-awk.el
index f717fec..373d5fd 100644
--- a/lisp/ob-awk.el
+++ b/lisp/ob-awk.el
@@ -44,7 +44,7 @@
 (defvar org-babel-awk-command "awk"
   "Name of the awk executable command.")
 
-(defun org-babel-expand-body:awk (body params &optional processed-params)
+(defun org-babel-expand-body:awk (body params)
   "Expand BODY according to PARAMS, return the expanded body."
   (dolist (pair (mapcar #'cdr (org-babel-get-header params :var)))
     (setf body (replace-regexp-in-string
diff --git a/lisp/ob-picolisp.el b/lisp/ob-picolisp.el
index e785366..1d17919 100644
--- a/lisp/ob-picolisp.el
+++ b/lisp/ob-picolisp.el
@@ -78,7 +78,7 @@
   :version "24.1"
   :type 'string)
 
-(defun org-babel-expand-body:picolisp (body params &optional processed-params)
+(defun org-babel-expand-body:picolisp (body params)
   "Expand BODY according to PARAMS, return the expanded body."
   (let ((vars (mapcar #'cdr (org-babel-get-header params :var)))
         (result-params (cdr (assoc :result-params params)))
-- 
1.8.2

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

* [PATCH 03/10] Clean up various org-babel-*-maybe commands
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
  2013-04-01  5:42 ` [PATCH 01/10] Fix org-babel-R-initiate-session Aaron Ecay
  2013-04-01  5:42 ` [PATCH 02/10] Clean up org-babel-expand-body: functions for awk and picolisp Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-02 19:31   ` Achim Gratz
  2013-04-03 14:18   ` Bastien
  2013-04-01  5:42 ` [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info Aaron Ecay
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-core.el (org-babel-if-in-src-block): New macro
(org-babel-execute-src-block-maybe),
(org-babel-expand-src-block-maybe),
(org-babel-load-in-session-maybe),
(org-babel-pop-to-session-maybe): Use it

org-babel-get-src-block-info is a potentially expensive operation, which
is why its ‘light’ argument exists.  But in any case, it is overkill to
query the whole info, if all that is needed is whether point is in a
block or not.  Factor the simplified common code out into a macro.
---
 lisp/ob-core.el | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 723aa9d..283628e 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -365,15 +365,22 @@ of potentially harmful code."
   (or (org-babel-execute-src-block-maybe)
       (org-babel-lob-execute-maybe)))
 
+(defmacro org-babel-when-in-src-block (&rest body)
+  `(if (or (org-babel-where-is-src-block-head)
+           (org-babel-get-inline-src-block-matches))
+       (progn
+	 ,@body
+	 t)
+     nil))
+
 (defun org-babel-execute-src-block-maybe ()
   "Conditionally execute a source block.
 Detect if this is context for a Babel src-block and if so
 then run `org-babel-execute-src-block'."
   (interactive)
-  (let ((info (org-babel-get-src-block-info)))
-    (if info
-	(progn (org-babel-eval-wipe-error-buffer)
-	       (org-babel-execute-src-block current-prefix-arg info) t) nil)))
+  (org-babel-when-in-src-block
+   (org-babel-eval-wipe-error-buffer)
+   (org-babel-execute-src-block current-prefix-arg)))
 
 ;;;###autoload
 (defun org-babel-view-src-block-info ()
@@ -409,10 +416,8 @@ a window into the `org-babel-get-src-block-info' function."
 Detect if this is context for a org-babel src-block and if so
 then run `org-babel-expand-src-block'."
   (interactive)
-  (let ((info (org-babel-get-src-block-info)))
-    (if info
-	(progn (org-babel-expand-src-block current-prefix-arg info) t)
-      nil)))
+  (org-babel-when-in-src-block
+   (org-babel-expand-src-block current-prefix-arg)))
 
 ;;;###autoload
 (defun org-babel-load-in-session-maybe ()
@@ -420,10 +425,8 @@ then run `org-babel-expand-src-block'."
 Detect if this is context for a org-babel src-block and if so
 then run `org-babel-load-in-session'."
   (interactive)
-  (let ((info (org-babel-get-src-block-info)))
-    (if info
-	(progn (org-babel-load-in-session current-prefix-arg info) t)
-      nil)))
+  (org-babel-when-in-src-block
+   (org-babel-load-in-session current-prefix-arg)))
 
 (add-hook 'org-metaup-hook 'org-babel-load-in-session-maybe)
 
@@ -433,8 +436,8 @@ then run `org-babel-load-in-session'."
 Detect if this is context for a org-babel src-block and if so
 then run `org-babel-pop-to-session'."
   (interactive)
-  (let ((info (org-babel-get-src-block-info)))
-    (if info (progn (org-babel-pop-to-session current-prefix-arg info) t) nil)))
+  (org-babel-when-in-src-block
+   (org-babel-pop-to-session current-prefix-arg)))
 
 (add-hook 'org-metadown-hook 'org-babel-pop-to-session-maybe)
 
-- 
1.8.2

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

* [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
                   ` (2 preceding siblings ...)
  2013-04-01  5:42 ` [PATCH 03/10] Clean up various org-babel-*-maybe commands Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-03 14:09   ` Eric Schulte
  2013-04-01  5:42 ` [PATCH 05/10] Remove info arg from several org-babel functions Aaron Ecay
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-core.el (org-babel-do-key-sequence-in-edit-buffer),
(org-babel-expand-noweb-references),
* lisp/ob-tangle.el (org-babel-tangle):
Use 'light argument to `org-babel-get-src-block-info'.
---
 lisp/ob-core.el   | 4 ++--
 lisp/ob-tangle.el | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 283628e..0aae998 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -912,7 +912,7 @@ source code block, otherwise return nil.  With optional prefix
 argument RE-RUN the source-code block is evaluated even if
 results already exist."
   (interactive "P")
-  (let ((info (org-babel-get-src-block-info)))
+  (let ((info (org-babel-get-src-block-info 'light)))
     (when info
       (save-excursion
 	;; go to the results, if there aren't any then run the block
@@ -2358,7 +2358,7 @@ would set the value of argument \"a\" equal to \"9\".  Note that
 these arguments are not evaluated in the current source-code
 block but are passed literally to the \"example-block\"."
   (let* ((parent-buffer (or parent-buffer (current-buffer)))
-         (info (or info (org-babel-get-src-block-info)))
+         (info (or info (org-babel-get-src-block-info 'light)))
          (lang (nth 0 info))
          (body (nth 1 info))
 	 (ob-nww-start org-babel-noweb-wrap-start)
diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el
index 4bcb2e3..4c6eb5c 100644
--- a/lisp/ob-tangle.el
+++ b/lisp/ob-tangle.el
@@ -211,7 +211,7 @@ used to limit the exported source code blocks by language."
 	       org-babel-default-header-args))
 	    (tangle-file
 	     (when (equal arg '(16))
-	       (or (cdr (assoc :tangle (nth 2 (org-babel-get-src-block-info))))
+	       (or (cdr (assoc :tangle (nth 2 (org-babel-get-src-block-info 'light))))
 		   (user-error "Point is not in a source code block"))))
 	    path-collector)
 	(mapc ;; map over all languages
-- 
1.8.2

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

* [PATCH 05/10] Remove info arg from several org-babel functions
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
                   ` (3 preceding siblings ...)
  2013-04-01  5:42 ` [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-03 13:58   ` Eric Schulte
  2013-04-01  5:42 ` [PATCH 06/10] Use prefix arg in org-edit-special Aaron Ecay
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-core.el (org-babel-load-in-session),
  (org-babel-initiate-session),
  (org-babel-switch-to-session)
  (org-babel-switch-to-session-with-code): Remove info optional arg

The info arg is threaded through this code, but never used by
callers (at least in org code).
---
 lisp/ob-core.el | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 0aae998..c69b736 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -794,13 +794,13 @@ arguments and pop open the results in a preview buffer."
 (add-hook 'org-tab-first-hook 'org-babel-header-arg-expand)
 
 ;;;###autoload
-(defun org-babel-load-in-session (&optional arg info)
+(defun org-babel-load-in-session (&optional arg)
   "Load the body of the current source-code block.
 Evaluate the header arguments for the source block before
 entering the session.  After loading the body this pops open the
 session."
   (interactive)
-  (let* ((info (or info (org-babel-get-src-block-info)))
+  (let* ((info (org-babel-get-src-block-info))
          (lang (nth 0 info))
          (params (nth 2 info))
          (body (if (not info)
@@ -820,13 +820,13 @@ session."
     (end-of-line 1)))
 
 ;;;###autoload
-(defun org-babel-initiate-session (&optional arg info)
+(defun org-babel-initiate-session (&optional arg)
   "Initiate session for current code block.
 If called with a prefix argument then resolve any variable
 references in the header arguments and assign these variables in
 the session.  Copy the body of the code block to the kill ring."
   (interactive "P")
-  (let* ((info (or info (org-babel-get-src-block-info (not arg))))
+  (let* ((info (org-babel-get-src-block-info (not arg)))
          (lang (nth 0 info))
          (body (nth 1 info))
          (params (nth 2 info))
@@ -849,19 +849,19 @@ the session.  Copy the body of the code block to the kill ring."
     (funcall init-cmd session params)))
 
 ;;;###autoload
-(defun org-babel-switch-to-session (&optional arg info)
+(defun org-babel-switch-to-session (&optional arg)
   "Switch to the session of the current code block.
 Uses `org-babel-initiate-session' to start the session.  If called
 with a prefix argument then this is passed on to
 `org-babel-initiate-session'."
   (interactive "P")
-  (pop-to-buffer (org-babel-initiate-session arg info))
+  (pop-to-buffer (org-babel-initiate-session arg))
   (end-of-line 1))
 
 (defalias 'org-babel-pop-to-session 'org-babel-switch-to-session)
 
 ;;;###autoload
-(defun org-babel-switch-to-session-with-code (&optional arg info)
+(defun org-babel-switch-to-session-with-code (&optional arg)
   "Switch to code buffer and display session."
   (interactive "P")
   (let ((swap-windows
@@ -870,10 +870,9 @@ with a prefix argument then this is passed on to
 	     (set-window-buffer (next-window) (current-buffer))
 	     (set-window-buffer (selected-window) other-window-buffer))
 	   (other-window 1)))
-	(info (org-babel-get-src-block-info))
 	(org-src-window-setup 'reorganize-frame))
     (save-excursion
-      (org-babel-switch-to-session arg info))
+      (org-babel-switch-to-session arg))
     (org-edit-src-code)
     (funcall swap-windows)))
 
-- 
1.8.2

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

* [PATCH 06/10] Use prefix arg in org-edit-special
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
                   ` (4 preceding siblings ...)
  2013-04-01  5:42 ` [PATCH 05/10] Remove info arg from several org-babel functions Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-03 13:42   ` Eric Schulte
  2013-04-01  5:42 ` [PATCH 07/10] Simplify org-babel-execute-src-block Aaron Ecay
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/org.el (org-edit-special): Use prefix arg, as docstring says we
  do

Only makes a difference for src-block editing.
---
 lisp/org.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 04ce386..1edfbc4 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -19943,7 +19943,7 @@ When in a fixed-width region, call `org-edit-fixed-width-region'.
 When at an #+INCLUDE keyword, visit the included file.
 On a link, call `ffap' to visit the link at point.
 Otherwise, return a user error."
-  (interactive)
+  (interactive "P")
   (let ((element (org-element-at-point)))
     (assert (not buffer-read-only) nil
 	    "Buffer is read-only: %s" (buffer-name))
@@ -19958,8 +19958,9 @@ Otherwise, return a user error."
              ;; At a src-block with a session and function called with
              ;; an ARG: switch to the buffer related to the inferior
              ;; process.
-             (funcall (intern (concat "org-babel-prep-session:" lang))
-                      session params)))))
+             (switch-to-buffer
+	      (funcall (intern (concat "org-babel-prep-session:" lang))
+		       session params))))))
       (keyword
        (if (member (org-element-property :key element) '("INCLUDE" "SETUPFILE"))
            (find-file
-- 
1.8.2

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

* [PATCH 07/10] Simplify org-babel-execute-src-block
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
                   ` (5 preceding siblings ...)
  2013-04-01  5:42 ` [PATCH 06/10] Use prefix arg in org-edit-special Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-02 19:41   ` Achim Gratz
  2013-04-01  5:42 ` [PATCH 08/10] Fix testing/lisp/test-ob-emacs-lisp.el Aaron Ecay
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-core.el (org-babel-execute-src-block): Simplify control flow

Avoid potential duplication of org-babel-process-params call.  Also
makes the code simpler.
---
 lisp/ob-core.el | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index c69b736..d8c11ee 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -557,13 +557,11 @@ Optionally supply a value for PARAMS which will be merged with
 the header arguments specified at the front of the source code
 block."
   (interactive)
-  (let* ((info (or info (org-babel-get-src-block-info)))
-	 (merged-params (org-babel-merge-params (nth 2 info) params)))
-    (when (org-babel-check-evaluate
-	   (let ((i info)) (setf (nth 2 i) merged-params) i))
-      (let* ((params (if params
-			 (org-babel-process-params merged-params)
-		       (nth 2 info)))
+  (let* ((info (or info (org-babel-get-src-block-info 'light))))
+    (setf (nth 2 info) (org-babel-merge-params (nth 2 info) params))
+    (when (org-babel-check-evaluate info)
+      (setf (nth 2 info) (org-babel-process-params (nth 2 info)))
+      (let* ((params (nth 2 info))
 	     (cachep (and (not arg) (cdr (assoc :cache params))
 			   (string= "yes" (cdr (assoc :cache params)))))
 	     (new-hash (when cachep (org-babel-sha1-hash info)))
@@ -578,8 +576,7 @@ block."
 	    (let ((result (org-babel-read-result)))
 	      (message (replace-regexp-in-string
 			"%" "%%" (format "%S" result))) result)))
-	 ((org-babel-confirm-evaluate
-	   (let ((i info)) (setf (nth 2 i) merged-params) i))
+	 ((org-babel-confirm-evaluate info)
 	  (let* ((lang (nth 0 info))
 		 (result-params (cdr (assoc :result-params params)))
 		 (body (setf (nth 1 info)
-- 
1.8.2

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

* [PATCH 08/10] Fix testing/lisp/test-ob-emacs-lisp.el
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
                   ` (6 preceding siblings ...)
  2013-04-01  5:42 ` [PATCH 07/10] Simplify org-babel-execute-src-block Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-03 13:47   ` Eric Schulte
  2013-04-01  5:42 ` [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro Aaron Ecay
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* testing/lisp/test-ob-emacs-lisp.el: Move stray test inside ert-deftest
---
 testing/lisp/test-ob-emacs-lisp.el | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/testing/lisp/test-ob-emacs-lisp.el b/testing/lisp/test-ob-emacs-lisp.el
index 94092e4..d03f048 100644
--- a/testing/lisp/test-ob-emacs-lisp.el
+++ b/testing/lisp/test-ob-emacs-lisp.el
@@ -62,17 +62,20 @@
     (should (string=
 	     ""
 	     (buffer-substring-no-properties (point-at-bol) (point-at-eol))))))
-(org-test-with-temp-text-in-file "
+
+(ert-deftest ob-emacs-lisp/commented-last-block-line ()
+  (org-test-with-temp-text-in-file "
 #+begin_src emacs-lisp :var a=2
 2;;
 #+end_src"
-  (org-babel-next-src-block)
-  (org-ctrl-c-ctrl-c)
-  (re-search-forward "results" nil t)
-  (forward-line)
-  (should (string=
-	   ": 2"
-	   (buffer-substring-no-properties (point-at-bol) (point-at-eol)))))
+    (org-babel-next-src-block)
+    (org-ctrl-c-ctrl-c)
+    (re-search-forward "results" nil t)
+    (forward-line)
+    (should (string=
+	     ": 2"
+	     (buffer-substring-no-properties (point-at-bol) (point-at-eol))))))
+
 (provide 'test-ob-emacs-lisp)
 
  ;;; test-ob-emacs-lisp.el ends here
-- 
1.8.2

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

* [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
                   ` (7 preceding siblings ...)
  2013-04-01  5:42 ` [PATCH 08/10] Fix testing/lisp/test-ob-emacs-lisp.el Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-02 19:53   ` Achim Gratz
  2013-04-01  5:42 ` [PATCH 10/10] Document how :var introduces code block dependencies Aaron Ecay
  2013-04-02 22:14 ` [PATCH 00/10] babel cleanups Eric Schulte
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-core.el (org-babel-check-confirm-evaluate): remove
  (org-babel-check-evaluate),
  (org-babel-confirm-evaluate): move logic here

This macro is used in only two places, and has two almost-independent
complex logics coded into it.  So, suppress the macro and move the logic
into the respective functions.
---
 lisp/ob-core.el | 89 ++++++++++++++++++++++++++-------------------------------
 1 file changed, 40 insertions(+), 49 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index d8c11ee..65c5a0b 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -284,49 +284,26 @@ Returns a list
       (setf (nth 2 info) (org-babel-process-params (nth 2 info))))
     (when info (append info (list name indent)))))
 
-(defvar org-current-export-file) ; dynamically bound
-(defmacro org-babel-check-confirm-evaluate (info &rest body)
-  "Evaluate BODY with special execution confirmation variables set.
-
-Specifically; NOEVAL will indicate if evaluation is allowed,
-QUERY will indicate if a user query is required, CODE-BLOCK will
-hold the language of the code block, and BLOCK-NAME will hold the
-name of the code block."
-  (declare (indent defun))
-  (org-with-gensyms
-      (lang block-body headers name eval eval-no export eval-no-export)
-    `(let* ((,lang           (nth 0 ,info))
-	    (,block-body     (nth 1 ,info))
-	    (,headers        (nth 2 ,info))
-	    (,name           (nth 4 ,info))
-	    (,eval           (or (cdr  (assoc :eval   ,headers))
-				 (when (assoc :noeval ,headers) "no")))
-	    (,eval-no        (or (equal ,eval "no")
-				 (equal ,eval "never")))
-	    (,export         (org-bound-and-true-p org-current-export-file))
-	    (,eval-no-export (and ,export (or (equal ,eval "no-export")
-					      (equal ,eval "never-export"))))
-	    (noeval          (or ,eval-no ,eval-no-export))
-	    (query           (or (equal ,eval "query")
-				 (and ,export (equal ,eval "query-export"))
-				 (when (functionp org-confirm-babel-evaluate)
-				   (funcall org-confirm-babel-evaluate
-					    ,lang ,block-body))
-				 org-confirm-babel-evaluate))
-	    (code-block      (if ,info (format  " %s "  ,lang) " "))
-	    (block-name      (if ,name (format " (%s) " ,name) " ")))
-       ,@body)))
+;; dynamically bound during export
+(defvar org-current-export-file)
+;; dynamically bound during asynchronous export
+(defvar org-babel-confirm-evaluate-answer-no)
 
 (defsubst org-babel-check-evaluate (info)
   "Check if code block INFO should be evaluated.
 Do not query the user."
-  (org-babel-check-confirm-evaluate info
-    (not (when noeval
-	   (message (format "Evaluation of this%scode-block%sis disabled."
-			    code-block block-name))))))
-
- ;; dynamically scoped for asynchroneous export
-(defvar org-babel-confirm-evaluate-answer-no)
+  (let* ((params (nth 2 info))
+	 (name (nth 4 info))
+	 (eval (cdr (assq :eval params)))
+         (can-eval (not (or (member eval '("never" "no"))
+			    (assq :noeval params)
+			    (and (org-bound-and-true-p org-current-export-file)
+				 (member eval '("no-export" "never-export")))))))
+    (when (not can-eval)
+      (message (format "Evaluation of this %s code-block (%s) is disabled."
+                       (nth 0 info)
+                       (if name (concat " (" name ") ") ""))))
+    can-eval))
 
 (defsubst org-babel-confirm-evaluate (info)
   "Confirm evaluation of the code block INFO.
@@ -341,16 +318,30 @@ confirmation from the user.
 
 Note disabling confirmation may result in accidental evaluation
 of potentially harmful code."
-  (org-babel-check-confirm-evaluate info
-    (not (when query
-	   (unless
-	       (and (not (org-bound-and-true-p
-			  org-babel-confirm-evaluate-answer-no))
-		    (yes-or-no-p
-		     (format "Evaluate this%scode block%son your system? "
-			     code-block block-name)))
-	     (message (format "Evaluation of this%scode-block%sis aborted."
-			      code-block block-name)))))))
+
+  (let* ((params (nth 2 info))
+         (name (if (nth 4 info) (concat " (" (nth 4 info) ") ") " "))
+	 (eval (cdr (assq :eval params)))
+         (should-query (or (equal eval "query")
+                           (and (org-bound-and-true-p org-current-export-file)
+                                (equal eval "query-export"))
+                           (and (functionp org-confirm-babel-evaluate)
+                                (funcall org-confirm-babel-evaluate
+                                         (nth 0 info)
+                                         (nth 1 info)))
+			   org-confirm-babel-evaluate))
+         (result (or (not should-query)
+		     (not (org-bound-and-true-p
+			   org-babel-confirm-evaluate-answer-no))
+		     (yes-or-no-p
+		      (format "Evaluate this %s code block%son your system? "
+			      (nth 0 info)
+			      name)))))
+    (when (not result)
+      (message (format "Evaluation of this %s code-block%sis aborted."
+		       (nth 0 info)
+		       name)))
+    result))
 
 ;;;###autoload
 (defun org-babel-execute-safely-maybe ()
-- 
1.8.2

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

* [PATCH 10/10] Document how :var introduces code block dependencies.
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
                   ` (8 preceding siblings ...)
  2013-04-01  5:42 ` [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro Aaron Ecay
@ 2013-04-01  5:42 ` Aaron Ecay
  2013-04-03 14:04   ` Eric Schulte
  2013-04-02 22:14 ` [PATCH 00/10] babel cleanups Eric Schulte
  10 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-01  5:42 UTC (permalink / raw)
  To: emacs-orgmode

* doc/org.texi: Document how :var introduces code block dependencies.
---
 doc/org.texi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/org.texi b/doc/org.texi
index 6791570..dda922e 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -13211,6 +13211,15 @@ include anything in the Org mode file that takes a @code{#+NAME:},
 @code{#+BEGIN_EXAMPLE} blocks, other code blocks, and the results of other
 code blocks.
 
+When a reference is made to another code block, the referenced block will be
+evaluated whenever needed, in order to supply its value to the referencing
+block.  If the referenced block is cached (see @ref{cache}), its value will
+be reused if possible, instead of being re-calculated.  If the referring code
+block is cached, its hash value will depend on the value of all the code
+blocks it references.  This system can thus be used to create a system of
+as-needed re-evaluation among code blocks similar to that provided by
+@uref{http://yihui.name/knitr/, knitr} or Sweave.
+
 Argument values can be indexed in a manner similar to arrays (see @ref{var,
 Indexable variable values}).
 
-- 
1.8.2

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

* Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands
  2013-04-01  5:42 ` [PATCH 03/10] Clean up various org-babel-*-maybe commands Aaron Ecay
@ 2013-04-02 19:31   ` Achim Gratz
  2013-04-03 14:18   ` Bastien
  1 sibling, 0 replies; 39+ messages in thread
From: Achim Gratz @ 2013-04-02 19:31 UTC (permalink / raw)
  To: emacs-orgmode

Aaron Ecay writes:
> * lisp/ob-core.el (org-babel-if-in-src-block): New macro
[…]
> +(defmacro org-babel-when-in-src-block (&rest body)
> +  `(if (or (org-babel-where-is-src-block-head)
> +           (org-babel-get-inline-src-block-matches))
> +       (progn
> +	 ,@body
> +	 t)
> +     nil))

Commit message and patch disagree about the name.

Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf rackAttack:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

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

* Re: [PATCH 07/10] Simplify org-babel-execute-src-block
  2013-04-01  5:42 ` [PATCH 07/10] Simplify org-babel-execute-src-block Aaron Ecay
@ 2013-04-02 19:41   ` Achim Gratz
  2013-04-03 13:54     ` Eric Schulte
  0 siblings, 1 reply; 39+ messages in thread
From: Achim Gratz @ 2013-04-02 19:41 UTC (permalink / raw)
  To: emacs-orgmode

Aaron Ecay writes:
> * lisp/ob-core.el (org-babel-execute-src-block): Simplify control flow
>
> Avoid potential duplication of org-babel-process-params call.  Also
> makes the code simpler.

You may be changing semantics here.  I'm not entirely certain if the
current way of dealing with the the unmerged and merged parameters and
the info block is necessary, but I'd be wary of such a change.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

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

* Re: [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro
  2013-04-01  5:42 ` [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro Aaron Ecay
@ 2013-04-02 19:53   ` Achim Gratz
  2013-04-03 14:05     ` Eric Schulte
  0 siblings, 1 reply; 39+ messages in thread
From: Achim Gratz @ 2013-04-02 19:53 UTC (permalink / raw)
  To: emacs-orgmode

Aaron Ecay writes:
> * lisp/ob-core.el (org-babel-check-confirm-evaluate): remove
>   (org-babel-check-evaluate),
>   (org-babel-confirm-evaluate): move logic here
>
> This macro is used in only two places, and has two almost-independent
> complex logics coded into it.  So, suppress the macro and move the logic
> into the respective functions.

I have recently introduced that macro because no amount of documentation
can guarantee that the two functions using these values compute them the
same way when somebody makes further changes down the road.  That is,
however, mandatory for these functions to work properly and safely.

I haven't checked if the logic hasn't changed with that patch, but I
don't think it's any easier to understand than before.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

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

* Re: [PATCH 00/10] babel cleanups
  2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
                   ` (9 preceding siblings ...)
  2013-04-01  5:42 ` [PATCH 10/10] Document how :var introduces code block dependencies Aaron Ecay
@ 2013-04-02 22:14 ` Eric Schulte
  2013-04-03 14:13   ` Eric Schulte
  10 siblings, 1 reply; 39+ messages in thread
From: Eric Schulte @ 2013-04-02 22:14 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> Here are several patches to fix things in and around org-babel.
> They're each independent of the others (and hopefully all apply
> cleanly, without depending on other members of the series).  Here's a
> little summary of each:
>

Thanks for these patches, many look like obvious wins, some less so.  I
likely won't have time to review them (or do much of anything) in the
near term, but at some point I will make time to review them and reply
here.

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 06/10] Use prefix arg in org-edit-special
  2013-04-01  5:42 ` [PATCH 06/10] Use prefix arg in org-edit-special Aaron Ecay
@ 2013-04-03 13:42   ` Eric Schulte
  2013-04-03 17:02     ` Bastien
  0 siblings, 1 reply; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 13:42 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> * lisp/org.el (org-edit-special): Use prefix arg, as docstring says we
>   do
>

This is beyond my ken.  I'll leave review of this patch to Bastien.

>
> Only makes a difference for src-block editing.
> ---
>  lisp/org.el | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 04ce386..1edfbc4 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -19943,7 +19943,7 @@ When in a fixed-width region, call `org-edit-fixed-width-region'.
>  When at an #+INCLUDE keyword, visit the included file.
>  On a link, call `ffap' to visit the link at point.
>  Otherwise, return a user error."
> -  (interactive)
> +  (interactive "P")
>    (let ((element (org-element-at-point)))
>      (assert (not buffer-read-only) nil
>  	    "Buffer is read-only: %s" (buffer-name))
> @@ -19958,8 +19958,9 @@ Otherwise, return a user error."
>               ;; At a src-block with a session and function called with
>               ;; an ARG: switch to the buffer related to the inferior
>               ;; process.
> -             (funcall (intern (concat "org-babel-prep-session:" lang))
> -                      session params)))))
> +             (switch-to-buffer
> +	      (funcall (intern (concat "org-babel-prep-session:" lang))
> +		       session params))))))
>        (keyword
>         (if (member (org-element-property :key element) '("INCLUDE" "SETUPFILE"))
>             (find-file

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 01/10] Fix org-babel-R-initiate-session
  2013-04-01  5:42 ` [PATCH 01/10] Fix org-babel-R-initiate-session Aaron Ecay
@ 2013-04-03 13:46   ` Eric Schulte
  0 siblings, 0 replies; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 13:46 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> * lisp/ob-R.el (org-babel-R-initiate-session): handle case where the
>   session buffer exists, but does not have a live process
>

Applied, but I removed an unnecessary save-excursion nested inside of a
save-window-excursion.

Thanks!

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 08/10] Fix testing/lisp/test-ob-emacs-lisp.el
  2013-04-01  5:42 ` [PATCH 08/10] Fix testing/lisp/test-ob-emacs-lisp.el Aaron Ecay
@ 2013-04-03 13:47   ` Eric Schulte
  0 siblings, 0 replies; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 13:47 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> * testing/lisp/test-ob-emacs-lisp.el: Move stray test inside
> ert-deftest

Applied, Thanks!

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 07/10] Simplify org-babel-execute-src-block
  2013-04-02 19:41   ` Achim Gratz
@ 2013-04-03 13:54     ` Eric Schulte
  2013-04-03 17:05       ` Achim Gratz
  0 siblings, 1 reply; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 13:54 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Aaron Ecay writes:
>> * lisp/ob-core.el (org-babel-execute-src-block): Simplify control flow
>>
>> Avoid potential duplication of org-babel-process-params call.  Also
>> makes the code simpler.
>
> You may be changing semantics here.  I'm not entirely certain if the
> current way of dealing with the the unmerged and merged parameters and
> the info block is necessary, but I'd be wary of such a change.
>

Can you check if this change causes any of the existing tests to fail?

Thanks,

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 02/10] Clean up org-babel-expand-body: functions for awk and picolisp
  2013-04-01  5:42 ` [PATCH 02/10] Clean up org-babel-expand-body: functions for awk and picolisp Aaron Ecay
@ 2013-04-03 13:56   ` Eric Schulte
  0 siblings, 0 replies; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 13:56 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> * lisp/ob-awk.el (org-babel-expand-body:awk),
>   lisp/ob-picolisp.el (org-babel-expand-body:picolisp): remove optional
>   arg from these functions
>
> The optional argument is apparently never passed by org-babel code.
> 

Applied, Thanks!

> 
> Maybe this is a relic of an earlier calling convention?
> 

Yes, that is exactly the case.

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 05/10] Remove info arg from several org-babel functions
  2013-04-01  5:42 ` [PATCH 05/10] Remove info arg from several org-babel functions Aaron Ecay
@ 2013-04-03 13:58   ` Eric Schulte
  2013-04-18  7:07     ` Aaron Ecay
  0 siblings, 1 reply; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 13:58 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> * lisp/ob-core.el (org-babel-load-in-session),
>   (org-babel-initiate-session),
>   (org-babel-switch-to-session)
>   (org-babel-switch-to-session-with-code): Remove info optional arg
>
> The info arg is threaded through this code, but never used by
> callers (at least in org code).

The rgrep command disagrees with this last statement.

./ob-core.el:411:	(progn (org-babel-load-in-session current-prefix-arg info) t)

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 10/10] Document how :var introduces code block dependencies.
  2013-04-01  5:42 ` [PATCH 10/10] Document how :var introduces code block dependencies Aaron Ecay
@ 2013-04-03 14:04   ` Eric Schulte
  0 siblings, 0 replies; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 14:04 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> * doc/org.texi: Document how :var introduces code block dependencies.

I've committed this patch, although I then simplified your discussion of
code block re-execution.

Thanks!

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro
  2013-04-02 19:53   ` Achim Gratz
@ 2013-04-03 14:05     ` Eric Schulte
  0 siblings, 0 replies; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 14:05 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Aaron Ecay writes:
>> * lisp/ob-core.el (org-babel-check-confirm-evaluate): remove
>>   (org-babel-check-evaluate),
>>   (org-babel-confirm-evaluate): move logic here
>>
>> This macro is used in only two places, and has two almost-independent
>> complex logics coded into it.  So, suppress the macro and move the logic
>> into the respective functions.
>
> I have recently introduced that macro because no amount of documentation
> can guarantee that the two functions using these values compute them the
> same way when somebody makes further changes down the road.  That is,
> however, mandatory for these functions to work properly and safely.
>
> I haven't checked if the logic hasn't changed with that patch, but I
> don't think it's any easier to understand than before.
>

I agree with Achim, I think we should retain the macro.

Best,

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info
  2013-04-01  5:42 ` [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info Aaron Ecay
@ 2013-04-03 14:09   ` Eric Schulte
  2013-04-18  7:09     ` Aaron Ecay
  0 siblings, 1 reply; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 14:09 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> * lisp/ob-core.el (org-babel-do-key-sequence-in-edit-buffer),
> (org-babel-expand-noweb-references),
> * lisp/ob-tangle.el (org-babel-tangle):
> Use 'light argument to `org-babel-get-src-block-info'.

I'd like to apply this patch, however tracing the effects of these
changes can be tricky and `org-babel-expand-noweb-references' and
`org-babel-tangle' are both core functions.

Have you run the test suite to confirm that these changes don't break
any existing tests?

Thanks,

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 00/10] babel cleanups
  2013-04-02 22:14 ` [PATCH 00/10] babel cleanups Eric Schulte
@ 2013-04-03 14:13   ` Eric Schulte
  2013-04-03 16:21     ` Bastien
  0 siblings, 1 reply; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 14:13 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Eric Schulte <schulte.eric@gmail.com> writes:

> Aaron Ecay <aaronecay@gmail.com> writes:
>
>> Here are several patches to fix things in and around org-babel.
>> They're each independent of the others (and hopefully all apply
>> cleanly, without depending on other members of the series).  Here's a
>> little summary of each:
>>
>
> Thanks for these patches, many look like obvious wins, some less so.  I
> likely won't have time to review them (or do much of anything) in the
> near term, but at some point I will make time to review them and reply
> here.

I've reviewed these patches and applied most of them.

Let me express a heartfelt THANKS.  It is always good to have more eyes
on code (as these patches indicate), and I'm very happy you're learning
the internals of Org mode's code block support.  Although these patches
are mainly refactoring I'm excited to have your help in bug-fixing and
feature enhancements in the future.

Cheers,

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands
  2013-04-01  5:42 ` [PATCH 03/10] Clean up various org-babel-*-maybe commands Aaron Ecay
  2013-04-02 19:31   ` Achim Gratz
@ 2013-04-03 14:18   ` Bastien
  2013-04-18  7:03     ` Aaron Ecay
  1 sibling, 1 reply; 39+ messages in thread
From: Bastien @ 2013-04-03 14:18 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Hi Aaron,

Aaron Ecay <aaronecay@gmail.com> writes:

> * lisp/ob-core.el (org-babel-if-in-src-block): New macro
> (org-babel-execute-src-block-maybe),
> (org-babel-expand-src-block-maybe),
> (org-babel-load-in-session-maybe),
> (org-babel-pop-to-session-maybe): Use it

A slightly enhanced version:

* lisp/ob-core.el (org-babel-if-in-src-block): New macro.
(org-babel-execute-src-block-maybe)
(org-babel-expand-src-block-maybe)
(org-babel-load-in-session-maybe, org-babel-pop-to-session-maybe):
Use it.

In a nutshell:

1. No commas outside parentheses;
2. A full-stop at the end of sentences...

C-x 4 a and M-q should be all what you need.

> org-babel-get-src-block-info is a potentially expensive operation, which
> is why its ‘light’ argument exists.  But in any case, it is overkill to
> query the whole info, if all that is needed is whether point is in a
> block or not.  Factor the simplified common code out into a macro.

The let-bound info variable is not only used to check if we are within
a src block, it is also passed as an argument to functions, see the ^^
marks below.

> ---
>  lisp/ob-core.el | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
> index 723aa9d..283628e 100644
> --- a/lisp/ob-core.el
> +++ b/lisp/ob-core.el
> @@ -365,15 +365,22 @@ of potentially harmful code."
>    (or (org-babel-execute-src-block-maybe)
>        (org-babel-lob-execute-maybe)))
>  
> +(defmacro org-babel-when-in-src-block (&rest body)
> +  `(if (or (org-babel-where-is-src-block-head)
> +           (org-babel-get-inline-src-block-matches))
> +       (progn
> +	 ,@body
> +	 t)
> +     nil))

(Please always add a docstring of defuns and defmacros)

>  (defun org-babel-execute-src-block-maybe ()
>    "Conditionally execute a source block.
>  Detect if this is context for a Babel src-block and if so
>  then run `org-babel-execute-src-block'."
>    (interactive)
> -  (let ((info (org-babel-get-src-block-info)))
> -    (if info
> -	(progn (org-babel-eval-wipe-error-buffer)
> -	       (org-babel-execute-src-block current-prefix-arg info) t) nil)))
                                                               ^^^^
> +  (org-babel-when-in-src-block
> +   (org-babel-eval-wipe-error-buffer)
> +   (org-babel-execute-src-block current-prefix-arg)))
>  
>  ;;;###autoload
>  (defun org-babel-view-src-block-info ()
> @@ -409,10 +416,8 @@ a window into the `org-babel-get-src-block-info' function."
>  Detect if this is context for a org-babel src-block and if so
>  then run `org-babel-expand-src-block'."
>    (interactive)
> -  (let ((info (org-babel-get-src-block-info)))
> -    (if info
> -	(progn (org-babel-expand-src-block current-prefix-arg info) t)
                                                              ^^^^
> -      nil)))
> +  (org-babel-when-in-src-block
> +   (org-babel-expand-src-block current-prefix-arg)))
>  
>  ;;;###autoload
>  (defun org-babel-load-in-session-maybe ()
> @@ -420,10 +425,8 @@ then run `org-babel-expand-src-block'."
>  Detect if this is context for a org-babel src-block and if so
>  then run `org-babel-load-in-session'."
>    (interactive)
> -  (let ((info (org-babel-get-src-block-info)))
> -    (if info
> -	(progn (org-babel-load-in-session current-prefix-arg info) t)
                                                             ^^^^
> -      nil)))
> +  (org-babel-when-in-src-block
> +   (org-babel-load-in-session current-prefix-arg)))
>  
>  (add-hook 'org-metaup-hook 'org-babel-load-in-session-maybe)
>  
> @@ -433,8 +436,8 @@ then run `org-babel-load-in-session'."
>  Detect if this is context for a org-babel src-block and if so
>  then run `org-babel-pop-to-session'."
>    (interactive)
> -  (let ((info (org-babel-get-src-block-info)))
> -    (if info (progn (org-babel-pop-to-session current-prefix-arg info) t) nil)))
                                                                    ^^^^
> +  (org-babel-when-in-src-block
> +   (org-babel-pop-to-session current-prefix-arg)))

(Let's use the current name `org-babel-switch-to-session' instead of
the obsolete alias.)

Maybe we don't always need to pass the info as an argument, but at
least for this last example it is needed.  

Thanks,

-- 
 Bastien

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

* Re: [PATCH 00/10] babel cleanups
  2013-04-03 14:13   ` Eric Schulte
@ 2013-04-03 16:21     ` Bastien
  0 siblings, 0 replies; 39+ messages in thread
From: Bastien @ 2013-04-03 16:21 UTC (permalink / raw)
  To: Eric Schulte; +Cc: Aaron Ecay, emacs-orgmode

Eric Schulte <schulte.eric@gmail.com> writes:

> I've reviewed these patches and applied most of them.
>
> Let me express a heartfelt THANKS.  It is always good to have more eyes
> on code (as these patches indicate), and I'm very happy you're learning
> the internals of Org mode's code block support.  Although these patches
> are mainly refactoring I'm excited to have your help in bug-fixing and
> feature enhancements in the future.

Yes, thanks a lot to both of you!

-- 
 Bastien

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

* Re: [PATCH 06/10] Use prefix arg in org-edit-special
  2013-04-03 13:42   ` Eric Schulte
@ 2013-04-03 17:02     ` Bastien
  0 siblings, 0 replies; 39+ messages in thread
From: Bastien @ 2013-04-03 17:02 UTC (permalink / raw)
  To: Eric Schulte; +Cc: Aaron Ecay, emacs-orgmode

Eric Schulte <schulte.eric@gmail.com> writes:

> Aaron Ecay <aaronecay@gmail.com> writes:
>
>> * lisp/org.el (org-edit-special): Use prefix arg, as docstring says we
>>   do
>>
>
> This is beyond my ken.  I'll leave review of this patch to Bastien.

Applied, thanks.

-- 
 Bastien

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

* Re: [PATCH 07/10] Simplify org-babel-execute-src-block
  2013-04-03 13:54     ` Eric Schulte
@ 2013-04-03 17:05       ` Achim Gratz
  2013-04-03 17:20         ` Eric Schulte
  0 siblings, 1 reply; 39+ messages in thread
From: Achim Gratz @ 2013-04-03 17:05 UTC (permalink / raw)
  To: emacs-orgmode

Eric Schulte writes:
> Can you check if this change causes any of the existing tests to fail?

I don't think there is a test for that, at least I don't remember
anything in that direction.  However when implementing my earlier change
w.r.t. confirmation I noticed that merging the parameters early has
potential for triggering execution of source blocks that would otherwise
lay dormant until the execution of the current block was already
confirmed.  As I said, I have no idea if this behaviour is intended, but
that was reason enough for me not to try to "optimize" this away.  The
behaviour Aaron tries to implement is maybe more sane, but it does alter
some corner cases and I can't tell how practically relevant this is.
But if we want to change it then I agree that the time is now.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

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

* Re: [PATCH 07/10] Simplify org-babel-execute-src-block
  2013-04-03 17:05       ` Achim Gratz
@ 2013-04-03 17:20         ` Eric Schulte
  0 siblings, 0 replies; 39+ messages in thread
From: Eric Schulte @ 2013-04-03 17:20 UTC (permalink / raw)
  To: Achim Gratz; +Cc: emacs-orgmode

Achim Gratz <Stromeko@nexgo.de> writes:

> Eric Schulte writes:
>> Can you check if this change causes any of the existing tests to fail?
>
> I don't think there is a test for that, at least I don't remember
> anything in that direction.  However when implementing my earlier change
> w.r.t. confirmation I noticed that merging the parameters early has
> potential for triggering execution of source blocks that would otherwise
> lay dormant until the execution of the current block was already
> confirmed.  As I said, I have no idea if this behaviour is intended, but
> that was reason enough for me not to try to "optimize" this away.  The
> behaviour Aaron tries to implement is maybe more sane, but it does alter
> some corner cases and I can't tell how practically relevant this is.
> But if we want to change it then I agree that the time is now.
>

I'm happy with the current implementation, even if it is a couple of
lines longer, it has the benefit of having been used in production for a
time and proven itself (sufficiently) bug free.

So lets discard this patch and stick with the current for now.

Thanks,

>
>
> Regards,
> Achim.

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands
  2013-04-03 14:18   ` Bastien
@ 2013-04-18  7:03     ` Aaron Ecay
  2013-04-18  8:06       ` [PATCH 1/4] " Aaron Ecay
  2013-04-20 10:10       ` [PATCH 03/10] " Eric Schulte
  0 siblings, 2 replies; 39+ messages in thread
From: Aaron Ecay @ 2013-04-18  7:03 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode

Hi Bastien,

Thanks for your comments.

2013ko apirilak 3an, Bastien-ek idatzi zuen:

[...]

>> org-babel-get-src-block-info is a potentially expensive operation, which
>> is why its ‘light’ argument exists.  But in any case, it is overkill to
>> query the whole info, if all that is needed is whether point is in a
>> block or not.  Factor the simplified common code out into a macro.
> 
> The let-bound info variable is not only used to check if we are within
> a src block, it is also passed as an argument to functions, see the ^^
> marks below.

All of these functions will re-calculate the info if it is not
passed, using org-babel-get-src-block-info.  So not passing it does no
harm.

> 
>> ---
>> lisp/ob-core.el | 31 +++++++++++++++++--------------
>> 1 file changed, 17 insertions(+), 14 deletions(-)
>> 
>> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
>> index 723aa9d..283628e 100644
>> --- a/lisp/ob-core.el
>> +++ b/lisp/ob-core.el
>> @@ -365,15 +365,22 @@ of potentially harmful code."
>> (or (org-babel-execute-src-block-maybe)
>> (org-babel-lob-execute-maybe)))
>> 
>> +(defmacro org-babel-when-in-src-block (&rest body)
>> +  `(if (or (org-babel-where-is-src-block-head)
>> +           (org-babel-get-inline-src-block-matches))
>> +       (progn
>> +	 ,@body
>> +	 t)
>> +     nil))
> 
> (Please always add a docstring of defuns and defmacros)

I’ll resend the patch with a docstring and fixing the commit message
problems you and Achim pointed out.


[...]


>> @@ -433,8 +436,8 @@ then run `org-babel-load-in-session'."
>> Detect if this is context for a org-babel src-block and if so
>> then run `org-babel-pop-to-session'."
>> (interactive)
>> -  (let ((info (org-babel-get-src-block-info)))
>> -    (if info (progn (org-babel-pop-to-session current-prefix-arg info) t) nil)))
>                                                                     ^^^^
>> +  (org-babel-when-in-src-block
>> +   (org-babel-pop-to-session current-prefix-arg)))
> 
> (Let's use the current name `org-babel-switch-to-session' instead of
> the obsolete alias.)

OK.

> 
> Maybe we don't always need to pass the info as an argument, but at
> least for this last example it is needed.

o-b-switch-to-session does nothing with the info argument but pass it
along to o-b-initiate-session, which will recalculate it if it is
missing.  So it takes 2 hops in contrast to the 1 in the other cases,
but it still gets recalculated.

-- 
Aaron Ecay

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

* Re: [PATCH 05/10] Remove info arg from several org-babel functions
  2013-04-03 13:58   ` Eric Schulte
@ 2013-04-18  7:07     ` Aaron Ecay
  0 siblings, 0 replies; 39+ messages in thread
From: Aaron Ecay @ 2013-04-18  7:07 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

Hi Eric,

Thanks for your comments on this and all the patches.

2013ko apirilak 3an, Eric Schulte-ek idatzi zuen:
> 
> Aaron Ecay <aaronecay@gmail.com> writes:
> 
>> * lisp/ob-core.el (org-babel-load-in-session),
>> (org-babel-initiate-session),
>> (org-babel-switch-to-session)
>> (org-babel-switch-to-session-with-code): Remove info optional arg
>> 
>> The info arg is threaded through this code, but never used by
>> callers (at least in org code).
> 
> The rgrep command disagrees with this last statement.
> 
> ./ob-core.el:411:	(progn (org-babel-load-in-session current-prefix-arg info) t)

This was removed in patch #3 of the series.  I tried to make all the
patches independent of each other, but this dependency did slip in.  I
can squash the two patches if you’d prefer.

-- 
Aaron Ecay

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

* Re: [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info
  2013-04-03 14:09   ` Eric Schulte
@ 2013-04-18  7:09     ` Aaron Ecay
  2013-04-20 10:09       ` Eric Schulte
  0 siblings, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-18  7:09 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

Hi Eric,

2013ko apirilak 3an, Eric Schulte-ek idatzi zuen:
> 
> Aaron Ecay <aaronecay@gmail.com> writes:
> 
>> * lisp/ob-core.el (org-babel-do-key-sequence-in-edit-buffer),
>> (org-babel-expand-noweb-references),
>> * lisp/ob-tangle.el (org-babel-tangle):
>> Use 'light argument to `org-babel-get-src-block-info'.
> 
> I'd like to apply this patch, however tracing the effects of these
> changes can be tricky and `org-babel-expand-noweb-references' and
> `org-babel-tangle' are both core functions.
> 
> Have you run the test suite to confirm that these changes don't break
> any existing tests?

The test suite gives no failures with this patch.

-- 
Aaron Ecay

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

* [PATCH 1/4] Clean up various org-babel-*-maybe commands
  2013-04-18  7:03     ` Aaron Ecay
@ 2013-04-18  8:06       ` Aaron Ecay
  2013-04-18 10:28         ` Bastien
  2013-04-20 10:10       ` [PATCH 03/10] " Eric Schulte
  1 sibling, 1 reply; 39+ messages in thread
From: Aaron Ecay @ 2013-04-18  8:06 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-core.el (org-babel-when-in-src-block): New macro.
  (org-babel-execute-src-block-maybe)
  (org-babel-expand-src-block-maybe)
  (org-babel-load-in-session-maybe, org-babel-pop-to-session-maybe):
  Use it.

org-babel-get-src-block-info is a potentially expensive operation, which
is why its ‘light’ argument exists.  But in any case, it is overkill to
query the whole info, if all that is needed is whether point is in a
block or not.  Factor the simplified common code out into a macro.
---
 lisp/ob-core.el | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index b8d93f2..e44fc02 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -351,15 +351,25 @@ of potentially harmful code."
   (or (org-babel-execute-src-block-maybe)
       (org-babel-lob-execute-maybe)))
 
+(defmacro org-babel-when-in-src-block (&rest body)
+  "Execute BODY if point is in a source block and return t.
+
+Otherwise do nothing and return nil."
+  `(if (or (org-babel-where-is-src-block-head)
+           (org-babel-get-inline-src-block-matches))
+       (progn
+	 ,@body
+	 t)
+     nil))
+
 (defun org-babel-execute-src-block-maybe ()
   "Conditionally execute a source block.
 Detect if this is context for a Babel src-block and if so
 then run `org-babel-execute-src-block'."
   (interactive)
-  (let ((info (org-babel-get-src-block-info)))
-    (if info
-	(progn (org-babel-eval-wipe-error-buffer)
-	       (org-babel-execute-src-block current-prefix-arg info) t) nil)))
+  (org-babel-when-in-src-block
+   (org-babel-eval-wipe-error-buffer)
+   (org-babel-execute-src-block current-prefix-arg)))
 
 ;;;###autoload
 (defun org-babel-view-src-block-info ()
@@ -395,10 +405,8 @@ a window into the `org-babel-get-src-block-info' function."
 Detect if this is context for a org-babel src-block and if so
 then run `org-babel-expand-src-block'."
   (interactive)
-  (let ((info (org-babel-get-src-block-info)))
-    (if info
-	(progn (org-babel-expand-src-block current-prefix-arg info) t)
-      nil)))
+  (org-babel-when-in-src-block
+   (org-babel-expand-src-block current-prefix-arg)))
 
 ;;;###autoload
 (defun org-babel-load-in-session-maybe ()
@@ -406,10 +414,8 @@ then run `org-babel-expand-src-block'."
 Detect if this is context for a org-babel src-block and if so
 then run `org-babel-load-in-session'."
   (interactive)
-  (let ((info (org-babel-get-src-block-info)))
-    (if info
-	(progn (org-babel-load-in-session current-prefix-arg info) t)
-      nil)))
+  (org-babel-when-in-src-block
+   (org-babel-load-in-session current-prefix-arg)))
 
 (add-hook 'org-metaup-hook 'org-babel-load-in-session-maybe)
 
@@ -419,8 +425,8 @@ then run `org-babel-load-in-session'."
 Detect if this is context for a org-babel src-block and if so
 then run `org-babel-pop-to-session'."
   (interactive)
-  (let ((info (org-babel-get-src-block-info)))
-    (if info (progn (org-babel-pop-to-session current-prefix-arg info) t) nil)))
+  (org-babel-when-in-src-block
+   (org-babel-switch-to-session current-prefix-arg)))
 
 (add-hook 'org-metadown-hook 'org-babel-pop-to-session-maybe)
 
-- 
1.8.2.1

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

* Re: [PATCH 1/4] Clean up various org-babel-*-maybe commands
  2013-04-18  8:06       ` [PATCH 1/4] " Aaron Ecay
@ 2013-04-18 10:28         ` Bastien
  0 siblings, 0 replies; 39+ messages in thread
From: Bastien @ 2013-04-18 10:28 UTC (permalink / raw)
  To: Aaron Ecay; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> * lisp/ob-core.el (org-babel-when-in-src-block): New macro.
>   (org-babel-execute-src-block-maybe)
>   (org-babel-expand-src-block-maybe)
>   (org-babel-load-in-session-maybe, org-babel-pop-to-session-maybe):
>   Use it.

Applied, thanks.

-- 
 Bastien

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

* Re: [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info
  2013-04-18  7:09     ` Aaron Ecay
@ 2013-04-20 10:09       ` Eric Schulte
  2013-04-22  3:51         ` Aaron Ecay
  0 siblings, 1 reply; 39+ messages in thread
From: Eric Schulte @ 2013-04-20 10:09 UTC (permalink / raw)
  To: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> Hi Eric,
>
> 2013ko apirilak 3an, Eric Schulte-ek idatzi zuen:
>> 
>> Aaron Ecay <aaronecay@gmail.com> writes:
>> 
>>> * lisp/ob-core.el (org-babel-do-key-sequence-in-edit-buffer),
>>> (org-babel-expand-noweb-references),
>>> * lisp/ob-tangle.el (org-babel-tangle):
>>> Use 'light argument to `org-babel-get-src-block-info'.
>> 
>> I'd like to apply this patch, however tracing the effects of these
>> changes can be tricky and `org-babel-expand-noweb-references' and
>> `org-babel-tangle' are both core functions.
>> 
>> Have you run the test suite to confirm that these changes don't break
>> any existing tests?
>
> The test suite gives no failures with this patch.

Then please go ahead and apply this patch (or re-send it to me and I can
apply it).

Cheers,

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands
  2013-04-18  7:03     ` Aaron Ecay
  2013-04-18  8:06       ` [PATCH 1/4] " Aaron Ecay
@ 2013-04-20 10:10       ` Eric Schulte
  2013-04-22  3:52         ` Aaron Ecay
  1 sibling, 1 reply; 39+ messages in thread
From: Eric Schulte @ 2013-04-20 10:10 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> Hi Bastien,
>
> Thanks for your comments.
>
> 2013ko apirilak 3an, Bastien-ek idatzi zuen:
>
> [...]
>
>>> org-babel-get-src-block-info is a potentially expensive operation, which
>>> is why its ‘light’ argument exists.  But in any case, it is overkill to
>>> query the whole info, if all that is needed is whether point is in a
>>> block or not.  Factor the simplified common code out into a macro.
>> 
>> The let-bound info variable is not only used to check if we are within
>> a src block, it is also passed as an argument to functions, see the ^^
>> marks below.
>
> All of these functions will re-calculate the info if it is not
> passed, using org-babel-get-src-block-info.  So not passing it does no
> harm.
>

Could re-calculating the info cause referenced blocks to be executed
more than once?

If so then we should continue passing the info and *not* simply
re-calculate it later on.

Cheers,

>
>> 
>>> ---
>>> lisp/ob-core.el | 31 +++++++++++++++++--------------
>>> 1 file changed, 17 insertions(+), 14 deletions(-)
>>> 
>>> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
>>> index 723aa9d..283628e 100644
>>> --- a/lisp/ob-core.el
>>> +++ b/lisp/ob-core.el
>>> @@ -365,15 +365,22 @@ of potentially harmful code."
>>> (or (org-babel-execute-src-block-maybe)
>>> (org-babel-lob-execute-maybe)))
>>> 
>>> +(defmacro org-babel-when-in-src-block (&rest body)
>>> +  `(if (or (org-babel-where-is-src-block-head)
>>> +           (org-babel-get-inline-src-block-matches))
>>> +       (progn
>>> +	 ,@body
>>> +	 t)
>>> +     nil))
>> 
>> (Please always add a docstring of defuns and defmacros)
>
> I’ll resend the patch with a docstring and fixing the commit message
> problems you and Achim pointed out.
>
>
> [...]
>
>
>>> @@ -433,8 +436,8 @@ then run `org-babel-load-in-session'."
>>> Detect if this is context for a org-babel src-block and if so
>>> then run `org-babel-pop-to-session'."
>>> (interactive)
>>> -  (let ((info (org-babel-get-src-block-info)))
>>> -    (if info (progn (org-babel-pop-to-session current-prefix-arg info) t) nil)))
>>                                                                     ^^^^
>>> +  (org-babel-when-in-src-block
>>> +   (org-babel-pop-to-session current-prefix-arg)))
>> 
>> (Let's use the current name `org-babel-switch-to-session' instead of
>> the obsolete alias.)
>
> OK.
>
>> 
>> Maybe we don't always need to pass the info as an argument, but at
>> least for this last example it is needed.
>
> o-b-switch-to-session does nothing with the info argument but pass it
> along to o-b-initiate-session, which will recalculate it if it is
> missing.  So it takes 2 hops in contrast to the 1 in the other cases,
> but it still gets recalculated.

-- 
Eric Schulte
http://cs.unm.edu/~eschulte

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

* Re: [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info
  2013-04-20 10:09       ` Eric Schulte
@ 2013-04-22  3:51         ` Aaron Ecay
  0 siblings, 0 replies; 39+ messages in thread
From: Aaron Ecay @ 2013-04-22  3:51 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

Hi Eric,

2013ko apirilak 20an, Eric Schulte-ek idatzi zuen:
> 
> Then please go ahead and apply this patch (or re-send it to me and I can
> apply it).

Done.  :)

-- 
Aaron Ecay

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

* Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands
  2013-04-20 10:10       ` [PATCH 03/10] " Eric Schulte
@ 2013-04-22  3:52         ` Aaron Ecay
  0 siblings, 0 replies; 39+ messages in thread
From: Aaron Ecay @ 2013-04-22  3:52 UTC (permalink / raw)
  To: Eric Schulte; +Cc: Bastien, emacs-orgmode

Hi Eric,

2013ko apirilak 20an, Eric Schulte-ek idatzi zuen:
> Could re-calculating the info cause referenced blocks to be executed
> more than once?
> 
> If so then we should continue passing the info and *not* simply
> re-calculate it later on.

This is a very good question.  I will look into it and send an update
when I have found out the answer.

Thanks,

-- 
Aaron Ecay

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

end of thread, other threads:[~2013-04-22  3:52 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-01  5:42 [PATCH 00/10] babel cleanups Aaron Ecay
2013-04-01  5:42 ` [PATCH 01/10] Fix org-babel-R-initiate-session Aaron Ecay
2013-04-03 13:46   ` Eric Schulte
2013-04-01  5:42 ` [PATCH 02/10] Clean up org-babel-expand-body: functions for awk and picolisp Aaron Ecay
2013-04-03 13:56   ` Eric Schulte
2013-04-01  5:42 ` [PATCH 03/10] Clean up various org-babel-*-maybe commands Aaron Ecay
2013-04-02 19:31   ` Achim Gratz
2013-04-03 14:18   ` Bastien
2013-04-18  7:03     ` Aaron Ecay
2013-04-18  8:06       ` [PATCH 1/4] " Aaron Ecay
2013-04-18 10:28         ` Bastien
2013-04-20 10:10       ` [PATCH 03/10] " Eric Schulte
2013-04-22  3:52         ` Aaron Ecay
2013-04-01  5:42 ` [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info Aaron Ecay
2013-04-03 14:09   ` Eric Schulte
2013-04-18  7:09     ` Aaron Ecay
2013-04-20 10:09       ` Eric Schulte
2013-04-22  3:51         ` Aaron Ecay
2013-04-01  5:42 ` [PATCH 05/10] Remove info arg from several org-babel functions Aaron Ecay
2013-04-03 13:58   ` Eric Schulte
2013-04-18  7:07     ` Aaron Ecay
2013-04-01  5:42 ` [PATCH 06/10] Use prefix arg in org-edit-special Aaron Ecay
2013-04-03 13:42   ` Eric Schulte
2013-04-03 17:02     ` Bastien
2013-04-01  5:42 ` [PATCH 07/10] Simplify org-babel-execute-src-block Aaron Ecay
2013-04-02 19:41   ` Achim Gratz
2013-04-03 13:54     ` Eric Schulte
2013-04-03 17:05       ` Achim Gratz
2013-04-03 17:20         ` Eric Schulte
2013-04-01  5:42 ` [PATCH 08/10] Fix testing/lisp/test-ob-emacs-lisp.el Aaron Ecay
2013-04-03 13:47   ` Eric Schulte
2013-04-01  5:42 ` [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro Aaron Ecay
2013-04-02 19:53   ` Achim Gratz
2013-04-03 14:05     ` Eric Schulte
2013-04-01  5:42 ` [PATCH 10/10] Document how :var introduces code block dependencies Aaron Ecay
2013-04-03 14:04   ` Eric Schulte
2013-04-02 22:14 ` [PATCH 00/10] babel cleanups Eric Schulte
2013-04-03 14:13   ` Eric Schulte
2013-04-03 16:21     ` Bastien

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