From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id wGY3AX5XJl+GDgAA0tVLHw (envelope-from ) for ; Sun, 02 Aug 2020 06:04:46 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id OAHFOH1XJl8iTQAA1q6Kng (envelope-from ) for ; Sun, 02 Aug 2020 06:04:45 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 662839403CC for ; Sun, 2 Aug 2020 06:04:45 +0000 (UTC) Received: from localhost ([::1]:37818 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1k276y-0004Ud-AV for larch@yhetil.org; Sun, 02 Aug 2020 02:04:44 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37178) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1k276R-0004US-Ft for emacs-orgmode@gnu.org; Sun, 02 Aug 2020 02:04:11 -0400 Received: from mail-wm1-x341.google.com ([2a00:1450:4864:20::341]:37420) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1k276P-0001QE-DS for emacs-orgmode@gnu.org; Sun, 02 Aug 2020 02:04:11 -0400 Received: by mail-wm1-x341.google.com with SMTP id k8so12640102wma.2 for ; Sat, 01 Aug 2020 23:04:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=TLDDPgtpnqfywns9sehyqgGC/DZ+0/Om96uKyjeMZEU=; b=GdLK5tz6uqENO1t5vXHLNaXBiWMfU2QypijRyGGnIm/oA0oJrvGpseOdbq7s1AwGMh rIFWNQROLfQOtooHY1oWpPBO4ndItEhuXKEDApW30bsEr/cz36yIcF7mngDkmlBbLaWn q5bs9PfWmCzecui+DuAZNor0+tW+7jeQYvDK1OodtdNXqVzLs2E443A3FxahCYi5G9BC bymeD5INHfJ9lZPMJnNHhSUXLk1fbkuQIiSYjrEi3BfJoqrVcAIy5aOqVMOe4piWaMoc HQAwAmpGogq/VqwZBLeBnqqBtAn2XIbi69ML/dIJLNda0T2ooDz/OXk20bIN6Zl0MiQD ohEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TLDDPgtpnqfywns9sehyqgGC/DZ+0/Om96uKyjeMZEU=; b=NKTPM09WLAw43xVY+53PFMdbB1M3U2XZswQXEwihVEhifcSNSGcQojAIOAgmVpk8f9 +I0X1nRo+AVs8BMW+kGb6aPVNS1VoySa1gh/i5GRzFhhL/yehZzMCn95a6lYQ9nFQgh+ PrDwzvlBOA/+nXe+19lOuAHUkdiYkmSQC832H1zi91LMTzEKyAL38mAbFsa+tmgLLqKK 9tQlX1BUA+eAVEB+H84EzK0ohayv/4pKKSWIVJ3cyBFjxwFW4eJbA759xsRz/9QjbqNp QrH0DI6QvFZl/xhQB10r7imYGH9QJV63Fl1CINuIL6Ln731BXBQFj9pwitw6NXf25K1U xI+g== X-Gm-Message-State: AOAM531enkmsuhVdKyLurREAJZ3Lw3MBDS96p5vLdXV7Vnr0suVJQUV3 VNih8pQmuaPTPmbqliGSuEGk8HHwRqp9yFB77oJ+pOeH X-Google-Smtp-Source: ABdhPJz+JzFJxZIRtioxvOf/f33S71ugn9zYoeTRogb+zgZYKng5r7qRwRkC3kCJW92c09S8qJS0OnoM1cVjf3YzVe4= X-Received: by 2002:a1c:e008:: with SMTP id x8mr10365557wmg.75.1596348245928; Sat, 01 Aug 2020 23:04:05 -0700 (PDT) MIME-Version: 1.0 References: <87k0ywyydv.fsf@kyleam.com> In-Reply-To: <87k0ywyydv.fsf@kyleam.com> From: Tom Gillespie Date: Sat, 1 Aug 2020 23:03:54 -0700 Message-ID: Subject: Re: [PATCH] lisp/ob-core.el: pass expanded body to org-confirm-babel-evaluate To: Kyle Meyer Content-Type: text/plain; charset="UTF-8" Received-SPF: pass client-ip=2a00:1450:4864:20::341; envelope-from=tgbugs@gmail.com; helo=mail-wm1-x341.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emacs-orgmode Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=GdLK5tz6; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Spam-Score: -1.71 X-TUID: OC7K/pmI1sa8 Hi Kyle, Sorry for the slow turnaround time on this one. Having now tested it, I think that your solution is a much better one for the time being, so please go ahead and apply it. From this discussion there are a number of good options for improvements in the future, but my priority would be to get the expanded version of the body passed to org-confirm-babel-evaluate asap with as few disturbances to the rest of the code base as possible. Best! Tom On Tue, Jul 21, 2020 at 9:20 PM Kyle Meyer wrote: > > Tom Gillespie writes: > > > On Sun, Jul 19, 2020 at 2:13 PM Kyle Meyer 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)