Skip to content

Fix EnableISEMode for Azure Data Studio #3735

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

Merged
merged 1 commit into from
Dec 21, 2021
Merged

Conversation

andyleejordan
Copy link
Member

Because ADS stripped out VS Code's debugger, the debug settings don't
exist, causing our update() call for the debug setting to throw an
error to the user when they run EnableISEMode. I couldn't find a
reliable way to test if the extension is running in the ADS fork instead
of the upstream VS Code, but a simple try {} catch {} that allows the
funciton to continue is a reliable fix, as we can't do anything about
this setting regardless.

Resolves #3709.

Because ADS stripped out VS Code's debugger, the `debug` settings don't
exist, causing our `update()` call for the debug setting to throw an
error to the user when they run `EnableISEMode`. I couldn't find a
reliable way to test if the extension is running in the ADS fork instead
of the upstream VS Code, but a simple `try {} catch {}` that allows the
funciton to continue is a reliable fix, as we can't do anything about
this setting regardless.
@ghost ghost added the Issue-Bug A bug to squash. label Dec 21, 2021
@andyleejordan
Copy link
Member Author

/cc @Charles-Gagnon

@andyleejordan andyleejordan merged commit 2384b7c into master Dec 21, 2021
@andyleejordan andyleejordan deleted the andschwa/fix-ads branch December 21, 2021 01:59
@Charles-Gagnon
Copy link

Thanks for fixing @andschwa !

There's a couple options for determining if it's running in ADS :

  1. You can check at runtime whether the azdata API object exists - such as what we do here : https://github.com/microsoft/azuredatastudio/blob/main/extensions/data-workspace/src/common/utils.ts#L54
  2. vscode.env.appName is another option (it should be Azure Data Studio for ADS) Note though that this may not be the best option as other versions of the app such as insiders may have a slightly different name.

Although unless you find another case that you can't easily solve like this one it's probably easiest to just leave it as this. Although I would suggest adding a log statement or something in the catch so you can see what settings were ignored in case a setting is erroring that you don't expect.

@andyleejordan
Copy link
Member Author

Thanks so much @Charles-Gagnon, you're always so helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UI Issue-Bug A bug to squash.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Enable ISE Mode command fails in Azure Data Studio
2 participants