From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id uDCLDqW3FF99cgAA0tVLHw (envelope-from ) for ; Sun, 19 Jul 2020 21:14:13 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id cLMnCqW3FF8EbQAAbx9fmQ (envelope-from ) for ; Sun, 19 Jul 2020 21:14:13 +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 4C860940630 for ; Sun, 19 Jul 2020 21:14:12 +0000 (UTC) Received: from localhost ([::1]:48192 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jxGdN-0002Hr-Vs for larch@yhetil.org; Sun, 19 Jul 2020 17:14:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51398) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jxGcz-0002Hg-EN for emacs-orgmode@gnu.org; Sun, 19 Jul 2020 17:13:45 -0400 Received: from pb-smtp21.pobox.com ([173.228.157.53]:55736) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jxGcw-00024P-HW for emacs-orgmode@gnu.org; Sun, 19 Jul 2020 17:13:44 -0400 Received: from pb-smtp21.pobox.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id 01576E14C8; Sun, 19 Jul 2020 17:13:39 -0400 (EDT) (envelope-from kyle@kyleam.com) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:in-reply-to:references:date:message-id:mime-version :content-type; s=sasl; bh=9YofsyjmWP656SCFq+1/ycAGdRI=; b=LoY4G1 M0dsqCRZ+CCw6m4ZoOcZHhJ8NsGSOqvPdvXO3amz85M1Q6/UizdlxHawmdIyvHU9 CURY1i1rpCPf6k47eMyqIV1mZ32UZwSfTbqKp0yWPwqL3edntuDsdndzKbIG8zFP jbL5aaO6ka0b7GjVtaEGjtEu0dR93ps9NPgQE= Received: from pb-smtp21.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id ED932E14C7; Sun, 19 Jul 2020 17:13:38 -0400 (EDT) (envelope-from kyle@kyleam.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=kyleam.com; h=from:to:cc:subject:in-reply-to:references:date:message-id:mime-version:content-type; s=mesmtp; bh=Y/XxUPanZNpd4MMPxr2p0/eqBMRuIiO9fy5FK29Vw2w=; b=C9E0DzF6SMapGM1GXzBATr0W3COBObr4fHx5Ahu4ieXVb39bHveC5jHRM5R5qZ2csR1Kk0NnERTrmq5sc6HafZTLVpjQFb28W/Os39hjA4PktumFjJfruoZgCUnuD4WDGIEfd4xP0NxZ3ZF974u+e68xKekSf10BhajUOJhS3f8= Received: from localhost (unknown [45.33.91.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp21.pobox.com (Postfix) with ESMTPSA id 61425E14C6; Sun, 19 Jul 2020 17:13:36 -0400 (EDT) (envelope-from kyle@kyleam.com) From: Kyle Meyer To: Tom Gillespie Subject: Re: [PATCH] lisp/ob-core.el: pass expanded body to org-confirm-babel-evaluate In-Reply-To: References: Date: Sun, 19 Jul 2020 17:13:34 -0400 Message-ID: <87h7u3dx8h.fsf@kyleam.com> MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: B5A15854-CA04-11EA-BC4A-843F439F7C89-24757444!pb-smtp21.pobox.com Received-SPF: pass client-ip=173.228.157.53; envelope-from=kyle@kyleam.com; helo=pb-smtp21.pobox.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/07/19 17:13:39 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 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, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=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@gnu.org 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=pobox.com header.s=sasl header.b=LoY4G1 M; dkim=pass header.d=kyleam.com header.s=mesmtp header.b=C9E0DzF6; dmarc=none; 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.21 X-TUID: litpQqFn3Ydz Tom Gillespie writes: > This is a patch to improve the behavior of > org-babel-check-confirm-evaluate and the usefulness of > org-confirm-babel-evaluate when a function is provided. Thank you. > This commit changes the behavior of org-babel-check-confirm-evaluate > so that org-confirm-babel-evaluate receives the fully expanded code to > be evaluated. This is important because there is no easy way to expand > noweb references inside org-confirm-babel-evaluate without calling > org-babel-get-src-block-info a second time. It is also important > because the current behavior makes it possible for code lurking behind > noweb references to change without the user being able to detect those > changes if they trust org-confirm-babel-evaluate is receiving the > actual body of the code that will be evalulated. I find that convincing. I'd guess that a org-confirm-babel-evaluate function would always want to see expanded noweb references. This doesn't mention coderefs, though, and the case there seems less clear. If there's not a strong reason to strip coderefs, then the new function wouldn't be necessary, and -check-confirm-evaluate could instead go with the pattern used elsewhere: (if (org-babel-noweb-p (nth 2 info) :eval) (org-babel-expand-noweb-references info) (nth 1 info)) (Perhaps it'd be worth adding a -maybe-expand variant of -expand-noweb-references since there are a good number of spots that do the same params check before calling -expand-noweb-references.) > These changes were made in such a way as to minimize changes to the > existing functions, however they come at the cost of making two calls > to org-babel-get-body-to-eval [...] Or potentially three calls to -get-body-to-eval: one call with the -check-evaluate call in -execute-src-block, one with the -confirm-evaluate in -execute-src-block, and then the direct call to -get-body-to-eval in -execute-src-block. > [...] since passing the expanded body through to > org-confirm-babel-evaluate would either require appending it to the > info list or changing the function signatures of > org-babel-confirm-evaluate and org-babel-check-confirm-evaluate which > is undesireable. Furthermore, to compute the expanded body only once > would require switching from using cond in org-babel-execute-src-block > to using a series of nested ifs. This change can be made, but starting > from the current implementation will provide a working reference for > comparison rather than trying to make all the changes at the same > time. 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. In fact, -execute-src-block did modify (nth 1 info) before 3b3fc520a (Fix coderef handling in source blocks, 2016-08-28), though too late to affect calls to a org-confirm-babel-evaluate function. Quickly checking the tests added in that commit as well as the example in the message [0] that prompted that commit, modifying (nth 1 info) didn't seem to break the fix there. [0] https://orgmode.org/list/87mvjyoja2.fsf@dell-desktop.WORKGROUP