Skip to content

Conversation

darichey
Copy link
Contributor

We subscribe to textDocument/didSave for filesToWatch, but the VFS doesn't contain those files. Before #18105, this would bring down the server. Now, it's only a benign error logged:

ERROR notification handler failed handler=textDocument/didSave error=file not found: /foo/bar/TARGETS

It's benign, because we will also receive a workspace/didChangeWatchedFiles for the file which will invalidate and load it.

Explicitly include the buildfiles in the VFS to prevent the handler from erroring.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 26, 2024
@darichey darichey force-pushed the read-buildfile-into-vfs branch from 2924b50 to 85ca217 Compare September 26, 2024 16:55
Copy link
Contributor

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

LGTM, but I won't bors this for conflict of interest reasons.

@Veykril
Copy link
Member

Veykril commented Sep 27, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Sep 27, 2024

📌 Commit 85ca217 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 27, 2024

⌛ Testing commit 85ca217 with merge 546339a...

@bors
Copy link
Contributor

bors commented Sep 27, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 546339a to master...

@bors bors merged commit 546339a into rust-lang:master Sep 27, 2024
11 checks passed
@lnicola lnicola changed the title Include buildfiles in VFS internal: Include buildfiles in VFS Sep 28, 2024
@darichey darichey deleted the read-buildfile-into-vfs branch September 30, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants