Skip to content

Conversation

corivera
Copy link
Member

This change opens the Azure Data Studio connection dialog when using an input token for the connection string in the connect magic command. Since we need to use some ADS-specific extension methods here, I moved the input handling into the versionSpecificFunctions files so that we can import azdata without breaking the vscode extension.

result = await azdata.connection.getConnectionString(connection.connectionId, true);
}
} else {
result = (inputTypeHint === 'file')
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be ideal if this block wasn't repeated code, e.g. a map of type hint to implementation that the ADS code could just add a handler to. The code is simple now but additional use of type hints is planned and it will get more complex.

Copy link
Member Author

Choose a reason for hiding this comment

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

Where would that input handler need to be added? Would it be another RequestInputType command handler that gets registered from the version specific code?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't need to be a kernel command handler, but just something the kernel calls into when handling an input request. If you add custom handlers there for the hinted type, then use the custom experience. Otherwise, return a value that allows the central handler to fall back to the default behaviors (i.e. currently "file" and "text").

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a new handler

@colombod colombod merged commit 02adb6a into dotnet:main Sep 29, 2022
@corivera corivera deleted the corivera/connectionInput branch September 29, 2022 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants