Skip to content

Conversation

@davidwengier
Copy link
Member

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1823464

We have file watchers, because we care about projects and things, and LSP has didOpen and didClose because LSP cares about documents. Sometimes these two things don't agree :)

We were removing all of our knowledge of a document when the file is deleted, which makes sense at first glance, but there is no rule in LSP that says a file that is open has to exist in the physical realm. This change makes us move an open file to our Misc Files workspace, instead of forgetting about it, if its open.

Matches Roslyn behaviour here: https://github.com/dotnet/roslyn/blob/1119057b76330c6ec80b2a8c04c7fb4973d48773/src/Features/LanguageServer/Protocol/Workspaces/LspWorkspaceManager.cs#L251-L257

@davidwengier davidwengier requested a review from a team as a code owner May 25, 2023 04:40
_ => throw new NotImplementedException(),
};
var message = formatter(state, exception);
if (message == "[null]" && exception is not null)
Copy link
Member Author

Choose a reason for hiding this comment

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

Snuck this one in, just because I saw it come up in the logs, and it wasn't exactly helpful. Not sure if its something on my machine, but whatever message formatter we're being passed here doesn't seem to use the exception at all.

Copy link
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

🚀

@davidwengier davidwengier merged commit 8175b43 into dotnet:main May 25, 2023
@davidwengier davidwengier deleted the FixDeleteFileIssue branch May 25, 2023 21:35
dibarbet pushed a commit to dibarbet/vscode-csharp that referenced this pull request Jun 8, 2023
Goes with dotnet/razor#8759 to fix #1823464
Won't be merged until I can update it with a Razor bump.

Essentially, when a file was deleted we were removing all our knowledge of the document, but with Green's delete behaviour, and VS Codes lazy `didClose` (and the fact that the file system exists) that could cause us to be in a bad state.

We already have code in place that calls `removeDocument` when a document is closed, if it isn't in the workspace.

Related work items: #1823464
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants