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 aEJ7KoSuVF8EXQAA0tVLHw (envelope-from ) for ; Sun, 06 Sep 2020 09:40:20 +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 IHA6JoSuVF/YcwAAbx9fmQ (envelope-from ) for ; Sun, 06 Sep 2020 09:40:20 +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 47A9F94021E for ; Sun, 6 Sep 2020 09:40:20 +0000 (UTC) Received: from localhost ([::1]:60830 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kEr9n-0007dR-9P for larch@yhetil.org; Sun, 06 Sep 2020 05:40:19 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:33464) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kEr9R-0007dL-2y for emacs-orgmode@gnu.org; Sun, 06 Sep 2020 05:39:57 -0400 Received: from mail-wr1-x443.google.com ([2a00:1450:4864:20::443]:44153) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kEr9P-0005U0-Fv for emacs-orgmode@gnu.org; Sun, 06 Sep 2020 05:39:56 -0400 Received: by mail-wr1-x443.google.com with SMTP id c15so11544833wrs.11 for ; Sun, 06 Sep 2020 02:39:55 -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=4wrlu7USgS8DsPozV1hsDb3LnQ5HJZ+XOej3I6wGLf8=; b=vQf+kmE/izaILSJr2IWO+sQJZYouiD3jiVhT9ljs3BjGDio4jz9g01oKNuV4zmEK/w ckkhvYCWoXwEi5Gmfa+JaFuHVO8DbsOiSB3jzniSNhzLfUD+H204or09poZwdyqkSK/3 /NINOd8fwbVnEZx/wvYWOd8OVT75v+YLt9TR6VfchcrzyjyqK4K4+P5GFFshLvjNoxWc /DC791vhi/BB9v9bmkY/LpjWIYWke6GL11a/Uc4xPIT+1VNHwnMXT+UeHWZSSDzj7aoK IN2hbBMPbyCg9XRBgrUVcFGp2HuA/ZjAth6dHCK2R4zK1p3Pz3W1qolZ1AReJYRAaJwx +aKg== 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=4wrlu7USgS8DsPozV1hsDb3LnQ5HJZ+XOej3I6wGLf8=; b=Szn9n4NXk4E0rjgy9FS2ESzgQV08NuD/VKdfIq7P7Rke9850Aorm7hOOSxuw9tt7Fa 4uDMOe7DHpt51WVDqxQX+D5E/tVxssxJ8vwxUJD4h1O8s0dmJAVnrZjBdarD+hn7kjyw pJr+pa3FmjPadAd0lDml04SuQBXu6IQgYluyoeq8yQMFuvuJ2Apl967wAqU9NxBwZgAa fTKzSrFAngGVFwtGD3q7VyVbPQo75CnKf/erV2cjSsbxuxHux35Ir98az9qSqratB8vD 0Hh+up2tPs/qNOj9q6FXKh6XUj8JKBBf/JMicDy83DKPxXvxFYWcBQDMmFMuTSY6yj2n 7NlA== X-Gm-Message-State: AOAM53309bHrcK14OfqEeS2yXL4NYrOdmn+0o4gD7j61dSu1nWWVnmXl szp9vylkaceqbDB2dqfZNOsC9k8O/G2bMRgpGu13pfxn X-Google-Smtp-Source: ABdhPJyQWo0Tf0Eo60nVf6pELYzL2lKiZLSTMJpoMWDIpJJQkci5/b+qa2Dqg4iHxYTGB9B+/qMnRQjhDOWt6piBD+o= X-Received: by 2002:adf:912b:: with SMTP id j40mr15990756wrj.42.1599385193638; Sun, 06 Sep 2020 02:39:53 -0700 (PDT) MIME-Version: 1.0 References: <87k0ywyydv.fsf@kyleam.com> <87k0yg4efs.fsf@kyleam.com> <87wo1736wj.fsf@kyleam.com> In-Reply-To: <87wo1736wj.fsf@kyleam.com> From: Tom Gillespie Date: Sun, 6 Sep 2020 02:39:42 -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::443; envelope-from=tgbugs@gmail.com; helo=mail-wr1-x443.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=vQf+kmE/; 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: /hMAbwer4mbG Hi Kyle, 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, Tom On Sat, Sep 5, 2020 at 8:45 PM Kyle Meyer 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).