[O] [patch][babel] `org-babel-result-end' bug fix and regression tests
`org-babel-result-end' bug fix and `org-babel-remove-result' regression tests. * lisp/ob.el: The code block below will currently act as though :results prepend is set. This is due to `org-babel-result-end' being unable to find the correct end of a raw result. This patch fixes that. #+begin_src emacs-lisp :results raw a line #+end_src #+results: a line a line * testing/lisp/test-ob.el: Several regression tests that test the correct (multiple) execution of code blocks in the various results formats. The tests also test that 'org-babel-remove-result' correctly removes the result. Best, Martyn From 5a3148fb1e3de288e5e3534ceb06eb64c20697aa Mon Sep 17 00:00:00 2001 From: Martyn Jago martyn.j...@btinternet.com Date: Fri, 6 Jan 2012 17:10:00 + Subject: [PATCH] `org-babel-result-end' bug fix and `org-babel-remove-result' regression tests. * lisp/ob.el: The code block below will currently act as though :results prepend is set. This is due to `org-babel-result-end' being unable to find the correct end of a raw result. This patch fixes that. a line * testing/lisp/test-ob.el: Several regression tests that test the correct (multiple) execution of code blocks in the various results formats. The tests also test that 'org-babel-remove-result' correctly removes the result. --- lisp/ob.el |2 +- testing/lisp/test-ob.el | 218 +-- 2 files changed, 212 insertions(+), 8 deletions(-) diff --git a/lisp/ob.el b/lisp/ob.el index 0288eb3..26792ea 100644 --- a/lisp/ob.el +++ b/lisp/ob.el @@ -1810,7 +1810,7 @@ code the results are extracted in the syntax of the source (if (looking-at (concat [ \t]*#\\+begin_ blocks-re)) (progn (re-search-forward (concat [ \t]*#\\+end_ blocks-re) nil t) (forward-char 1)) - (while (looking-at [ \t]*\\(: \\|\\[\\[\\)) + (while (not (looking-at ^\s*$)) (forward-line 1 (point) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index dac6866..f616776 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -137,13 +137,7 @@ #+name: i-have-a-name #+begin_src emacs-lisp 42 -#+end_src - -#+name: -: 42 - -#+name: i-have-a-name -: 42 +#+end_src (progn (org-babel-next-src-block 1) @@ -671,6 +665,216 @@ on two lines (org-babel-balanced-split :a 1 :b [2 3] :c (4 :d (5 6)) '((32 9) . 58) +(ert-deftest test-ob/commented-last-block-line-no-var () + (org-test-with-temp-text-in-file +#+begin_src emacs-lisp +;; +#+end_src +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (should (re-search-forward \\#\\+results: nil t)) + (forward-line) + (should + (string= + + (buffer-substring-no-properties (point-at-bol) (point-at-eol)) + (org-test-with-temp-text-in-file +#+begin_src emacs-lisp +\some text\;; +#+end_src + +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (should (re-search-forward \\#\\+results: nil t)) + (forward-line) + (should + (string= + : some text + (buffer-substring-no-properties (point-at-bol) (point-at-eol))) + +(ert-deftest test-ob/commented-last-block-line-with-var () + (org-test-with-temp-text-in-file +#+begin_src emacs-lisp :var a=1 +;; +#+end_src +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (re-search-forward \\#\\+results: nil t) + (forward-line) + (should (string= + + (buffer-substring-no-properties (point-at-bol) (point-at-eol)) + (org-test-with-temp-text-in-file +#+begin_src emacs-lisp :var a=2 +2;; +#+end_src +(progn + (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))) + +(defun test-ob-verify-result-and-removed-result (result buffer-text) + Test helper function to test `org-babel-remove-result'. +A temp buffer is populated with BUFFER-TEXT, the first block is executed, +and the result of execution is verified against RESULT. + +The block is actually executed /twice/ to ensure result +replacement happens correctly. + (org-test-with-temp-text + buffer-text +(progn + (org-babel-next-src-block) (org-ctrl-c-ctrl-c) (org-ctrl-c-ctrl-c) + (should (re-search-forward \\#\\+results: nil t)) + (forward-line) + (should (string= result + (buffer-substring-no-properties + (point-at-bol) + (- (point-max) 16 + (org-babel-previous-src-block) (org-babel-remove-result) + (should (string= buffer-text + (buffer-substring-no-properties + (point-min) (point-max))) + +(ert-deftest test-ob/org-babel-remove-result--results-default () + Test `org-babel-remove-result' with default :results. + (mapcar (lambda (language) + (test-ob-verify-result-and-removed-result + \n + (concat +*
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Hi Martyn, Unfortunately there is no way to remove raw results because there is no way to know where the results end. While your patch will certainly work most of the time, it will not work in cases where the results includes an empty line, and ultimately I think any attempt to remove raw results will result in confusion. If removable raw results are desired then the :results wrap option may be used. I believe this is mentioned in the manual (if not it should be). I think this patch should not be applied (although maybe some of the test cases could still be useful). Thanks, Martyn Jago martyn.j...@btinternet.com writes: `org-babel-result-end' bug fix and `org-babel-remove-result' regression tests. * lisp/ob.el: The code block below will currently act as though :results prepend is set. This is due to `org-babel-result-end' being unable to find the correct end of a raw result. This patch fixes that. #+begin_src emacs-lisp :results raw a line #+end_src #+results: a line a line * testing/lisp/test-ob.el: Several regression tests that test the correct (multiple) execution of code blocks in the various results formats. The tests also test that 'org-babel-remove-result' correctly removes the result. Best, Martyn -- Eric Schulte http://cs.unm.edu/~eschulte/
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Eric Schulte eric.schu...@gmx.com writes: Hi Eric Hi Martyn, Unfortunately there is no way to remove raw results because there is no way to know where the results end. While your patch will certainly work most of the time, it will not work in cases where the results includes an empty line, and ultimately I think any attempt to remove raw results will result in confusion. Yes I appreciate that would be a problem if there were empty lines. I just thought that most of the time is better than none of the time, but I understand your choice. Certainly wrap works fine. I have noticed one small issue regarding :results wrap, which is that an extra newline is appended to the end of the result each time `org-babel-execute-src-block' is executed. I imagine this can be safely removed? There are a few other minor issues also with `org-babel-remove-results' that probably should be fixed at some time (regarding append and prepend). I think this patch should not be applied (although maybe some of the test cases could still be useful). All the tests supplied with the exception of `test-ob/org-babel-remove-result--results-raw' will still pass without the change. Best, Martyn
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Martyn Jago martyn.j...@btinternet.com writes: Eric Schulte eric.schu...@gmx.com writes: Hi Eric Hi Martyn, Unfortunately there is no way to remove raw results because there is no way to know where the results end. While your patch will certainly work most of the time, it will not work in cases where the results includes an empty line, and ultimately I think any attempt to remove raw results will result in confusion. Yes I appreciate that would be a problem if there were empty lines. I just thought that most of the time is better than none of the time, but I understand your choice. Certainly wrap works fine. I have noticed one small issue regarding :results wrap, which is that an extra newline is appended to the end of the result each time `org-babel-execute-src-block' is executed. I imagine this can be safely removed? Yes, this should be fixed. These newline-append issues are tricky as it can be difficult to fix for one type of result without accidentally breaking for other types of results. There are a few other minor issues also with `org-babel-remove-results' that probably should be fixed at some time (regarding append and prepend). I think this patch should not be applied (although maybe some of the test cases could still be useful). All the tests supplied with the exception of `test-ob/org-babel-remove-result--results-raw' will still pass without the change. Alright, would you be willing to resubmit the patch including only those tests which should still apply? This results handling is certainly an area which will benefit from beefing up the test suite. Thanks, Best, Martyn -- Eric Schulte http://cs.unm.edu/~eschulte/
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Hi Eric Eric Schulte eric.schu...@gmx.com writes: [...] All the tests supplied with the exception of `test-ob/org-babel-remove-result--results-raw' will still pass without the change. Alright, would you be willing to resubmit the patch including only those tests which should still apply? This results handling is certainly an area which will benefit from beefing up the test suite. The attached patch contains the remaining tests. Best, Martyn From 6700e3f350c76f68891ad0ccd35538b5523312d9 Mon Sep 17 00:00:00 2001 From: Martyn Jago martyn.j...@btinternet.com Date: Fri, 6 Jan 2012 19:36:28 + Subject: [PATCH] Regression tests regarding code block results and result removal/replacement. * testing/lisp/test-ob.el: Regression tests regarding code block results and result removal/replacement --- testing/lisp/test-ob.el | 205 +-- 1 files changed, 198 insertions(+), 7 deletions(-) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index dac6866..64e261a 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -137,13 +137,7 @@ #+name: i-have-a-name #+begin_src emacs-lisp 42 -#+end_src - -#+name: -: 42 - -#+name: i-have-a-name -: 42 +#+end_src (progn (org-babel-next-src-block 1) @@ -671,6 +665,203 @@ on two lines (org-babel-balanced-split :a 1 :b [2 3] :c (4 :d (5 6)) '((32 9) . 58) +(ert-deftest test-ob/commented-last-block-line-no-var () + (org-test-with-temp-text-in-file +#+begin_src emacs-lisp +;; +#+end_src +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (should (re-search-forward \\#\\+results: nil t)) + (forward-line) + (should + (string= + + (buffer-substring-no-properties (point-at-bol) (point-at-eol)) + (org-test-with-temp-text-in-file +#+begin_src emacs-lisp +\some text\;; +#+end_src + +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (should (re-search-forward \\#\\+results: nil t)) + (forward-line) + (should + (string= + : some text + (buffer-substring-no-properties (point-at-bol) (point-at-eol))) + +(ert-deftest test-ob/commented-last-block-line-with-var () + (org-test-with-temp-text-in-file +#+begin_src emacs-lisp :var a=1 +;; +#+end_src +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (re-search-forward \\#\\+results: nil t) + (forward-line) + (should (string= + + (buffer-substring-no-properties (point-at-bol) (point-at-eol)) + (org-test-with-temp-text-in-file +#+begin_src emacs-lisp :var a=2 +2;; +#+end_src +(progn + (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))) + +(defun test-ob-verify-result-and-removed-result (result buffer-text) + Test helper function to test `org-babel-remove-result'. +A temp buffer is populated with BUFFER-TEXT, the first block is executed, +and the result of execution is verified against RESULT. + +The block is actually executed /twice/ to ensure result +replacement happens correctly. + (org-test-with-temp-text + buffer-text +(progn + (org-babel-next-src-block) (org-ctrl-c-ctrl-c) (org-ctrl-c-ctrl-c) + (should (re-search-forward \\#\\+results: nil t)) + (forward-line) + (should (string= result + (buffer-substring-no-properties + (point-at-bol) + (- (point-max) 16 + (org-babel-previous-src-block) (org-babel-remove-result) + (should (string= buffer-text + (buffer-substring-no-properties + (point-min) (point-max))) + +(ert-deftest test-ob/org-babel-remove-result--results-default () + Test `org-babel-remove-result' with default :results. + (mapcar (lambda (language) + (test-ob-verify-result-and-removed-result + \n + (concat +* org-babel-remove-result +#+begin_src language +#+end_src + +* next heading))) + '(sh emacs-lisp))) + +(ert-deftest test-ob/org-babel-remove-result--results-list () + Test `org-babel-remove-result' with :results list. + (test-ob-verify-result-and-removed-result + - 1 +- 2 +- 3 +- (quote (4 5)) + +* org-babel-remove-result +#+begin_src emacs-lisp :results list +'(1 2 3 '(4 5)) +#+end_src + +* next heading)) + +;; TODO FIXME Activate when Eric's trailing newline fix has been committed +;; (ert-deftest test-ob/org-babel-remove-result--results-wrap () +;; (test-ob-verify-result-and-removed-result +;;:RESULTS: +;; hello there +;; :END: +;; +;; * org-babel-remove-result +;; +;; +begin_src emacs-lisp :results wrap +;; \hello there\ +;; #+end_src +;; +;; * next heading)) + +(ert-deftest test-ob/org-babel-remove-result--results-org () + Test `org-babel-remove-result' with :results org. + (test-ob-verify-result-and-removed-result + #+BEGIN_ORG +* heading +**
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Applied, Thanks, Martyn Jago martyn.j...@btinternet.com writes: Hi Eric Eric Schulte eric.schu...@gmx.com writes: [...] All the tests supplied with the exception of `test-ob/org-babel-remove-result--results-raw' will still pass without the change. Alright, would you be willing to resubmit the patch including only those tests which should still apply? This results handling is certainly an area which will benefit from beefing up the test suite. The attached patch contains the remaining tests. Best, Martyn -- Eric Schulte http://cs.unm.edu/~eschulte/
