[PATCH 1/2] lisp/ob.el: Fix org-babel-result-regexp to match users

[PATCH 1/2] lisp/ob.el: Fix org-babel-result-regexp to match users

[Lawrence Mitchell]

* lisp/ob.el (org-babel-result-regexp): Use non-shy group around
org-babel-data-names.

By default regexp-opt returns a shy group around its arguments.  But
users of org-babel-result-regexp expect the third match-string to
contain the hash.  With a shy group, the second match-string contains
the hash.
---
 lisp/ob.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lisp/ob.el b/lisp/ob.el
index 27f005c..e1f4372 100644
--- a/lisp/ob.el
+++ b/lisp/ob.el
@@ -310,7 +310,7 @@ specific header arguments as well.")
 
 (defvar org-babel-result-regexp
   (concat "^[ \t]*#\\+"
-	  (regexp-opt org-babel-data-names)
+	  (regexp-opt org-babel-data-names t)
 	  "\\(\\[\\([[:alnum:]]+\\)\\]\\)?\\:[ \t]*")
   "Regular expression used to match result lines.
 If the results are associated with a hash key then the hash will
-- 
1.7.4.1

[PATCH 2/2] lisp/ob.el: Don't modify babel info when hashing it

[Lawrence Mitchell]

* lisp/ob.el (org-babel-sha1-hash): Don't modify info argument by
side-effect when sorting result-params list.

copy-sequence only does shallow copies, so if we're going to modify a
sub-list, we need to make sure we copy it first.
---
 lisp/ob.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lisp/ob.el b/lisp/ob.el
index e1f4372..36649f0 100644
--- a/lisp/ob.el
+++ b/lisp/ob.el
@@ -767,7 +767,7 @@ the current subtree."
 				  (cond
 				   ((and (listp v) ; lists are sorted
 					 (member (car arg) '(:result-params)))
-				    (sort v #'string<))
+				    (sort (copy-sequence v) #'string<))
 				   ((and (stringp v) ; strings are sorted
 					 (member (car arg) '(:results :exports)))
 				    (mapconcat #'identity (sort (split-string v)
-- 
1.7.4.1

Re: [PATCH 2/2] lisp/ob.el: Don't modify babel info when hashing it

[Eric Schulte]

Hi Lawrence,

Is there a reason to make this copy?  Given that params is used like a
hash/dictionary the order of it's elements should not matter.  Is there
a case where this patch is necessary to avoid buggy behavior?

If so then I'm happy to apply the patch, but if there is no need then
I'd rather avoid the (admittedly small) overhead of making a copy of the
params list.

Thanks -- Eric

Lawrence Mitchell <wence@gmx.li> writes:

> * lisp/ob.el (org-babel-sha1-hash): Don't modify info argument by
> side-effect when sorting result-params list.
>
> copy-sequence only does shallow copies, so if we're going to modify a
> sub-list, we need to make sure we copy it first.
> ---
>  lisp/ob.el |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lisp/ob.el b/lisp/ob.el
> index e1f4372..36649f0 100644
> --- a/lisp/ob.el
> +++ b/lisp/ob.el
> @@ -767,7 +767,7 @@ the current subtree."
>  				  (cond
>  				   ((and (listp v) ; lists are sorted
>  					 (member (car arg) '(:result-params)))
> -				    (sort v #'string<))
> +				    (sort (copy-sequence v) #'string<))
>  				   ((and (stringp v) ; strings are sorted
>  					 (member (car arg) '(:results :exports)))
>  				    (mapconcat #'identity (sort (split-string v)

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

Re: [PATCH 2/2] lisp/ob.el: Don't modify babel info when hashing it

[Lawrence Mitchell]

Eric Schulte wrote:
> Hi Lawrence,

> Is there a reason to make this copy?  Given that params is used like a
> hash/dictionary the order of it's elements should not matter.  Is there
> a case where this patch is necessary to avoid buggy behavior?

Ah, but the sorting is happening so that the hashing function is
stable to changes in the order of the arguments, no?  So I think
sorting is necessary.  At least, the sort was already there, I've
just added the copy-sequence.

> If so then I'm happy to apply the patch, but if there is no need then
> I'd rather avoid the (admittedly small) overhead of making a copy of the
> params list.

[...]

It seems to be necessary to correctly link to the produced
graphics output from (say) R plotting if :cache yes is specified.
Try this without the patch:

* header
#+begin_src R :cache yes :exports results :file plot.png :results graphics
  curve(1*x, from=1, to=10, lwd=2)
#+end_src

#+results:

and evaluate the code block, we get:

#+results[4d3e7ef5e40f7b608d400c310401c15e913a22c0]:
: plot.png

Rather than the (correct):
#+results[4d3e7ef5e40f7b608d400c310401c15e913a22c0]:
[[file:plot.png]]

The problem is that (sort v 'string<) destructively modifies v,
and in this case, that means that the "file" argument to
:result-params in thrown away.

Lawrence
-- 
Lawrence Mitchell <wence@gmx.li>

Re: [PATCH 1/2] lisp/ob.el: Fix org-babel-result-regexp to match users

[Eric Schulte]

Applied, Thanks -- Eric

Lawrence Mitchell <wence@gmx.li> writes:

> * lisp/ob.el (org-babel-result-regexp): Use non-shy group around
> org-babel-data-names.
>
> By default regexp-opt returns a shy group around its arguments.  But
> users of org-babel-result-regexp expect the third match-string to
> contain the hash.  With a shy group, the second match-string contains
> the hash.
> ---
>  lisp/ob.el |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lisp/ob.el b/lisp/ob.el
> index 27f005c..e1f4372 100644
> --- a/lisp/ob.el
> +++ b/lisp/ob.el
> @@ -310,7 +310,7 @@ specific header arguments as well.")
>  
>  (defvar org-babel-result-regexp
>    (concat "^[ \t]*#\\+"
> -	  (regexp-opt org-babel-data-names)
> +	  (regexp-opt org-babel-data-names t)
>  	  "\\(\\[\\([[:alnum:]]+\\)\\]\\)?\\:[ \t]*")
>    "Regular expression used to match result lines.
>  If the results are associated with a hash key then the hash will

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