-
Notifications
You must be signed in to change notification settings - Fork 128
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
Tweak settings #1235
Tweak settings #1235
Conversation
ElianHugh
commented
Oct 24, 2022
- Add vsc.dev.args setting
- Add enum descriptions to r.source.focus
- Minor stylistic changes to item descriptions
- Add vsc.dev.args setting - Add enum descriptions to r.source.focus - Minor stylistic changes
R/session/vsc.R
Outdated
@@ -28,6 +28,7 @@ load_settings <- function() { | |||
vsc.object_timeout = session$objectTimeout, | |||
vsc.globalenv = session$watchGlobalEnvironment, | |||
vsc.plot = setting(session$viewers$viewColumn$plot, Disable = FALSE), | |||
vsc.dev.args = c(width = plot$devArgs$width, height = plot$devArgs$height), |
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.
The option is used as following:
dev_args <- getOption("vsc.dev.args")
do.call(png, c(list(filename = plot_file), dev_args))
Maybe it makes more sense to just keep it as a list and not just limited to width
and height
.
"default": 1200 | ||
} | ||
}, | ||
"additionalProperties": false |
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.
Maybe more arguments could be allowed here.
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.
Ah, seeing how it leads to a call to png, that's a good point. If we remove the restriction, the settings UI will fallback to the "edit in JSON" dialog. I'm not sure if that's ideal? I think there's an argument to be made that if dots is required, people can use the R option. Alternatively, we could have '...' dots as an item for passing further args
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 this one is good enough. Let's revisit if somebody complains.
- Keep devArgs as a list - Width + height defaults set to 480 - Add some more options to devArgs Pending discussion on args + settings UI
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.
LGTM