[PATCH] ob-core: check argument to goto-char

[PATCH] ob-core: check argument to goto-char

[Eike]

[-- Attachment #1.1: Type: text/plain, Size: 546 bytes --]


Hi,

I have some code that uses `org-babel-insert-result' and I've found that
evaluating for example

   (org-babel-insert-result "a")

results in an error. The reason is that `goto-char' is called with a nil
argument. I simply put the snippet in a `when' clause, but since the
function is quite large I'm not so sure if it's now doing always the
correct thing. At least my org files seem still to work….

I've added the patch in case it is ok to be applied. Maybe someone can
have a look at it.

Thanks and regards
Eike



[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 212 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-core-check-argument-to-goto-char.patch --]
[-- Type: text/x-patch, Size: 1031 bytes --]

From fd2182dd3edfb4887cb272a5c93c72660eac0efe Mon Sep 17 00:00:00 2001
From: Eike Kettner <eike.kettner@posteo.de>
Date: Sat, 30 Apr 2016 12:59:41 +0200
Subject: [PATCH] ob-core: check argument to `goto-char'

The argument to `goto-char' must not be nil, which occurs if
`org-element-property' is called with nil element argument.
---
 lisp/ob-core.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 71c7aea..088f744 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -2210,8 +2210,10 @@ INFO may provide the values of these header arguments (in the
 	      (progn
 		(when outside-scope (widen))
 		(if existing-result (goto-char existing-result)
-		  (goto-char (org-element-property :end inline))
-		  (skip-chars-backward " \t"))
+		  (let ((end (org-element-property :end inline)))
+		    (when end
+		      (goto-char end)
+		      (skip-chars-backward " \t"))))
 		(unless inline
 		  (setq indent (org-get-indentation))
 		  (forward-line 1))
-- 
2.7.4


[-- Attachment #3: Type: text/plain, Size: 83 bytes --]


-- 
gpg: AD7AC35E
finger print: 137F BB0B 1639 D25F DC5D E59C B412 C5F5 AD7A C35E

Re: [PATCH] ob-core: check argument to goto-char

[Charles C. Berry]

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

On Sat, 30 Apr 2016, Eike wrote:

>
> Hi,
>
> I have some code that uses `org-babel-insert-result' and I've found that
> evaluating for example
>
>   (org-babel-insert-result "a")
>
> results in an error.

Although not explicitly marked as such, `org-babel-insert-result' seems 
intended as an internal function for processing babel RESULTs.

> The reason is that `goto-char' is called with a nil
> argument.

When your snippet is in a src block or inline src block, there is no 
error. Also, no error when point is in a src block and you run the snippet 
with

:  M-x eval-expression RET (org-babel-insert-result "a") RET

So, it looks like you are trying to evaluate the snippet above when 
point is not in a src block. AFAICS, there is no guarantee that such usage 
will succeed.

Failure in such uses seems more like a feature than a bug.

> I simply put the snippet in a `when' clause, but since the
> function is quite large I'm not so sure if it's now doing always the
> correct thing. At least my org files seem still to work….
>
> I've added the patch in case it is ok to be applied. Maybe someone can
> have a look at it.
>

See http://orgmode.org/worg/org-contribute.html for details on how to 
contribute. Also, running `make test' on new code is a good idea.

If you can explain what you are trying to achieve, someone may suggest a 
fix that does not require retooling babel internals.

Or if you can provide an ECM that suggests a bug in babel, then someone 
may suggest a better fix.

HTH,

Chuck

Re: [PATCH] ob-core: check argument to goto-char

[Eike]

Charles C. Berry <ccberry@ucsd.edu> writes:

> On Sat, 30 Apr 2016, Eike wrote:
>
>>
>> Hi,
>>
>> I have some code that uses `org-babel-insert-result' and I've found that
>> evaluating for example
>>
>>   (org-babel-insert-result "a")
>>
>> results in an error.
>
> Although not explicitly marked as such, `org-babel-insert-result' seems 
> intended as an internal function for processing babel RESULTs.

ok, I didn't know.


>> The reason is that `goto-char' is called with a nil
>> argument.
>
> When your snippet is in a src block or inline src block, there is no 
> error. Also, no error when point is in a src block and you run the snippet 
> with
>
> :  M-x eval-expression RET (org-babel-insert-result "a") RET
>
> So, it looks like you are trying to evaluate the snippet above when 
> point is not in a src block. AFAICS, there is no guarantee that such usage 
> will succeed.
>
> Failure in such uses seems more like a feature than a bug.

Ok, so it wasn't clear to me that point must be in a src block. Since
the results are passed as argument, I made the false assumption that it
inserts them where point currently is /or/ after a src block.

The lisp code at that point (ob-core.el l.2212) invokes goto-char with
nil which is caused by `inline' being nil. But the next line checks for
`inline' being nil as does the line before: (if existing-result
(goto-char existing-result) …) so I thought it might be good to check
for that in the else-branch, too.

>> I simply put the snippet in a `when' clause, but since the
>> function is quite large I'm not so sure if it's now doing always the
>> correct thing. At least my org files seem still to work….
>>
>> I've added the patch in case it is ok to be applied. Maybe someone can
>> have a look at it.
>>
>
> See http://orgmode.org/worg/org-contribute.html for details on how to 
> contribute. Also, running `make test' on new code is a good idea.
>
> If you can explain what you are trying to achieve, someone may suggest a 
> fix that does not require retooling babel internals.

I didn't really mean you to apply my patch, but rather have a look at
it: thanks for that. I thought it's simpler to send a patch file than to
explain in words… `make test' doesn't show unexpected failures to me,
did I miss something? I'm sorry for breaking any contributing
rules, I'll try to do better next time.

What I want to do: I want to insert an org table somewhere in an org
buffer. The data is not from an src block but retrieved from somewhere
else. So I have a list like `(("id" "num") hline ("a" "1") ("b" "2"))'
and I'd like to put it in a buffer as an org table (the buffer is in
org-mode).

It is certainly not my intention to retool babel internals to achieve
this. My impression was that this may be a bug. But it may not, that's
why I asking here.

Thanks
Eike

-- 
gpg: AD7AC35E
finger print: 137F BB0B 1639 D25F DC5D E59C B412 C5F5 AD7A C35E

Re: [PATCH] ob-core: check argument to goto-char

[Charles C. Berry]

On Sat, 30 Apr 2016, Eike wrote:

>
>
> Charles C. Berry <ccberry@ucsd.edu> writes:
>
>> On Sat, 30 Apr 2016, Eike wrote:
>>
>>>
>>> Hi,
>>>

[much deleted]

>
> What I want to do: I want to insert an org table somewhere in an org
> buffer. The data is not from an src block but retrieved from somewhere
> else. So I have a list like `(("id" "num") hline ("a" "1") ("b" "2"))'
> and I'd like to put it in a buffer as an org table (the buffer is in
> org-mode).
>

The easiest way to borrow babel tools is to use babel to execute src 
blocks directly. Here is a start. Run this src block to define a function
`insert-quoted-list-as-result':

#+BEGIN_SRC emacs-lisp
   (defun insert-quoted-list-as-result (my-list)
   (save-excursion
     (insert
      (format
       "#+BEGIN_SRC emacs-lisp\n '%S\n#+END_SRC\n\n" my-list))
     (let ((org-confirm-babel-evaluate nil))
       (org-babel-execute-src-block)))
     (delete-region (point)
 		   (org-babel-where-is-src-block-result)))
#+END_SRC


Then put the next line in an *.org buffer and try it out by typing C-x C-e 
just below the line:

: (insert-quoted-list-as-result '(("id" "num") hline ("a" "1") ("b" "2")))

Note that it only works if point is on its own line. So you want to add 
some checks to prevent execution otherwise or to insert newlines as 
needed before invoking the function.

Also note: if there is a #+RESULTS: block immediately following, it will 
be replaced, which you may not want. And you may want to remove the 
#+RESULTS: line, too.


HTH,

Chuck

Re: [PATCH] ob-core: check argument to goto-char

[Nicolas Goaziou]

Hello,

"Charles C. Berry" <ccberry@ucsd.edu> writes:

> The easiest way to borrow babel tools is to use babel to execute src 
> blocks directly. Here is a start. Run this src block to define a function
> `insert-quoted-list-as-result':
>
> #+BEGIN_SRC emacs-lisp
>    (defun insert-quoted-list-as-result (my-list)
>    (save-excursion
>      (insert
>       (format
>        "#+BEGIN_SRC emacs-lisp\n '%S\n#+END_SRC\n\n" my-list))
>      (let ((org-confirm-babel-evaluate nil))
>        (org-babel-execute-src-block)))
>      (delete-region (point)
>  		   (org-babel-where-is-src-block-result)))
> #+END_SRC
>
> Then put the next line in an *.org buffer and try it out by typing C-x C-e 
> just below the line:
>
> : (insert-quoted-list-as-result '(("id" "num") hline ("a" "1") ("b" "2")))

I suggest to use built-in `orgtbl-to-orgtbl' instead:

  (insert (orgtbl-to-orgtbl '(("id" "num") hline ("a" "1") ("b" "2")) nil))


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] ob-core: check argument to goto-char

[Eike]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> I suggest to use built-in `orgtbl-to-orgtbl' instead:
>
>   (insert (orgtbl-to-orgtbl '(("id" "num") hline ("a" "1") ("b" "2")) nil))
>

Ah, that's what I'm looking for. Thanks!

Eike

-- 
gpg: AD7AC35E
finger print: 137F BB0B 1639 D25F DC5D E59C B412 C5F5 AD7A C35E

Re: [PATCH] ob-core: check argument to goto-char

[Eike]

Charles C. Berry <ccberry@ucsd.edu> writes:

> The easiest way to borrow babel tools is to use babel to execute src 
> blocks directly. Here is a start. Run this src block to define a function
> `insert-quoted-list-as-result':
>
> #+BEGIN_SRC emacs-lisp
>    (defun insert-quoted-list-as-result (my-list)
>    (save-excursion
>      (insert
>       (format
>        "#+BEGIN_SRC emacs-lisp\n '%S\n#+END_SRC\n\n" my-list))
>      (let ((org-confirm-babel-evaluate nil))
>        (org-babel-execute-src-block)))
>      (delete-region (point)
>  		   (org-babel-where-is-src-block-result)))
> #+END_SRC
>

Oh I see. It is more complicated than I hoped.

Thanks,
Eike

-- 
gpg: AD7AC35E
finger print: 137F BB0B 1639 D25F DC5D E59C B412 C5F5 AD7A C35E