Hi Bastien, In short, we need two variables due to the case where a user wants to set nil for all vars and a function for code. More replies in line. Best! Tom
> I see nothing in etc/ORG-NEWS that describes this change: am I missing > something? Looks like any mention of the change is missing to me as well. > >> Whether it changed or not, what is the problem in 9.6? > > > > The problem is that Org now displays more queries. > > > >> How does the patch solves this problem? > > > > It allows disabling these new queries about lisp evaluation outside > > code blocks without disabling code block eval confirmation completely. > Is there a real use-case for this? Yes. To give one example. I have many files that need to run hundreds of cells at tangling/export time. I have reviewed all the code in the cells and know patterns that are safe and wont' burn me. I do not want to allow execution of code blocks blindly during tangling and confirm eval is a secondary line of defense against running those blocks during export. I want to allow the cells to run uninhibited as they did before this change, so that I don't have to fight with org during the tangling process. As it is right now the change has completely broken various CI pipelines for me, and if these changes do not get in to emacs 29 in time I will be unable to use emacs 29 for CI until they are in the base image without having to resort to insane hacks and massive additional configuration changes to work around the issues discussed in the loading multiple org versions thread. > It looks like the patch fixes the "too many queries" problem by > providing a setup that is similar to what was the previous default > (i.e. only asking about code block evaluation when > org-confirm-babel-evaluate-cell is set to nil.) This interpretation is not quite right. org-confirm-babel-evaluate-cell has no interaction with evaluating code blocks, only with evaluating cells. When org-confirm-babel-evaluate-cell is nil or evaluates to nil then the org-babel-confirm-evaluate code runs on a fake block that is created out of the cell. (This is where there is still a bug that I mentioned up thread.) > But are we sure that users need to confirm header args evaluation > separately from code blocks evaluation? I am sure that in my workflow I want them split, especially for global nil/t behavior. I need to think a bit more about your suggestion to add more values to org-confirm-babel-evaluate to see whether it might work and what the relative complexity would be. It seems to me that duplicating the variable is certainly easier to maintain, if not to explain to those who do not use babel regularly. I would be interested to hear from other babel users their thoughts on the design, because it seems obvious to me, but again I wrote the code so am the last person whose view on the clarity of the should be considered. It seems that it was not clear to Max initially. > IOW I have difficulties understanding when these would be relevant: > (setq org-confirm-babel-evaluate-cell nil) Anywhere that a parenthesized elisp expression occurs that is not in #+begin_src block. > (setq org-confirm-babel-evaluate t) Only inside #+begin_src blocks. > I think that's the best route: providing, optionnally, more, while not > annoying users who don't want more. I think this is probably the right default as well. > > Yes. Ideally, we need to improve the code evaluation query. It should > > allow confirming evaluation in bulk and add some code blocks/files to > > whitelist. Similar to `org--confirm-resource-safe'. > > I see, indeed. Improving general code auditing prior to evaluation of blocks would be greatly appreciated. The fighting between the minibuffer and the primary buffer for priority and the fact that you often cannot see that code that will be run is a significant issue. However, this is orthogonal to the issue at hand. > > A concern have been expressed that more queries may annoy users and > > drive them towards setting `org-confirm-babel-evaluate' to nil globally. > > Upon doing this, the future more flexible security queries may be not > > used by such users. > > The future more flexible security queries will be made visibile enough > so that users currently setting `org-confirm-babel-evaluate' to nil > will *want* to have a look at it. Yep. We need to get the fundamentals in place to enable those workflows. I'm too paranoid to simply set that variable to nil, even when I'm only running code that I wrote, but I suppose that is not the case for everyone. > I find "cell" confusing here (as Max said earlier in the thread). It > either refers to a table cell or, in Elisp jargon, a "cons cell" (or a > "function cell"). I also dislike the name cell, but i'm not sure what to call it instead. I mentioned "parenthized elisp expression" above, since strictly speaking they aren't closures, they are any valid elisp sexpression. In the context of org-outside-emacs I imagine a day when those top level expressions might also be written in some other language and there would be some way to indicate at a file or section level or perhaps even single block expression level what language a given cell should be interpreted as. In the context of naming this means that "parentized expression" might not be general enough. Maybe embedded-code or embedded-expression? org-confirm-babel-evaluate-embedded-expressions? > What about modifying `org-confirm-babel-evaluate' to allow these values: > > - t : confirm header vars *and* code blocks > - 'code : confirm code blocks > - 'vars : confirm vars > - nil : don't confirm > > and set the default value to 'code, while allowing concerned users to > set it to `t' -- until we have a better system for evaluation query. > > WDYT? In short this is not a viable solution because there is no way to compose nil for embedded expressions with a function for blocks. We really do not want to change the function signature for the function to also have to accept whether it is a block or an embedded expression. That will break code for everyone. The solution I have proposed keeps blocks and embedded expressions orthogonal precisely to avoid these kinds of major disruptions. Better for users to be confused about how to use a variable but be able to learn about it or ask about it than for all users of a feature to have to suffer major breakages to their existing workflows and have to rewrite existing code to work around something that supposedly increased security. Having two separate variables is the only way to retain backward compatibility and give granular control over evaluation of code blocks and embedded expressions.