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 Intellisense and other features of Bicep in Visual Studio that were broken by VS 17.10 #14532

Merged
merged 9 commits into from
Jul 17, 2024

Conversation

StephenWeatherford
Copy link
Contributor

@StephenWeatherford StephenWeatherford commented Jul 11, 2024

Fixes #14316

VS 17.10 added support for language server capabilities. This means that it started firing notifications only for events that were explicitly requested by the client (which is normal and works in vscode). Unfortunately, their LSP implementation lacks support for document selectors based on language ID (they only support file patterns, language/scheme are NYI). This means it didn't recognize any of our registrations and VS was giving us virtually no notifications, e.g. for file changes. The fix is to use file patterns for VS, language/scheme for any other hosts including vscode.

I fixed this such that it should only affect the behavior in VS, not VsCode or other hosts, out of an abundance of caution (it would be possible to return language/scheme as well as file patterns, but it does cause minor behavior changes in vscode and would also mean VS behavior would change in minor ways again after this fix this problem - better to just make the behavior explicit for both hosts).

The actual fix is in DocumentSelectorFactory, almost all of the rest is just passing along the dependencies it needs to know when to apply the fix.

Copy link
Contributor

github-actions bot commented Jul 11, 2024

Test this change out locally with the following install scripts (Action run 9965886474)

VSCode
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 9965886474
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 9965886474"
Azure CLI
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 9965886474
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 9965886474"

Copy link

@AbhitejJohn AbhitejJohn left a comment

Choose a reason for hiding this comment

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

Do we have tests/manual validation workflow to let us know early if this breaks in the future? I'm also curious if there is a set of partner tests/a process we can use so folks can reach out to us before these are broken. Even if that is us subscribed to the right breaking change notifications, that would be helpful as well.

Copy link
Contributor

github-actions bot commented Jul 11, 2024

Dotnet Test Results

    72 files   -     36      72 suites   - 36   24m 5s ⏱️ - 8m 56s
10 955 tests  -     20  10 955 ✅  -     20  0 💤 ±0  0 ❌ ±0 
25 806 runs   - 12 899  25 806 ✅  - 12 899  0 💤 ±0  0 ❌ ±0 

Results for commit 7a83378. ± Comparison against base commit 9dc6238.

♻️ This comment has been updated with latest results.

@StephenWeatherford
Copy link
Contributor Author

Do we have tests/manual validation workflow to let us know early if this breaks in the future?

@AbhitejJohn The manual weekly testing will easily catch this. Right now the section on completion notes that it's broken. I'll be updating and also adding a few more simple scenarios. Hopefully I can get the automated testing back online, soon, too.

Copy link
Member

@anthony-c-martin anthony-c-martin left a comment

Choose a reason for hiding this comment

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

See comments

Copy link
Member

@anthony-c-martin anthony-c-martin left a comment

Choose a reason for hiding this comment

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

LGTM!

@StephenWeatherford StephenWeatherford merged commit 76a95dc into main Jul 17, 2024
44 checks passed
@StephenWeatherford StephenWeatherford deleted the sw/fix-vsbicep-langserver branch July 17, 2024 00:10
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.

Bicep for Visual Studio Extension No Longer Works with Visual Studio 17.10.2
4 participants