r-lib / devtools

Tools to make an R developer's life easier
https://devtools.r-lib.org
Other
2.37k stars 753 forks source link

Adjust to new RStudio version numbering #2410

Closed jennybc closed 2 years ago

jennybc commented 2 years ago

Closes #2397

As per @jgutman advice:

Also makes this helper much less likely to throw an error, even if it does not succeed.

jennybc commented 2 years ago

Would appreciate a quick :eyes: @romainfrancois and/or @jgutman (I can't officially request a review from you).

Do you foresee any negative consequences of this?

jennybc commented 2 years ago

@jgutman can you give me concrete examples of the different version number forms I need to handle?

jennybc commented 2 years ago

The quoting here is a surprise:

$version
[1] ‘1.4.1717’
jennybc commented 2 years ago

It looks like rstudioapi::versionInfo()$version is an instance of c("package_version", "numeric_version"), whereas $long_version is a simple string. So I guess it needs an as.character().

jennybc commented 2 years ago

I'm under the impression that I can't / really shouldn't use RStudio.Version() because it's made available in an RStudio context via some sort of shim, whereas it's much safer to always go through rstudioapi.

jgutman commented 2 years ago

@jgutman can you give me concrete examples of the different version number forms I need to handle?

An older version https://dailies.rstudio.com/rstudio/juliet-rose/desktop/macos/ Returns $version [1] ‘1.4.1717’ but no long_version available

A newer release version https://dailies.rstudio.com/version/2022.02.0+443/ $version [1] ‘2022.2.0.444’ $long_version [1] "2022.02.0+444"

A preview version https://dailies.rstudio.com/version/2022.02.0-preview+391/ $version [1] ‘2022.2.0.391’ $long_version [1] "2022.02.0-preview+391"

A daily version https://dailies.rstudio.com/version/2021.09.0-daily+328/ $version [1] ‘2021.9.0.328’ $long_version [1] "2021.09.0-daily+328"

jgutman commented 2 years ago

I'm under the impression that I can't / really shouldn't use RStudio.Version() because it's made available in an RStudio context via some sort of shim, whereas it's much safer to always go through rstudioapi.

Probably right, rstudioapi::versionInfo() will be equivalent!

jennybc commented 2 years ago

Here's how the IDE does this:

https://github.com/rstudio/rstudio/blob/main/src/cpp/session/modules/SessionUpdates.R

It gets called with manual = TRUE, secure = TRUE. Or at least that seems to be a safe assumption for me, in terms of updating here.

jennybc commented 2 years ago

OK this was more than I bargained for, but I think it's done.