Re: [O] [PATCH 03/10] Clean up various org-babel-*-maybe commands

[Aaron Ecay]

Hi Bastien,

Thanks for your comments.

2013ko apirilak 3an, Bastien-ek idatzi zuen:

[...]

>> org-babel-get-src-block-info is a potentially expensive operation, which
>> is why its ‘light’ argument exists.  But in any case, it is overkill to
>> query the whole info, if all that is needed is whether point is in a
>> block or not.  Factor the simplified common code out into a macro.
> 
> The let-bound info variable is not only used to check if we are within
> a src block, it is also passed as an argument to functions, see the ^^
> marks below.

All of these functions will re-calculate the info if it is not
passed, using org-babel-get-src-block-info.  So not passing it does no
harm.

> 
>> ---
>> lisp/ob-core.el | 31 +++++++++++++++++--------------
>> 1 file changed, 17 insertions(+), 14 deletions(-)
>> 
>> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
>> index 723aa9d..283628e 100644
>> --- a/lisp/ob-core.el
>> +++ b/lisp/ob-core.el
>> @@ -365,15 +365,22 @@ of potentially harmful code."
>> (or (org-babel-execute-src-block-maybe)
>> (org-babel-lob-execute-maybe)))
>> 
>> +(defmacro org-babel-when-in-src-block (&rest body)
>> +  `(if (or (org-babel-where-is-src-block-head)
>> +           (org-babel-get-inline-src-block-matches))
>> +       (progn
>> +     ,@body
>> +     t)
>> +     nil))
> 
> (Please always add a docstring of defuns and defmacros)

I’ll resend the patch with a docstring and fixing the commit message
problems you and Achim pointed out.


[...]


>> @@ -433,8 +436,8 @@ then run `org-babel-load-in-session'."
>> Detect if this is context for a org-babel src-block and if so
>> then run `org-babel-pop-to-session'."
>> (interactive)
>> -  (let ((info (org-babel-get-src-block-info)))
>> -    (if info (progn (org-babel-pop-to-session current-prefix-arg info) t) 
>> nil)))
>                                                                     ^^^^
>> +  (org-babel-when-in-src-block
>> +   (org-babel-pop-to-session current-prefix-arg)))
> 
> (Let's use the current name `org-babel-switch-to-session' instead of
> the obsolete alias.)

OK.

> 
> Maybe we don't always need to pass the info as an argument, but at
> least for this last example it is needed.

o-b-switch-to-session does nothing with the info argument but pass it
along to o-b-initiate-session, which will recalculate it if it is
missing.  So it takes 2 hops in contrast to the 1 in the other cases,
but it still gets recalculated.

-- 
Aaron Ecay