Re: [O] [PATCH 2/2] lisp/ob.el: Don't modify babel info when hashing it
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 we...@gmx.li
[O] [PATCH 2/2] lisp/ob.el: Don't modify babel info when hashing it
* 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: [O] [PATCH 2/2] lisp/ob.el: Don't modify babel info when hashing it
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 we...@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/
