Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First draft of scope_() #33

Closed
wants to merge 1 commit into from
Closed

First draft of scope_() #33

wants to merge 1 commit into from

Conversation

krlmlr
Copy link
Member

@krlmlr krlmlr commented Nov 3, 2015

Fixes #28 (eventually).

@jimhester: That was easy.

@kevinushey: Thanks for your package. Try, for instance, scope_(setwd). Any plans for CRAN release yet? ;-)

@krlmlr krlmlr mentioned this pull request Nov 3, 2015
@jimhester
Copy link
Member

@krlmlr you will have to explicitly attach devtools to get Remotes: to work on travis with CRAN devtools. Due to r-lib/devtools#914

@kevinushey
Copy link

That's awesome! Some definite wizardry going on in the scope_ definition 😄

I'm still waiting a bit to convince myself that the defer_parent implementation will work as expected in all contexts; I think I will let it 'bake' for a bit longer and submit after that.

@krlmlr
Copy link
Member Author

krlmlr commented Nov 3, 2015

Thanks. Do you think it's better to add .scope_env = parent.frame() to the formals of the generated function, instead of relying on defer_parent?

@kevinushey
Copy link

Sorry, I'm a bit foggy on what you mean -- can you elaborate a bit more?

@krlmlr
Copy link
Member Author

krlmlr commented Nov 4, 2015

Currently:

> scope_(setwd)
function (dir) 
{
    old <- setwd(dir = dir)
    later::defer_parent(setwd(old))
    old
}

But it could be:

> scope_(setwd)
function (dir, .scope_env = parent.frame()) 
{
    old <- setwd(dir = dir)
    later::defer(setwd(old), envir = .scope_env)
    old
}

@jimhester
Copy link
Member

Ended up merging this manually (54becef) and doing the rest of the work to get it all the functions integrated + tests.

It mostly uses this implementation plus @krlmlr's suggestion to use .scoped_envir, I also ended up calling the functions scoped_ as that sounds a little nicer to me.

@kevinushey Were you planning on submitting later to CRAN soon, if so we can coordinate a withr release as well.

@jimhester jimhester closed this Nov 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants