ark: Should Console code execution trigger a full regeneration of document diagnostics?

[DavisVaughan]

Currently we only regenerate diagnostics within a document if a change is made within the document itself.

But changes at the console can be highly entangled with document diagnostics.

For example, look at this case where inform() (from rlang) isn't available until I run load_all() on the R package I'm working on, which does bring the rlang namespace into session scope. Even though running load_all() at the console technically brings inform() into scope, I don't actually see that reflected in the diagnostics until I go up and make a change to the document itself (which finally reruns its diagnostics).

Screen.Recording.2023-08-09.at.4.08.21.PM.mov

[juliasilge]

One thing I notice when working on a TypeScript project in a vscode workspace is that changes in one document propagate out to diagnostics for other documents; if I do something bad in one file, it shows me problems I have caused in another file. An alternative to thinking about executing code at the console triggering diagnostics would be using the files in the workspace to do diagnostics. I like that more because what is in the other documents is more "real" than work in the console, and it is more aligned with vscode workspace behavior for other kinds of projects.

To be more concrete for this example, maybe the workspace should know that this is an R package, and be able to use what is in another document (in this case #' @import rlang in vetiver-package.R) for diagnostics.

[DavisVaughan]

changes in one document propagate out to diagnostics for other documents

We maintain a list of open documents, so one approach could be:

• For the primary visible open document, on any change we enqueue diagnostics with our standard 1 second delay
• For the secondary open documents (open tabs, but not visible), on any change to the primary document we enqueue diagnostics with a 3-4 second delay
• Relatedly, for any code executed in the Console, we:
  • Enqueue diagnostics for the primary document with a 1 second delay
  • Enqueue diagnostics for the secondary documents with a 3-4 second delay

The longer delay hopefully avoids any performance issues from running diagnostics on a larger number of files.

[juliasilge]

Recent changes have fixed the situation around diagnostics a lot; see #997 and #999.

I want to lay out how the experience now works for further consideration. If I open my package totally fresh and starting editing a test file, I see all the testthat functions identified as problems:

If I run library(testthat) in the console, nothing changes.

If I then go back to the file and starting editing again, the testthat problems then resolve:

I can get the rest of those problems to resolve by doing devtools::load_all() and then editing the file.

There are two aspects of this that still need addressing:

• It will be surprising to folks that they must do devtools::load_all() and also edit the file (with a likely fake edit) to get the diagnostics to re-run.
• Just even to start with, it's more common to edit files and then do something like devtools::load_all() to test out changes (I don't think I've ever heard or seen someone say, "OK, time to start my package development work!" and then first thing do devtools::load_all())

[jennybc]

To expand on this a bit:

To be more concrete for this example, maybe the workspace should know that this is an R package, and be able to use what is in another document (in this case #' @import rlang in vetiver-package.R) for diagnostics.

It feels like NAMESPACE should be take into consideration from the very start, which would solve the rlang::inform() problem. And it feels like a testthat-using package should be detected/detectable, in which case symbols in the testthat namespace should never be flagged.

So, I get that this issue might be mostly about regenerating document diagnostics. But for these two examples, it feels like we should be able to get it right from the very start, i.e. has nothing to do with regenerating diagnostics.

[DavisVaughan]

Some key action items:

• Need to be able to do per file diagnostics again (Implement did_close() and per-file diagnostics ark#81)
• Need a way to refresh diagnostics for open files after input is executed in the console
• On save, refresh all open (or more?) file diagnostics (?) (in theory we want a function name change in one file to show up as an "symbol not found" issue in another file that needs to be resolved, rust is very good at this)
• Parse NAMESPACE if in a package for import() and importFrom() directives and preemptively add those package symbols to the diagnostics database (so opening a package before running load_all() doesn't look horrible)
• Parse DESCRIPTION for usage of testthat to preemptively add testthat symbols to diagnostics database (so opening a testthat file before running load_all() doesn't look horrible)

[DavisVaughan]

posit-dev/ark#224 fixes the exact problem that this issue was opened for (i.e. the diagnostics should update after running something in the Console). I have moved the additional comments (which are good ideas) into #2252

[juliasilge]

NICE 🤩

diagostics.mov

In Positron 2024.02.0 (Universal) build 1535, I see diagnostics update after I execute code in the console.