Go to definition in another file

[lyh970817]

I'm wondering if it would be possible to set up go to definition for function definitions in another file?

[renkun-ken]

How are the files related to each other?

If your workspace is a package, it is already the current behavior. Otherwise, you need to open these files to make languageserver aware of all function definitions and their locations.

In the future, we may link multiple files by scaning source() calls as suggested in #20.

[lyh970817]

Would be possible to set it up for files in the same git directory?

[renkun-ken]

If the git repo contains too many files, then it would be too slow to parse all files, just like my use case at #15 (comment).

[MilesMcBain]

I really like the idea of the git repo as definition context.

Most of my projects are individual analyses of a few thousand lines of code. I could definitely be biased but I suspect this size is more common than the 70K LOC R code bases described in #15. In my experience, things tend to get broken up into packages before reaching that size.

So while I agree this could work poorly for very large projects, I suspect it would work quite well for most people.

Also I wonder do all the files need to be parsed? They just need to be searched for a few patterns? This would substantially increase the size of projects that could work smoothly.

I've seen that strategy work quite well over in Emacs land with dumb-jump. It uses very fast search tools to find specific patterns relating to definitions. The downside is dependency on those tools (The Silver Searcher, or ripgrep). Using it with R over a number of years, it's always just worked.

A convention I've seen over a few similar implementaitons is to ignore things in the .gitignore, and that would protect against renv local libraries being parsed, which I think would be the main footgun of the git repo approach.

Perhaps the LSP could expose a setting that disables repo-wide search for definitions?

[renkun-ken]

Another thing to consider is that if a repo is a collection of analysis scripts, then it is very likely that they are only loosely related to each other and a symbol could be defined many times in different scripts. If this is the case, then the last file loaded to contain the definition of a particular symbol would overwrite the previous definition of that symbol with current implementation of workspace symbol definition. Then the order to load files will matter in this case.

[MilesMcBain]

I just realised I can implement a dumb-jump style thing fairly cheaply as an addin, and it will work with VSCode now ;)

[MilesMcBain]

I was thinking about the multiple match issue and I think some sort of hierarchy could help, e.g:

same file preferred over same folder, preferred over any other folder.

I've been able to make an implementation with addins work quite well. For now it's living in https://github.com/MilesMcBain/fnmate/blob/master/R/rstudio-addin.R#L105

But I think I will refactor some stuff out of that package and move the jump to it's own package.

[andycraig]

Most of my projects are not a single package so I'm keen for a bit more flexibility here too. Another option might be file-based inclusion/exclusion. For example, the language server parses all .R files unless the root folder contains a file .Rignore. Or the opposite: the language server only parses all .R files if the root directory contains a file .Rindex. Yet another option could be to add a property to .Rproj (e.g., Index: false).

If we go with one of these options, ignoring everything in .gitignore as well is a good idea.

I don't have any data but I'm inclined to agree with @MilesMcBain that most projects are probably 'small', and I think the current behaviour of not parsing all the files is a bit surprising (e.g., this issue was created). Parsing by default, and opting out by placing an .Rignore file, is my personal preference.

[ManuelHentschel]

When packages are installed from source with the flag --with-keep.source, the functions from that package contain the attribute srcref which points to the original source file of the function. Would it be possible to check for this attribute and open the file (if it still exists), otherwise falling back to the current use of temp files?

I would expect this to be rather efficient since the srcref already contains the full filename and line/column in the file, so the language server wouldn't need to do much parsing (except for maybe verifying the location, in case the file has been modified after the installation).

[karlvurdst]

First of, thank you for a very good extension and all your hard work!

Is there any update on this? I have a repo with several packages, and it is very nice to be able to quickly go to a function, but currently it only works if I have the specified .R file open, and we have 100s of .R files.

[renkun-ken]

I have a repo with several packages

You mean a workspace folder that contains multiple R packages as subfolders?

[karlvurdst]

I have a repo with several packages

You mean a workspace folder that contains multiple R packages as subfolders?

Well, yes.

[renkun-ken]

Maybe we could support some kind of config file per project under the workspace folder to tell languageserver to include/exclude files that match a list of regex or glob patterns.

[karlvurdst]

That would be excellent, I would not need something that searches my whole computer, it would suffice to search my workspace. I have no knowledge of what you do and how difficult it is. But is it possible to have languageserver by default search workspace folders, and maybe as you say, a config file where users may put additional paths if they really need?

I know this is additional work for you, but I sadly don't have the skills to help ...

[renkun-ken]

Introducing a separate file for per-project config for languageserver might be unnecessary. Does it make sense to support reading language server include/exclude from {workspaceFolder}.Rproj:

Version: 1.0

RestoreWorkspace: Default
SaveWorkspace: Default
AlwaysSaveHistory: Default

EnableCodeIndexing: Yes
UseSpacesForTab: Yes
NumSpacesForTab: 2
Encoding: UTF-8

RnwWeave: Sweave
LaTeX: pdfLaTeX

AutoAppendNewline: Yes
StripTrailingWhitespace: Yes

BuildType: Package
PackageUseDevtools: Yes
PackageInstallArgs: --no-multiarch --with-keep.source
PackageCheckArgs: --as-cran
PackageRoxygenize: rd,collate,namespace

LanguageServerInclude: R, tests
LanguageServerExclude: inst, tests/testthat

Then we use the results from the following code:

setdiff(
  list.files(c("R", "tests"), "\\.[rR]$", full.names = TRUE, recursive = TRUE),
  list.files(c("inst", "tests/testthat"), "\\.[rR]$", full.names = TRUE, recursive = TRUE)
)

[karlvurdst]

Sounds good, I'm not sure that I have a .Rproj file for each of my .code-workspace, but I would definetly create one so that the "Go to definition" works for my whole project.

[pat-s]

AFAICS this is already working as of today by using "Go to Definition"? I've just tried it on a function definition of a package and it opened the definition which actually lives in another .R file in the active package.