search

emacs-ess / ESS

Emacs Speaks Statistics: ESS
https://ess.r-project.org/
GNU General Public License v3.0
626 stars 161 forks source link

Fix command hang with `{` expressions when `browser()` is active #1170

Closed lionel- closed 2 years ago

lionel- commented 3 years ago

When ess-command is passed a { expression and browser() is active, we get a hang because R steps through the components of{. This didn't happen before because we sent the expression directly. Now we evaluate it from the R function .ess.command() and this causes R to stop when evaluation returns to the debugged environment when the argument is forced.

To avoid this, I can't think of anything else besides running the command in local(). A nice effect of this change is that the ESS commands are now sandboxed by default. However this is a breaking change, for instance inspecting the current environment from an ESS command will now require ls(envir = parent.env(environment()). We could also provide something like .ess.command.env() to make this easier.

I think the breaking change is worth it because requiring users to wrap their { expressions in local() is error prone and is likely to result in ESS hangs when debugging.

lionel- commented 3 years ago

Added .ess.environment() to access current environment from ess-command.

I'm now thinking that if we're going to introduce a breaking change, we should go all the way and evaluate commands in a child of base env rather than in a child of the current env. This way we improve robustness as objects and functions defined by the user can't interfere with commands. But this means functions in other packages (utils in particular) must be namespaced with ::, which will cause trouble for a while as people update their commands.

lionel- commented 2 years ago

Another option is to create a clone of the utils package env that inherits from base. I think that's be a reasonable choice for the command environment, basically a private namespace with an import(utils) setting. Each command would be evaluated in a child so they can create bindings without littering the namespace. With both base and utils functions in scope, the breakages to existing commands should be limited.

However we need to find a solution for the ESS commands that use .ess.foo() utils. Those are currently attached to the search path and won't be in scope in such a command namespace. I'll keep thinking about this before drawing up a proposal.