-
Notifications
You must be signed in to change notification settings - Fork 23
Set .Platform$GUI to "Positron" in console sessions only
#977
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
Conversation
crates/ark/src/interface.rs
Outdated
| // Set .Platform$GUI to "Positron" for console sessions only | ||
| if session_mode == SessionMode::Console { | ||
| let result = harp::parse_eval_global(r#" | ||
| unlockBinding(".Platform", baseenv()) | ||
| .Platform$GUI <- "Positron" | ||
| lockBinding(".Platform", baseenv()) | ||
| "#); | ||
| match result { | ||
| Ok(_) => log::info!("Set .Platform$GUI to 'Positron' for console session"), | ||
| Err(err) => log::warn!("Failed to set .Platform$GUI: {err:?}"), | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what the best approach is, but this one is at least easy to reason about.
Some other approaches I tried out included:
- Do something like
r_symbol!(".Platform")in Rust and find theGUIelement. I like that this doesn't involve executing R code during some initialization step, but you still have to lock/unlock (just at the C level). - Do the locking/unlocking in
startup.R. This feels like a better place to run such R code, but I'm not sure we can check session mode there and the other options there are more user-facing.
RStudio does it here, which is not exactly the same but similar in that in happens post-initialization:
https://github.com/rstudio/rstudio/blob/5381a9a4363eae00007fd170302fa8a5c93050b5/src/cpp/r/R/Options.R#L126
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer this to be in an R file, maybe in modules/positron/positron.R. Then you can use:
.Platform <- base::.Platform
.Platform$GUI <- "Positron"
env_bind_force(baseenv(), ".Platform", .Platform)
lionel-
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably need to set GUI to something else than "Positron" when Ark runs with another frontend. To determine that, we could detect whether Sys.getenv("POSITRON_VERSION") is defined. If not, we'd set GUI to "Ark". What do you think?
crates/ark/src/interface.rs
Outdated
| // Set .Platform$GUI to "Positron" for console sessions only | ||
| if session_mode == SessionMode::Console { | ||
| let result = harp::parse_eval_global(r#" | ||
| unlockBinding(".Platform", baseenv()) | ||
| .Platform$GUI <- "Positron" | ||
| lockBinding(".Platform", baseenv()) | ||
| "#); | ||
| match result { | ||
| Ok(_) => log::info!("Set .Platform$GUI to 'Positron' for console session"), | ||
| Err(err) => log::warn!("Failed to set .Platform$GUI: {err:?}"), | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer this to be in an R file, maybe in modules/positron/positron.R. Then you can use:
.Platform <- base::.Platform
.Platform$GUI <- "Positron"
env_bind_force(baseenv(), ".Platform", .Platform)|
Ah, I am experimenting and it looks like the |
|
This is a much better approach. Thanks, @lionel-! |
| .Platform <- base::.Platform | ||
| .Platform$GUI <- "Positron" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it "Ark" if Sys.getenv("POSITRON") is not set? Or leave the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just so I understand, files in modules/positron get evaluated outside of Positron? All the options in options.R, etc? Can you explain a bit more about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think #588 is about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just so I understand, files in modules/positron get evaluated outside of Positron? All the options in options.R, etc? Can you explain a bit more about this?
They do because some of the files in there provide essential Ark functionality, for instance regarding error handling.
Maybe the folder really should be called ark? Or maybe we should split ark from the positron folder and the latter would only get sourced in Positron? That would be a nice way to easily differentiate generic and positron-specific functionality.
We could then have a .ark. prefix in addition to the .ps. prefix, for symbols exported from the ark folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went ahead and checked the env var in 1b11568, ahead of improvements like that.
Co-authored-by: Lionel Henry <lionel.hry@proton.me>
Addresses posit-dev/positron#9933
With this change, I see results like this in the Positron console:
And results like this in an R session running in an integrated terminal in Positron (the same as RStudio):
Does
SessionMode::Consolereally just mean only in Positron? Is there some better way for ark to know when it's running in Positron at initialization?