Add sqlcmd open vscode and sqlcmd open ssms commands
#688
+1,904
−10
Conversation
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
- Created VSCode tool implementation with platform-specific search locations - Created vscode command with connection profile management - Updated open command to include vscode subcommand - Added tests for VSCode functionality - Updated README.md to use VSCode instead of ADS Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
- Improved test skip message with detailed explanation - Added security comment about encrypt setting - Removed unnecessary else block in VSCode tool - Renamed settingsDir to dir for consistency Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
- Created SSMS tool with platform-specific search paths (Windows only) - Created SSMS command to launch with connection parameters - Added SSMS to open command alongside VSCode and ADS - Uses SSMS command-line parameters: -S (server), -U (username), -E (integrated auth) - Note: -P (password) removed in SSMS 18+ for security - Tests passing for all platforms Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
Use strings.ReplaceAll instead of strings.Replace with -1 for better readability Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
- Use promoted fields from embedded structs (endpoint.Address, endpoint.Port, endpoint.ContainerDetails) - Wrap all user-facing strings with localizer.Sprintf for proper localization - Fix VSCode Insiders settings path detection - prefer Insiders if directory exists - Remove redundant fmt imports from platform-specific files - Addresses staticcheck QF1008 warnings about redundant embedded-field selectors Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
- Use promoted field endpoint.Id instead of endpoint.ContainerDetails.Id - Applied to both ssms.go and vscode.go ensureContainerIsRunning functions - All staticcheck QF1008 warnings now resolved Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Add 'sqlcmd open vscode' command with connection profile creation - Add 'sqlcmd open ssms' command for Windows with SQL authentication - Auto-install MSSQL extension when launching VS Code - Copy password to clipboard for easy paste in login dialogs - Add trustServerCertificate for local container connections - Add comprehensive tests (15 new tests) - Update README with documentation for both commands
dlevy-msft-sql
added
Size: S
There was a problem hiding this comment.
Pull request overview
This PR adds support for opening Visual Studio Code and SQL Server Management Studio (SSMS) to work with SQL Server connections managed by sqlcmd, addressing the deprecation of Azure Data Studio. The implementation uses clipboard-based password sharing since both tools use sandboxed credential storage.
Changes:
- Adds
sqlcmd open vscodecommand that creates connection profiles in VS Code settings and auto-installs the MSSQL extension - Adds
sqlcmd open ssmscommand (Windows-only) that launches SSMS with pre-configured connection parameters - Implements cross-platform clipboard support for secure password sharing
- Includes comprehensive tests and documentation updates
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
cmd/modern/root/open.go |
Updates open command to include VSCode and SSMS subcommands |
cmd/modern/root/open/vscode.go |
Main VSCode command implementation with settings file manipulation |
cmd/modern/root/open/vscode_*.go |
Platform-specific VSCode implementations |
cmd/modern/root/open/ssms.go |
Main SSMS command implementation |
cmd/modern/root/open/ssms_*.go |
Platform-specific SSMS implementations (Fatal on non-Windows) |
cmd/modern/root/open/clipboard.go |
Helper function for clipboard-based password sharing |
cmd/modern/root/open/*_test.go |
Comprehensive test coverage for new commands |
internal/tools/tool/vscode*.go |
VSCode tool detection and configuration |
internal/tools/tool/ssms*.go |
SSMS tool detection and configuration |
internal/pal/clipboard*.go |
Cross-platform clipboard implementation using native APIs |
internal/tools/tools.go |
Registers VSCode and SSMS tools |
README.md |
Adds clear documentation with usage examples |
.gitignore |
Adds /modern build artifact |
- Check error returns from os.WriteFile and json.Unmarshal in tests - Simplify embedded field access (endpoint.ContainerDetails instead of endpoint.AssetDetails.ContainerDetails)
- Add safe type assertion for profileName in updateOrAddProfile to prevent panic - Add nil check for user.BasicAuth before accessing Username in vscode.go - Add nil check for user.BasicAuth before accessing Username in ssms.go
- Change endpoint.ContainerDetails.Id to endpoint.Id in ssms.go - Change endpoint.ContainerDetails.Id to endpoint.Id in vscode.go
- Use explicit endpoint.AssetDetails.ContainerDetails pattern for consistency - Use endpoint.AssetDetails.ContainerDetails.Id instead of embedded field - Replace custom contains() with strings.Contains() in tests - Replace custom itoa() with strconv.Itoa() in tests - Move SSMS platform check to run() for early exit on non-Windows - Set authenticationType and savePassword conditionally in VS Code profiles
Fixes govulncheck CI failure caused by duplicate run() method declaration. The ssms.go file (Windows SSMS launcher) was being compiled on all platforms, conflicting with ssms_linux.go and ssms_darwin.go which define platform-specific run() methods that exit with a friendly error message.
- Simplify embedded field access per staticcheck QF1008: endpoint.AssetDetails.ContainerDetails -> endpoint.ContainerDetails - Add DefineCommand to Linux/macOS SSMS stubs so run() is referenced and the command is properly registered on all platforms
- Simplify endpoint.ContainerDetails.Id to endpoint.Id (QF1008) The Id field is promoted through Go embedding chain - Remove unused displayPreLaunchInfo stubs from Linux/macOS files These were dead code since run() exits via Fatal()
Copilot correctly identified that accessing promoted field endpoint.ContainerDetails will panic if endpoint.AssetDetails is nil. Changed back to explicit path endpoint.AssetDetails.ContainerDetails to ensure nil-safety. The staticcheck QF1008 suggestion doesn't apply here because the embedded pointer must be checked before accessing promoted fields.
Staticcheck QF1008 suggests simplifying endpoint.AssetDetails.ContainerDetails to endpoint.ContainerDetails, but this is unsafe when AssetDetails is an embedded *pointer*. Accessing promoted fields through a nil pointer panics. The explicit path is intentional for nil-safety, so we suppress the warning.
…nflicts The previous approach caused a cycle between: - Copilot review: use explicit path for nil-safety - staticcheck QF1008: simplify embedded field access Solution: Pass the container ID string directly to ensureContainerIsRunning() instead of the whole endpoint. This way: 1. The nil-safe check happens once at the call site with explicit path 2. The helper function receives just the ID, avoiding embedded field issues 3. No nolint directives needed
- Fix panic when exeName is empty (tool not found during Init) - Update TestAds to skip when ADS is not installed - ADS is deprecated and being replaced by VS Code/SSMS support
…sion - Add --install-extension flag to 'sqlcmd open vscode' command - Only install MSSQL extension when explicitly requested (no auto-install) - Update tests to skip when VS Code or SSMS are not installed - Follows principle of asking permission before installing anything
- Add RunWithOutput() method to Tool interface to capture command output - Check for ms-mssql.mssql extension using 'code --list-extensions' - Show helpful error with --install-extension hint if extension is missing
Use local variable for AssetDetails to avoid embedded field selector warning while maintaining nil-safety checks.
ContainerDetails is embedded in AssetDetails, so Id is promoted directly.
- Windows: winget and choco commands for VS Code and SSMS - macOS: brew command for VS Code - Linux: apt, dnf, and snap commands for VS Code - Also mention --install-extension flag for MSSQL extension
|
|
||
| // If the context has a local container, ensure it is running, otherwise bail out | ||
| if endpoint.AssetDetails != nil && endpoint.AssetDetails.ContainerDetails != nil { | ||
| c.ensureContainerIsRunning(endpoint.AssetDetails.ContainerDetails.Id) |
There was a problem hiding this comment.
Inconsistent access pattern for container ID. The VSCode command uses asset.Id (line 60 of vscode.go) which leverages struct embedding from AssetDetails, while this code uses the longer form endpoint.AssetDetails.ContainerDetails.Id. Both work due to Go's struct embedding, but for consistency with the VSCode command in the same PR, consider using endpoint.AssetDetails.Id instead.
Suggested change
| c.ensureContainerIsRunning(endpoint.AssetDetails.ContainerDetails.Id) | |
| c.ensureContainerIsRunning(endpoint.AssetDetails.Id) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
This PR adds two new commands to replace the deprecated Azure Data Studio integration:
sqlcmd open vscode- Opens VS Code with a configured connection profilesqlcmd open ssms- Opens SQL Server Management Studio (Windows only)Both commands use clipboard-based password sharing since these tools use sandboxed credential storage that can't be accessed directly.
Changes
New Features
sqlcmd open vscodemssql.connections)mssql,mssql2)trustServerCertificate: trueandencrypt: Optionalfor local container connections--install-extensionflag to install the MSSQL extension (not auto-installed)sqlcmd open ssms(Windows only)-Cflag to trust server certificate-Pflag for security)New Files
cmd/modern/root/open/vscode.gocmd/modern/root/open/vscode_*.gocmd/modern/root/open/ssms.gocmd/modern/root/open/ssms_linux.gocmd/modern/root/open/ssms_darwin.gocmd/modern/root/open/clipboard.gointernal/pal/clipboard*.gointernal/tools/tool/vscode*.gointernal/tools/tool/ssms*.goBug Fixes
tool.IsInstalled()when tool executable path is emptyTestAdsto skip when Azure Data Studio is not installed (deprecated)Usage Examples
VS Code
VS Code with Extension Installation
SSMS (Windows)
SSMS on Non-Windows
Technical Notes
Why Clipboard-Based Password Sharing?
SecretStorageAPI which is sandboxed per-extension-Pcommand-line parameter for security reasonsTrust Server Certificate
Both commands configure trust for server certificates on local container connections:
trustServerCertificate: trueandencrypt: "Optional"in profile-Ccommand-line flagThis is required because local SQL Server containers use self-signed certificates.
Extension Installation
The MSSQL extension is not auto-installed to respect user consent. Use
--install-extensionto explicitly opt-in:Build Constraints
ssms.gohas//go:build windows- only compiles on Windowsssms_linux.goandssms_darwin.goprovide stubs that exit with helpful error messagesTesting
Tests skip gracefully when tools are not installed:
Checklist
Supersedes #685
Fixes #584