[PATCH] org-babel-goto-named-src-block bugfix and test

[PATCH] org-babel-goto-named-src-block bugfix and test

[Charles C. Berry]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]



Hi all,

The revision a few months back of `org-babel-goto-named-src-block' broke 
some of the interactive uses. I have fixed these in the attached patch.

Also, I provide an ERT test for those interactive uses --- filling in the 
initial-input with the name of the symbol, results block name, #+call: 
name, or not if point is elsewhere.

The test uses lines like:

 	(execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")

to emulate interactive usage.

This feels like a hack, and the messages generated spill into my *shell* 
buffer when I run `make test'.  If there is a cleaner way to do this, 
I'd like to know it.

WDYT?

Chuck

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4135 bytes --]

From 6b6d0beb908474b75e903c86649a04b6ef71048c Mon Sep 17 00:00:00 2001
From: Charles Berry <ccberry@ucsd.edu>
Date: Sat, 11 Jun 2016 15:45:20 -0700
Subject: [PATCH] org-babel-goto-named-src-block bugfix

* lisp/ob-core.el (org-babel-goto-named-src-block): The user prompt
  (i.e. `initial-input' arg of `completing-read') will be the name of
  the results block, noweb reference, call reference, or symbol if
  point is in such.

* testing/lisp/test-ob.el (test-ob/goto-named-src-block): Simulate
  interactive use of `org-babel-goto-named-src-block'.
---
 lisp/ob-core.el         | 18 ++++++++++++------
 testing/lisp/test-ob.el | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 2e9a4d1..63983d5 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -1690,16 +1690,22 @@ If the point is not on a source block then return nil."
 	 (all-block-names (org-babel-src-block-names)))
      (list (completing-read
 	    "source-block name: " all-block-names nil t
-	    (let* ((context (org-element-context))
-		   (type (org-element-type context)))
+	    (let* ((context (org-element-context)) 
+		   (type (org-element-type context))
+		   (noweb-ref
+		    (and (memq type '(inline-src-block src-block))
+			 (org-in-regexp (org-babel-noweb-wrap))))) 
 	      (cond
-	       ((and (memq type '(inline-src-block src-block)) ;<<noweb>>
-		     (org-in-regexp (org-babel-noweb-wrap))))
+	       (noweb-ref
+		(buffer-substring
+		 (+ (car noweb-ref) (length org-babel-noweb-wrap-start))
+		 (- (cdr noweb-ref) (length org-babel-noweb-wrap-end))))
 	       ((memq type '(babel-call inline-babel-call)) ;#+CALL:
 		(org-element-property :call context))
-	       ((org-element-property :results context)) ;#+RESULTS:
+	       ((car (org-element-property :results context))) ;#+RESULTS:
 	       ((let ((symbol (thing-at-point 'symbol))) ;Symbol.
-		  (and (member-ignore-case symbol all-block-names)
+		  (and symbol
+		       (member-ignore-case symbol all-block-names)
 		       symbol)))
 	       (t "")))))))
   (let ((point (org-babel-find-named-block name)))
diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el
index aa26602..fe5bbea 100644
--- a/testing/lisp/test-ob.el
+++ b/testing/lisp/test-ob.el
@@ -1581,6 +1581,48 @@ echo \"$data\"
    (org-test-with-temp-text "#+results: foo"
      (org-babel-find-named-result "foo"))))
 
+(ert-deftest test-ob/goto-named-src-block ()
+    "Test interactive use of `org-babel-goto-named-src-block'."
+    (org-test-with-temp-text-in-file
+	 	"
+#+NAME: abc
+#+BEGIN_SRC emacs-lisp :results value 
+(1+ 1)
+#+END_SRC
+#+CALL: abc( lorem() ) :results raw :wrap EXAMPLE
+#+BEGIN_SRC emacs-lisp
+<<abc>>
+#+END_SRC
+abc
+#+RESULTS: abc
+: 2
+"
+       ;; non-existent name
+       (should-not
+         (execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\ndef\n"))
+       ;; correct name
+       (execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\nabc\n")
+       (should  (= 14 (point)))  
+       ;; call line   - autocompletion
+       (forward-line 3)
+       (execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")
+       (should  (= 14 (point)))
+       ;; noweb reference  - autocompletion
+       (forward-line 5)
+       (execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")
+       (should  (= 14 (point)))
+       ;; at symbol  - autocompletion
+       (forward-line 7)
+       (execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")
+       (should  (= 14 (point)))
+       ;; in results  - autocompletion
+       (forward-line 8)
+       (execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")
+       (should  (= 14 (point)))
+       (forward-line 9)
+       (execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")
+       (should  (= 14 (point)))))
+
 (ert-deftest test-ob/where-is-src-block-result ()
   "Test `org-babel-where-is-src-block-result' specifications."
   ;; Find anonymous results.
-- 
2.6.4 (Apple Git-63)

Re: [PATCH] org-babel-goto-named-src-block bugfix and test

[Marco Wahl]

Hi Charles,

> [...]
> The test uses lines like:
>
> 	(execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")
>
> to emulate interactive usage.
>
> This feels like a hack, and the messages generated spill into my
> *shell* buffer when I run `make test'.  If there is a cleaner way to
> do this, I'd like to know it.
>
> WDYT?

Why not just use e.g.

  (org-babel-goto-named-src-block "def")

instead of

  (execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\ndef\n")

?


Best regards,
-- 
Marco Wahl <-> 0x49010A040A3AE6F2

Re: [PATCH] org-babel-goto-named-src-block bugfix and test

[Charles C. Berry]

On Sun, 12 Jun 2016, Marco Wahl wrote:

> The following message is a courtesy copy of an article
> that has been posted to gmane.emacs.orgmode as well.
>
> Hi Charles,
>
>> [...]
>> The test uses lines like:
>>
>> 	(execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")
>>
>> to emulate interactive usage.
>>
>> This feels like a hack, and the messages generated spill into my
>> *shell* buffer when I run `make test'.  If there is a cleaner way to
>> do this, I'd like to know it.
>>
>> WDYT?
>
> Why not just use e.g.
>
>  (org-babel-goto-named-src-block "def")
>

Because that does not test the `(interactive ...)' form, which comprises 
most of the code in `org-babel-goto-named-src-block'.

Chuck

Re: [PATCH] org-babel-goto-named-src-block bugfix and test

[Marco Wahl]

>>> The test uses lines like:
>>>
>>> 	(execute-kbd-macro  "\M-xorg-babel-goto-named-src-block\n\n")
>>>
>>> to emulate interactive usage.
>>>
>>> This feels like a hack, and the messages generated spill into my
>>> *shell* buffer when I run `make test'.  If there is a cleaner way to
>>> do this, I'd like to know it.
>>>
>>> WDYT?
>>
>> Why not just use e.g.
>>
>>  (org-babel-goto-named-src-block "def")
>>
>
> Because that does not test the `(interactive ...)' form, which
> comprises most of the code in `org-babel-goto-named-src-block'.

Okay.

I have no idea for a cleaner implementation of these tests.  AFAICS
these tests look good (and also your code.)  If someone has a better
idea for the tests they can be changed.

I would drop test

#v+
;; non-existent name
(should-not
  (execute-kbd-macro
"\M-xorg-babel-goto-named-src-block\ndef\n"))
#v-

because its outcome has nothing to do with the existence of the name
"def" AFAICS.  Or maybe replace with

(should (string= "source-code block `non-existent' not found in this buffer"
(org-babel-goto-named-src-block "non-existent")))

+1 for commit!


Best regards,
-- 
Marco Wahl -- GPG: 0x49010A040A3AE6F2

Re: [PATCH] org-babel-goto-named-src-block bugfix and test

[Nicolas Goaziou]

Hello,

Marco Wahl <marcowahlsoft@gmail.com> writes:

> I have no idea for a cleaner implementation of these tests.  AFAICS
> these tests look good (and also your code.)  If someone has a better
> idea for the tests they can be changed.

I don't.

> I would drop test
>
> #v+
> ;; non-existent name
> (should-not
>   (execute-kbd-macro
> "\M-xorg-babel-goto-named-src-block\ndef\n"))
> #v-
>
> because its outcome has nothing to do with the existence of the name
> "def" AFAICS.  Or maybe replace with
>
> (should (string= "source-code block `non-existent' not found in this buffer"
> (org-babel-goto-named-src-block "non-existent")))
>
> +1 for commit!

I'm not very keen on having `should' macro in the inner parts of tests,
because it makes debugging failing tests harder, but a test is better
than nothing. So, all in all, it looks good enough to be pushed.

Thank you !


Regards,

-- 
Nicolas Goaziou