REditorSupport / REditorSupport/languageserver
Configurable source dirs for file parsing
Nobody has claimed this yet.
- Dominant language
- R
- Stars
- 675
- Forks
- 118
- Avg merge
- 1d 37m
- Merged PRs (30d)
- 11
Description
It is a well known issue that the languageserver only parses files other than those that it is notified are opened in the case that it detects the workspace root is a package.
https://github.com/REditorSupport/languageserver/issues/253
This means that the way that workspace symbols and definitions work are not how many people expect and seem to deviate a little from the protocol specification: https://microsoft.github.io/language-server-protocol/specification#workspace_symbol.
There is also a related issue that, even in open files, source() is not understood as something that affects the workspace namespace:
https://github.com/REditorSupport/vscode-r-lsp/issues/65
I believe a pretty cheap solution to this is to allow users to configure directories that the server should parse E.g. options(languageserver.r.source.dirs = c("R", "scripts") would cause workspace$load_all() parse R files in those two top level folders, even if it cannot identify the project as a package.
Another possibly nicer alternative is for the option to work as an exclusion list, so parsing starts at "." and any paths containing folders with names defined in the option are ignored. But there seems to a lot of inertia behind the "no parse by default" position.
The implementation would be pretty cheap:
in:
on_initialized <- function(self, params) {
logger$info("on_initialized")
project_root <- self$rootPath
if (length(project_root) && is_package(project_root)) {
# a bit like devtools::load_all()
self$workspace$load_all(self)
# TODO: result lint result of the package
# lint_result <- lintr::lint_package(rootPath)
}
workspace$load_all could take a vector of paths to parse.
E.g. workspace$load_all(self, source_paths)
So then we'd just need to build the vector of paths which is whatever languageserver.r.source.dirs says unioned with "R" in the case that the project is a package.
source_paths <- union(
getOption("languageserver.r.source.dirs", NULL),
if (is_package(project_root)) "R" else NULL)
)
The change to workspace$load_all is minimal too, since file.path and list.files are already vectorised:
load_all = function(langserver, source_paths) {
source_dirs <- file.path(self$root, source_paths)
files <- list.files(source_dirs, pattern = "\\.r$", ignore.case = TRUE)
for (f in files) {
logger$info("load ", f)
path <- file.path(source_dir, f)
uri <- path_to_uri(path)
doc <- Document$new(uri, NULL, stringi::stri_read_lines(path))
self$documents$set(uri, doc)
# TODO: move text_sync to Workspace!?
langserver$text_sync(uri, document = doc, parse = TRUE)
}
self$import_from_namespace_file()
},
I'd also argue for recursive = TRUE in list.files.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at on_initialized and workspace$load_all, then trace how project_root, is_package(), and getOption("languageserver.r.source.dirs") are used. Define the configuration behavior, pass the selected source paths into workspace$load_all, and verify that configured directories are parsed, with package R files included and recursive parsing handled as intended.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- r
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100