From: Tom Gillespie <email@example.com>
To: Kyle Meyer <firstname.lastname@example.org>
Cc: emacs-orgmode <email@example.com>
Subject: Re: [PATCH] lisp/ob-core.el: pass expanded body to org-confirm-babel-evaluate
Date: Sun, 6 Sep 2020 02:39:42 -0700 [thread overview]
Message-ID: <CA+G3_POgPjYgEWCM7Fckn5f0yaF6_GzXm-HzWcKr+x76kTfxhA@mail.gmail.com> (raw)
Great. That sounds like the best solution given that correctness
is more important than performance at the moment. We have a good idea
of how to improve the performance going forward so providing the
correct behavior asap seems like the right thing to do. Thank you very
much for getting this in! Best,
On Sat, Sep 5, 2020 at 8:45 PM Kyle Meyer <firstname.lastname@example.org> wrote:
> Tom Gillespie writes:
> > Hi Kyle,
> > Following up in this thread having investigated the impact of coderefs.
> > My conclusion is that coderefs need to be stripped out before they are
> > passed to org-confirm-babel-evaluate. They are not present in the
> > executed code and removing them is not something that a definition
> > of org-confirm-babel-evaluate should have to know anything about.
> > Right now I work around them by suggesting that users comment
> > out their coderefs. This works because my use case is restricted to
> > elisp code and I strip the comments using read, but other languages
> > would not have such an easy solution.
> Thanks for revisiting this. This change (df5a83637) hasn't made it into
> a release yet, so it'd be good to make this move now.
> > I have included a patch against maint that reuses the let block
> > from org-babel-execute-src-block to accomplish this.
> > diff --git a/lisp/ob-core.el b/lisp/ob-core.el
> > index cd876da0f..44b02feb9 100644
> > --- a/lisp/ob-core.el
> > +++ b/lisp/ob-core.el
> > @@ -240,9 +240,14 @@ should be asked whether to allow evaluation."
> > (funcall org-confirm-babel-evaluate
> > ;; Language, code block body.
> > (nth 0 info)
> > - (if (org-babel-noweb-p headers :eval)
> > - (org-babel-expand-noweb-references info)
> > - (nth 1 info)))
> > + (let ((coderef (nth 6 info))
> > + (expand
> > + (if (org-babel-noweb-p params :eval)
> params is undefined here. I've s/params/headers/ when applying.
> > + (org-babel-expand-noweb-references info)
> > + (nth 1 info))))
> > + (if (not coderef) expand
> > + (replace-regexp-in-string
> > + (org-src-coderef-regexp coderef) "" expand nil nil 1))))
> > org-confirm-babel-evaluate))))
> > (cond
> > (noeval nil)
> Okay, so this is equivalent to your original patch, though your initial
> approach avoided duplicating the logic, which I think is worth doing.
> I'd like to make sure this gets in before a release, so I've applied
> this message's patch (3e1c0b0f4) and then followed it up with a patch
> that adds a test, and another that extracts the duplicated logic out to
> a helper (as in your original patch).
prev parent reply other threads:[~2020-09-06 9:40 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
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 [this message]
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:
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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
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).