Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix code actions that add files to the workspace #751

Merged

Conversation

DustinCampbell
Copy link
Contributor

Fixes dotnet/vscode-csharp#975

This PR cleans up a lot of the code for running code actions. It also tries to add some defensive code around a strange interaction in the '/v2/runcodeaction' end point. Essentially, this end point can either go ahead apply the code action to the workspace or supply the file edits needed to apply the code action. However, in the cases of code actions that add files, this is weird.

When file edits are requested for a code action that adds files, we add an empty file with that name to the workspace. This is very weird and could potentially leave the workspace in a bad state where an empty file was added if the editor host requesting the file edits never actually applies them. This needs revisiting in the future, but doing so now would potentially break editors.

History aside, the actual bug is that the endpoint tried to remove the document from the workspace before adding it. This throws an exception if the document doesn't already exist, which causes the end point fail. This code has been in OmniSharp for a long time, so this either didn't already work or the exception was added to Roslyn at some point. Now, we no longer try to remove it as that doesn't really make sense since a document is being added.

@DustinCampbell DustinCampbell merged commit 7af625e into OmniSharp:dev Feb 5, 2017
@DustinCampbell
Copy link
Contributor Author

Thanks!

@filipw
Copy link
Member

filipw commented Feb 5, 2017

History aside, the actual bug is that the endpoint tried to remove the document from the workspace before adding it. This throws an exception if the document doesn't already exist, which causes the end point fail. This code has been in OmniSharp for a long time, so this either didn't already work or the exception was added to Roslyn at some point.

looks like it has never worked in OmniSharp properly 😄 https://github.com/dotnet/roslyn/blob/version-1.0.0/src/Workspaces/Core/Portable/Workspace/Workspace.cs#L648

@DustinCampbell DustinCampbell deleted the fix-code-actions-that-add-files branch February 8, 2017 01:26
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.

3 participants