-
Notifications
You must be signed in to change notification settings - Fork 82
Add sqlcmd open vscode and sqlcmd open ssms commands
#688
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
base: main
Are you sure you want to change the base?
Conversation
- 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 9 comments.
- 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()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 2 comments.
- Only use relaxed TLS settings (encrypt=Optional, trustServerCertificate=true) for local connections (containers, localhost, 127.0.0.1, host.docker.internal) - Use secure defaults (encrypt=Mandatory, trustServerCertificate=false) for production/remote connections - Add isLocalEndpoint helper to detect local connections - Update SSMS to only add -C flag for local connections
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 4 comments.
- Add nil check for ProcessState in tool.Run() to prevent panic - Fix grammar: 'e.g' -> 'e.g.,' in open command description - Clarify SQL authentication comment in ssms.go - Add warning when MSSQL extension verification fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
Use 'asset := endpoint.AssetDetails' pattern to satisfy staticcheck QF1008 while maintaining nil-safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.
Replace complex Win32 API calls with simple exec.Command to clip.exe. This eliminates unsafe code, manual memory management, and nolint directives while being more maintainable and reliable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.
Collect all errors from clipboard utility attempts (xclip, xsel, wl-copy) and return a combined error message instead of just the last error. This provides better debugging information when all utilities fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
…erateCommandLine - README: Update VS Code section to reflect --install-extension is opt-in - tool_linux.go: Implement generateCommandLine (was panic placeholder)
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