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 command enablement by using specific activation contexts for O#, Roslyn standalone, and Roslyn devkit #6782

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

dibarbet
Copy link
Member

@dibarbet dibarbet commented Jan 4, 2024

There were various bugs in the old enablement clauses. For example

  1. Devkit overrides the useOmnisharp configuration setting. If useOmnisharp was true and devkit was installed, O# commands were displayed (but would not work since devkit activated, not O#).
  2. Some of the restore commands were only enabled when using Roslyn standalone, but not using O#.

This PR modifies the enablement clauses to look at an activation context. We explicitly set the activation context after we've run all the other logic to determine which server to actually launch.

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1891108 (and other non-filed bugs)

@dibarbet dibarbet requested a review from a team as a code owner January 4, 2024 00:05
package.json Show resolved Hide resolved
@jasonmalinowski
Copy link
Member

@dibarbet Does this mean though that commands aren't visible until the extension has launched? For some that's probably not a big deal, but what about the ones that might trigger the server launch in the first place?

@dibarbet
Copy link
Member Author

dibarbet commented Jan 4, 2024

@dibarbet Does this mean though that commands aren't visible until the extension has launched? For some that's probably not a big deal, but what about the ones that might trigger the server launch in the first place?

@jasonmalinowski Kind of. For the Roslyn commands, that was already the case. We only set the standalone context when we launch the server, and that makes sense for the commands we expose from the new server (all of them require the server to be started in order to function).

For O# ones they are now only available after activation. But note the O# commands are still available before the server launch (so o.pickProjectAndStart will function normally if the extension is activated). In my opinion this is the correct experience because

  1. I cannot think of a motivating scenario where we wouldn't be active, but still want to trigger those commands (we already activate if there is any C# language file or csproj/sln/etc...)
  2. There is not a correct set of commands to show, because we have no idea how the extension is going to activate. For example o.pickProjectAndStart would not be correct to show if you had devkit installed.

src/lsptoolshost/roslynLanguageServer.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@dibarbet dibarbet merged commit 5946723 into dotnet:main Jan 5, 2024
13 checks passed
@dibarbet dibarbet deleted the fix_command_enablement branch January 5, 2024 22:42
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.

4 participants