emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: Tom Gillespie <tgbugs@gmail.com>
Cc: emacs-orgmode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] lisp/ob-core.el: pass expanded body to org-confirm-babel-evaluate
Date: Wed, 22 Jul 2020 00:19:56 -0400	[thread overview]
Message-ID: <87k0ywyydv.fsf@kyleam.com> (raw)
In-Reply-To: <CA+G3_PPCdwkKDnaNuFCd8jZum1c2VAa64yZF+41pcscCPk0eAA@mail.gmail.com>

Tom Gillespie writes:

> On Sun, Jul 19, 2020 at 2:13 PM Kyle Meyer <kyle@kyleam.com> wrote:

>> An option not mentioned above is to replace (nth 1 info) with the
>> expanded body upstream of (when (org-babel-check-evaluate info) ...).
>> Modifying the body in INFO is admittedly not pretty, but it's in line
>> with what is done elsewhere (e.g., -expand-src-block,
>> -exp-process-buffer, -load-in-session), as well as with how other INFO
>> elements in -execute-src-block are handled.
>
> I considered this as well, and in fact I assumed that this is how it worked
> before I looked to see the code was actually doing. I didn't know what the
> consequences of making it work that way would be and tend to err on the
> side of not kobbering data that some other function might expect to be in
> an unmodified state. This would certainly be the most expedient solution.
>
> There would need to be a way to indicate that info should not expand noweb
> references so that :noweb no-export can get the unexpanded form. Maybe
> and optional argument =expand= that defaults to t?
> (defun org-babel-get-src-block-info (&optional light datum expand)
> ... )

Hmm, it looks like you're thinking of a different spot than I was
referring to.  I was talking about modifying (nth 1 info) in
-execute-src-block, before the (when (org-babel-check-evaluate ...).

Prior to commit 3b3fc520a, (nth 1 info) was modified in
-execute-src-block but after org-babel-check-evaluate and
org-babel-confirm-evaluate.  That makes me think it'd probably be safe
to do again, just a bit upstream in the same function.  And I don't see
a spot where modifying that element would be problematic.  On the
incoming end, INFO is passed to copy-tree if it is provided as an
argument, and I don't think any of the functions that -execute-src-block
feeds INFO to would be affected.  But I of course could be overlooking
something.

On the other hand, if org-babel-get-src-block-info did the expansion,
I'd be a bit more worried about downstream effects (though I haven't
tried to trace things too closely).  Also, -execute-src-block takes a
PARAMS argument, and I think that needs to be merged with (nth 2 info)
before considering whether to expand, so it seems like expanding in
-get-src-block-info would be too soon.

Regardless of whether modifying (nth 1 info) would work, I also think
it'd be fine to instead go ahead with something along the lines of your
initial patch.  In that case, the main question is whether coderefs
should be removed (as they currently are in your patch).  If not,
perhaps something like below (untested) would suffice.

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index e798595bd..230706b7f 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -238,7 +238,10 @@ (defun org-babel-check-confirm-evaluate (info)
 		    (if (functionp org-confirm-babel-evaluate)
 			(funcall org-confirm-babel-evaluate
 				 ;; Language, code block body.
-				 (nth 0 info) (nth 1 info))
+				 (nth 0 info)
+				 (if (org-babel-noweb-p headers :eval)
+				     (org-babel-expand-noweb-references info)
+				   (nth 1 info)))
 		      org-confirm-babel-evaluate))))
     (cond
      (noeval nil)


  reply	other threads:[~2020-07-22  4:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13 19:53 [PATCH] lisp/ob-core.el: pass expanded body to org-confirm-babel-evaluate Tom Gillespie
2020-07-19 21:13 ` Kyle Meyer
2020-07-20 20:27   ` Tom Gillespie
2020-07-22  4:19     ` Kyle Meyer [this message]
2020-08-02  6:03       ` Tom Gillespie
2020-08-03  3:05         ` Kyle Meyer
2020-09-05  3:54           ` Tom Gillespie
2020-09-06  3:45             ` Kyle Meyer
2020-09-06  9:39               ` Tom Gillespie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k0ywyydv.fsf@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=tgbugs@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).