Provide `local_bindings()`?

[hadley]

[lionel-]

We'd reexport rlang::local_bindings()? Currently withr does not depend on rlang.

To properly do and undo the bindings, we need zap support to represent missing bindings, so I think this non-trivial functionality should have only one implementation and live in rlang.

In the previous discussion in #87 we decided to keep it in rlang only.

[hadley]

It's weird that it's the only local_ function that lives in rlang, and I just hit a package (evaluate) where it'd be nice to use it (it doesn't have an rlang dep)

[lionel-]

rlang has a bunch of local functions:

export(local_bindings)
export(local_error_call)
export(local_interactive)
export(local_options)
export(local_use_cli)

Wouldn't evaluate benefit from adding a dependency on rlang?

[hadley]

I realised that I don't actually like the function signature of rlang::local_binding() because you almost always call it with an environment:

# e.g. instead of this:
local_bindings(is_testing = FALSE, .env = the)

# it would be nicer to write
local_bindings(the, is_testing = FALSE)

[lionel-]

We could move .env up front without breaking the signature

[hadley]

Oh yeah that's true. Still I think we should also provide in withr; probably just via copy and paste.

[lionel-]

Unfortunately it can't be copy/paste, local_bindings() wraps aroung env_bind() which is implemented in C.

If reimplementing in R, worth noting there's some subtle behaviour regarding the return value of local_bindings(). The current state of bindings are returned, and when missing they are assigned to an rlang::zap(). The zap sentinel is a singleton and creating it without going through the constructor is going to cause issues with rlang C code which assumes it can compare sentinels by pointer.

Solutions:

• We don't return old from withr::local_bindings() but that'd look like a bug/omission in the context of other local_ functions.
• If we switch the rlang zap sentinel to a symbol, we'd be able to validly create it from other packages. We then end up with two completely different impls of local_bindings() but at least they're compatible.
• We keep local_bindings() in rlang because it's an extension of env_bind().

[hadley]

IMO we don't need to return old from local_bindings(), since I don't think people actually use the return values. And I looked at a couple of withr functions and it doesn't seem like the return values are even documented.