refactor: Update BuildCmdWrapper to use args splatting and add unit tests#67
refactor: Update BuildCmdWrapper to use args splatting and add unit tests#67GordonBeeming merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the Windows .cmd wrapper generation to forward arguments into PowerShell in a way that preserves quoting, and adds unit tests around the wrapper content to prevent regressions.
Changes:
- Update
ShellIntegration.BuildCmdWrapperto forward args using PowerShell@argssplatting (and a--separator) instead of embedding%*inside the-Commandstring. - Add unit tests validating the generated wrapper contains the new forwarding approach.
- Remove the
.worktrees/issue-65subproject commit reference file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
app/Infrastructure/ShellIntegration.cs |
Changes Windows CMD wrapper content to forward args via @args and --, and exposes the wrapper builder for unit tests. |
tests/CopilotHere.UnitTests/ShellIntegrationTests.cs |
Adds tests asserting the wrapper includes @args forwarding (and partially asserts -- %*). |
.worktrees/issue-65 |
Removes an extraneous worktree/subproject commit reference. |
| internal static string BuildCmdWrapper(string functionName, string psPath) | ||
| { | ||
| // Prefer pwsh, fall back to powershell. | ||
| // Keep wrapper simple and rely on PowerShell script for all logic. | ||
| // Pass arguments after `--` and forward via PowerShell `@args` so quoted | ||
| // values (for example --prompt "What is 1 + 1 ?") stay as a single arg. | ||
| return "@echo off\r\n" + | ||
| "setlocal\r\n" + | ||
| "set \"SCRIPT=%USERPROFILE%\\.copilot_here.ps1\"\r\n" + | ||
| "where pwsh >nul 2>nul\r\n" + | ||
| "if %ERRORLEVEL%==0 (\r\n" + | ||
| $" pwsh -NoProfile -ExecutionPolicy Bypass -Command \". '%USERPROFILE%\\.copilot_here.ps1'; {functionName} %*\"\r\n" + | ||
| $" pwsh -NoProfile -ExecutionPolicy Bypass -Command \"& {{ . '%USERPROFILE%\\.copilot_here.ps1'; {functionName} @args }}\" -- %*\r\n" + | ||
| ") else (\r\n" + | ||
| $" powershell -NoProfile -ExecutionPolicy Bypass -Command \". '%USERPROFILE%\\.copilot_here.ps1'; {functionName} %*\"\r\n" + | ||
| $" powershell -NoProfile -ExecutionPolicy Bypass -Command \"& {{ . '%USERPROFILE%\\.copilot_here.ps1'; {functionName} @args }}\" -- %*\r\n" + |
There was a problem hiding this comment.
BuildCmdWrapper takes psPath but never uses it (the wrapper still hardcodes %USERPROFILE%\.copilot_here.ps1). Either remove the unused parameter (and adjust call sites/tests) or use psPath to build the script path in the generated CMD so the wrapper reflects the actual installed location.
| $" pwsh -NoProfile -ExecutionPolicy Bypass -Command \"& {{ . '%USERPROFILE%\\.copilot_here.ps1'; {functionName} @args }}\" -- %*\r\n" + | ||
| ") else (\r\n" + | ||
| $" powershell -NoProfile -ExecutionPolicy Bypass -Command \". '%USERPROFILE%\\.copilot_here.ps1'; {functionName} %*\"\r\n" + | ||
| $" powershell -NoProfile -ExecutionPolicy Bypass -Command \"& {{ . '%USERPROFILE%\\.copilot_here.ps1'; {functionName} @args }}\" -- %*\r\n" + |
There was a problem hiding this comment.
This change alters user-facing behavior (Windows CMD wrapper argument forwarding) but the repo’s versioning rule requires bumping the version consistently across all version locations so users re-download updates. Please increment the version in copilot_here.sh, copilot_here.ps1, Directory.Build.props, app/Infrastructure/BuildInfo.cs, and update the “Current version” in .github/copilot-instructions.md (see .github/copilot-instructions.md Script Versioning section).
This pull request improves the way command-line arguments are forwarded from Windows batch wrapper scripts to PowerShell functions, ensuring that quoted arguments are handled correctly. It also adds unit tests to verify the new behavior.
Improvements to command-line argument forwarding:
BuildCmdWrappermethod inShellIntegrationto use PowerShell's@argssplatting and the--separator when forwarding arguments, which preserves quoted arguments as single values.Testing enhancements:
ShellIntegrationTests.csto verify that the wrapper script uses@argsand the--separator for bothpwshand Windows PowerShell.Other changes:
.worktrees/issue-65subproject commit reference.