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.

Sign up for GitHub

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

update R startup heuristic #1289

Merged
merged 5 commits into from
Sep 12, 2023
Merged

update R startup heuristic #1289

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
11144a2
update R startup heuristic
seeM Sep 11, 2023
85da650
fix casing
seeM Sep 11, 2023
a725cef
add more R files and qmd files with R code blocks to the R startup he…
seeM Sep 12, 2023
c4e0555
revert qmd R startup heuristic
seeM Sep 12, 2023
e93110d
Update extensions/positron-r/src/provider.ts
seeM Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions extensions/positron-r/src/provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,11 @@ export async function* rRuntimeProvider(context: vscode.ExtensionContext): Async
return semver.compare(b.semVersion, a.semVersion) || a.arch.localeCompare(b.arch);
});

// For now, we recommend R for the workspace if the user is an RStudio user.
// For now, we recommend the first R runtime for the workspace based on a set of
// non-runtime-specific heuristics.
// In the future, we will use more sophisticated heuristics, such as
// checking an renv lockfile for a match against a system version of R.
let recommendedForWorkspace = isRStudioUser();
let recommendedForWorkspace = await shouldRecommendForWorkspace();

// Loop over the R installations and create a language runtime for each one.
//
Expand Down Expand Up @@ -295,6 +296,38 @@ function binFragment(version: string): string {
}
}

// Should we recommend an R runtime for the workspace?
async function shouldRecommendForWorkspace(): Promise<boolean> {
// Check if the workspace contains R-related files.
const globs = [
'**/*.R',
'**/*.Rmd',
'**/.Rprofile',
'**/renv.lock',
'**/.Rbuildignore',
seeM marked this conversation as resolved.
Show resolved Hide resolved
'**/.Renviron',
'**/*.Rproj'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For performance, I think almost all of these should only be searched for at the workspace root since that's the only location at which we expect to find them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added **s in all cases to cater for users who open a monorepo at the repo root. My assumption re performance was that, in the worst case, we would still have to iterate through every file in the workspace to check the **/*.R pattern, so it shouldn't be a significant performance drop to add more ** patterns on top of that. Similarly, in the best case, we would match one of the ** patterns early, and the rest wouldn't matter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen a somewhat common pattern where such repositories will have separate sub-directories for different language bindings; e.g. you have a folder for R bindings, Python bindings, and so on. In this scenario, because we cannot really ascertain what the "right" default runtime is, I would argue it would be better to search at the workspace root.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking was that in that case we would start both an R and Python runtime. But if e.g you had multiple sub-directories with R projects using different R versions, I suppose we wouldn't know which specific R version to start, so I agree that we shouldn't set "immediate" startup behavior unless we're very certain. I'll make a follow-up PR.

];
// Convert to the glob format used by vscode.workspace.findFiles.
const glob = `{${globs.join(',')}}`;
if (await hasFiles(glob)) {
return true;
}

// Check if the workspace is empty and the user is an RStudio user.
if (!(await hasFiles('**/*')) && isRStudioUser()) {
return true;
}

return false;
}

// Check if the current workspace contains files matching a glob pattern.
async function hasFiles(glob: string): Promise<boolean> {
// Exclude node_modules for performance reasons
return (await vscode.workspace.findFiles(glob, '**/node_modules/**', 1)).length > 0;
}

/**
* Attempts to heuristically determine if the user is an RStudio user by
* checking for recently modified files in RStudio's state directory.
Expand Down