Adding support for default connections#22140
Open
Benjin wants to merge 2 commits into
Open
Conversation
Benjin
requested review from
aasimkhan30,
allancascante,
caohai,
kburtram,
laurenastrid1,
lewis-sanchez and
ssreerama
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces the boolean mssql.transferActiveEditorConnections setting with a tri‑state mssql.newEditorConnectionBehavior enum (transferActive | none | defaultConnection) and adds a paired mssql.defaultConnectionId setting so new SQL editors can auto‑connect to a designated saved connection. The old setting is auto‑migrated on activation and marked deprecated; an invalid defaultConnection configuration triggers a one‑shot prompt to fix it.
Changes:
- New settings + enum, deprecation message, and migration logic from the old boolean (controller activation + telemetry).
SqlDocumentServicerewrites itshandleNewQueryCommandprecedence andonDidOpenTextDocumentauto-connect to branch on the new behavior enum and adds aUseDefaultConnectionstrategy.ConnectionConfig.validateDefaultConnectionIdwarns and prompts the user (select connection / change setting) when the configuration is invalid.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/mssql/package.json | Adds new settings (newEditorConnectionBehavior, defaultConnectionId) and deprecation metadata on the old setting. |
| extensions/mssql/package.nls.json | Adds localized strings for the new settings and deprecation. |
| extensions/mssql/src/constants/constants.ts | Adds setting keys and NewEditorConnectionBehavior enum; marks old constant deprecated. |
| extensions/mssql/src/constants/locConstants.ts | Adds connection-default warning/prompt strings. |
| extensions/mssql/src/controllers/mainController.ts | Calls one-shot migration from the old setting on activation and reports new behavior via telemetry. |
| extensions/mssql/src/controllers/sqlDocumentService.ts | Rewires connection precedence in handleNewQueryCommand/onDidOpenTextDocument, adds UseDefaultConnection strategy and helper getters; removes OE-single-selection fallback. |
| extensions/mssql/src/connectionconfig/connectionconfig.ts | Adds validateDefaultConnectionId + prompt helpers fired during initialization. |
| extensions/mssql/src/sharedInterfaces/telemetry.ts | Adds MigrateEditorConnectionBehavior action. |
| extensions/mssql/test/unit/sqlDocumentService.test.ts | Replaces old setting tests with comprehensive coverage of new behavior and UseDefaultConnection. |
| extensions/mssql/test/unit/mainController.test.ts | New suite covering migration of the old setting across scopes and failure paths. |
| extensions/mssql/test/unit/connectionConfig.test.ts | Adds coverage for validateDefaultConnectionId prompts and follow-up actions. |
| extensions/mssql/l10n/bundle.l10n.json, localization/xliff/vscode-mssql.xlf | Auto-generated localization entries (not reviewed per repo guidelines). |
Comments suppressed due to low confidence (1)
extensions/mssql/src/controllers/sqlDocumentService.ts:162
- This change removes the previous Case 3 — when the user runs "New Query" from the command palette with no last‑active connection but with exactly one selected node in the Object Explorer tree, the new editor used that node's connection profile. After this PR, that fallback is gone for every behavior value (transferActive, none, defaultConnection); the only remaining OE path is a direct right‑click that passes
nodein. This is a user-facing behavior regression that isn't called out in the PR description ("Replaces transferActiveEditorConnections setting…"). If the removal is intentional, please call it out explicitly; otherwise consider keeping the OE single-selection branch as a context source (e.g., before falling through to PromptForConnection in theelsebranch). The updated testhandleNewQueryCommand uses OE selection when exactly one node is selected and behavior is transferActivenow assertsPromptForConnection, which silently encodes this behavior change.
const behavior = this.getNewEditorConnectionBehavior();
if (node) {
// Case 1: User right-clicked on an OE node and selected "New Query"
nodeType = node.nodeType;
connectionStrategy = ConnectionStrategy.CopyConnectionFromInfo;
sourceNode = node;
} else if (behavior === Constants.NewEditorConnectionBehavior.DefaultConnection) {
// Case 2: Use the configured default connection
nodeType = "defaultConnection";
connectionStrategy = ConnectionStrategy.UseDefaultConnection;
} else if (
behavior === Constants.NewEditorConnectionBehavior.TransferActive &&
this._lastActiveConnectionInfo
) {
// Case 3: User triggered "New Query" from command palette and the active document has a connection
nodeType = "previousEditor";
connectionStrategy = ConnectionStrategy.CopyLastActive;
} else {
// Case 4: User triggered "New Query" from command palette and there's no reasonable context
connectionStrategy = ConnectionStrategy.PromptForConnection;
}
Comment on lines
+126
to
+164
| if (selected) { | ||
| await this._vscodeWrapper.setConfiguration( | ||
| Constants.extensionName, | ||
| "defaultConnectionId", | ||
| selected.profile.id, | ||
| vscode.ConfigurationTarget.Global, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** Shows a QuickPick to switch newEditorConnectionBehavior away from 'defaultConnection'. */ | ||
| private async promptChangeEditorConnectionBehavior(): Promise<void> { | ||
| interface BehaviorQuickPickItem extends vscode.QuickPickItem { | ||
| value: string; | ||
| } | ||
|
|
||
| const items: BehaviorQuickPickItem[] = [ | ||
| { | ||
| label: LocalizedConstants.Connection.defaultConnectionBehaviorTransferActive, | ||
| value: Constants.NewEditorConnectionBehavior.TransferActive, | ||
| }, | ||
| { | ||
| label: LocalizedConstants.Connection.defaultConnectionBehaviorNone, | ||
| value: Constants.NewEditorConnectionBehavior.None, | ||
| }, | ||
| ]; | ||
|
|
||
| const selected = await this._vscodeWrapper.showQuickPick<BehaviorQuickPickItem>(items, { | ||
| placeHolder: LocalizedConstants.Connection.defaultConnectionChangeSettingPlaceholder, | ||
| }); | ||
|
|
||
| if (selected) { | ||
| await this._vscodeWrapper.setConfiguration( | ||
| Constants.extensionName, | ||
| "newEditorConnectionBehavior", | ||
| selected.value, | ||
| vscode.ConfigurationTarget.Global, | ||
| ); | ||
| } |
Comment on lines
+51
to
53
| void this.validateDefaultConnectionId(); | ||
|
|
||
| this.initialized.resolve(); |
Comment on lines
1736
to
1742
| "mssql.transferActiveEditorConnections": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "%mssql.transferActiveEditorConnections.description%", | ||
| "deprecationMessage": "%mssql.transferActiveEditorConnections.deprecationMessage%", | ||
| "scope": "application" | ||
| }, |
Comment on lines
+408
to
+428
| if (behavior === Constants.NewEditorConnectionBehavior.DefaultConnection) { | ||
| const defaultProfile = await this.getDefaultConnectionProfile(); | ||
| if (defaultProfile) { | ||
| this._logger.info("Auto-connecting opened SQL document from default connection", { | ||
| uri: docUri, | ||
| }); | ||
| await this._connectionMgr.connect(docUri, Utils.deepClone(defaultProfile)); | ||
| return; | ||
| } | ||
| // Default connection is not configured or invalid — fall through to transferActive | ||
| } | ||
|
|
||
| // behavior === TransferActive (or defaultConnection fallback): requires a last active connection | ||
| if (!this._lastActiveConnectionInfo) { | ||
| return; | ||
| } | ||
|
|
||
| this._logger.info("Auto-connecting opened SQL document from last active connection", { | ||
| uri: docUri, | ||
| }); | ||
| await this._connectionMgr.connect(docUri, Utils.deepClone(this._lastActiveConnectionInfo)); |
PR Changes
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #22140 +/- ##
==========================================
+ Coverage 74.88% 74.99% +0.11%
==========================================
Files 388 396 +8
Lines 121083 121881 +798
Branches 7299 7348 +49
==========================================
+ Hits 90668 91404 +736
- Misses 30415 30477 +62
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
aasimkhan30
approved these changes
May 19, 2026
lewis-sanchez
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adding support for default connections. Replaces
mssql.transferActiveEditorConnectionssetting with a three-waymssql.newEditorConnectionBehaviorenum (transferActive|none|defaultConnection) and adds a newmssql.defaultConnectionIdsetting, allowing users to designate a specific saved connection that new query editors automatically connect to. The old setting is auto-migrated on activation and marked deprecated.transferActiveEditorConnectionssetting is auto-migrated on activationdefaultConnectionconfig prompts to select or change settingtransferActiveCode Changes Checklist
npm run test)Reviewers: Please read our reviewer guidelines