[PATCH 00/10] babel cleanups

[PATCH 00/10] babel cleanups

[Aaron Ecay]

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

[PATCH 01/10] Fix org-babel-R-initiate-session

[Aaron Ecay]

* 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

[PATCH 02/10] Clean up org-babel-expand-body: functions for awk and picolisp

[Aaron Ecay]

* 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

[PATCH 03/10] Clean up various org-babel-*-maybe commands

[Aaron Ecay]

* 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

[PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info

[Aaron Ecay]

* 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

[PATCH 05/10] Remove info arg from several org-babel functions

[Aaron Ecay]

* 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

[PATCH 06/10] Use prefix arg in org-edit-special

[Aaron Ecay]

* 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

[PATCH 07/10] Simplify org-babel-execute-src-block

[Aaron Ecay]

* 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

[PATCH 08/10] Fix testing/lisp/test-ob-emacs-lisp.el

[Aaron Ecay]

* 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

[PATCH 09/10] Remove org-babel-check-confirm-evaluate macro

[Aaron Ecay]

* 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

[PATCH 10/10] Document how :var introduces code block dependencies.

[Aaron Ecay]

* 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

Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands

[Achim Gratz]

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

Re: [PATCH 07/10] Simplify org-babel-execute-src-block

[Achim Gratz]

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

Re: [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro

[Achim Gratz]

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

Re: [PATCH 00/10] babel cleanups

[Eric Schulte]

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

Re: [PATCH 06/10] Use prefix arg in org-edit-special

[Eric Schulte]

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

Re: [PATCH 01/10] Fix org-babel-R-initiate-session

[Eric Schulte]

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

Re: [PATCH 08/10] Fix testing/lisp/test-ob-emacs-lisp.el

[Eric Schulte]

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

Re: [PATCH 07/10] Simplify org-babel-execute-src-block

[Eric Schulte]

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

Re: [PATCH 02/10] Clean up org-babel-expand-body: functions for awk and picolisp

[Eric Schulte]

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

Re: [PATCH 05/10] Remove info arg from several org-babel functions

[Eric Schulte]

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

Re: [PATCH 10/10] Document how :var introduces code block dependencies.

[Eric Schulte]

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

Re: [PATCH 09/10] Remove org-babel-check-confirm-evaluate macro

[Eric Schulte]

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

Re: [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info

[Eric Schulte]

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

Re: [PATCH 00/10] babel cleanups

[Eric Schulte]

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

Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands

[Bastien]

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

Re: [PATCH 00/10] babel cleanups

[Bastien]

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

Re: [PATCH 06/10] Use prefix arg in org-edit-special

[Bastien]

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

Re: [PATCH 07/10] Simplify org-babel-execute-src-block

[Achim Gratz]

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

Re: [PATCH 07/10] Simplify org-babel-execute-src-block

[Eric Schulte]

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

Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands

[Aaron Ecay]

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

Re: [PATCH 05/10] Remove info arg from several org-babel functions

[Aaron Ecay]

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

Re: [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info

[Aaron Ecay]

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

[PATCH 1/4] Clean up various org-babel-*-maybe commands

[Aaron Ecay]

* 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

Re: [PATCH 1/4] Clean up various org-babel-*-maybe commands

[Bastien]

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

Re: [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info

[Eric Schulte]

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

Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands

[Eric Schulte]

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

Re: [PATCH 04/10] Add 'light argument to some uses of org-babel-get-src-block-info

[Aaron Ecay]

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

Re: [PATCH 03/10] Clean up various org-babel-*-maybe commands

[Aaron Ecay]

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