Skip to content

Bugfix: Summaries on Windows#14

Open
thomas-intramotev wants to merge 2 commits intosalam:mainfrom
thomas-intramotev:bug/summaries-on-windows
Open

Bugfix: Summaries on Windows#14
thomas-intramotev wants to merge 2 commits intosalam:mainfrom
thomas-intramotev:bug/summaries-on-windows

Conversation

@thomas-intramotev
Copy link
Copy Markdown
Contributor

Summary

Fixes #13: since this commit moved the Summaries prompt string from stdin to a CLI arg, AI summaries have been broken on Windows.

That said: the real source of the problem seems to have been my last PR on this feature where I set shell: true for multiple child process calls on Windows. This evidently made Windows CMD unable to handle the newlines in the prompt string, and from what I can tell: it was never necessary in the first place, and might even have had security implications. Claude, ironically, led me wrong on that one 🙃

Changes

  • Removes the shell: true child process argument in several places (all tested to have no impact on Windows)
  • In a similar vein: strips the environment passed by StandaloneMessageHandler.ts to child processes, similar to how SummaryService.ts does it

Checklist

  • npm test passes
  • npm run lint passes
  • Webview builds (cd webview && npm run build)
  • Tested manually in the Extension Development Host
  • Updated CHANGELOG.md (if user-facing change)
  • User-facing strings use vscode.l10n.t() for i18n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Summaries on Windows

1 participant