Skip to content

Conversation

@karthiknadig
Copy link
Member

@karthiknadig karthiknadig commented Feb 10, 2025

fixes #173

@karthiknadig karthiknadig self-assigned this Feb 10, 2025
@karthiknadig karthiknadig added the bug Issue identified by VS Code Team member as probable bug label Feb 10, 2025
@karthiknadig karthiknadig added this to the February 2025 milestone Feb 10, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR fixes shell detection issues on Windows by ensuring GitBash paths use forward slashes and refactors fallback order for shell detection. Key changes include:

  • Adding unit tests for various shell types.
  • Updating the GitBash regex in shellDetector to cover additional patterns.
  • Replacing unsafeEntries usage with a new _entries function in shellDetector.
  • Refactoring Venv and Conda utility modules to include a pathForGitBash helper that replaces backslashes.
  • Removing the now redundant unsafeEntries utility file.

Changes

File Description
src/test/features/common/shellDetector.unit.test.ts Added tests for multiple shell types with a check to skip “unknown” shells
src/features/common/shellDetector.ts Updated GitBash regex, replaced unsafeEntries with _entries, and reordered shell detection fallbacks
src/managers/builtin/venvUtils.ts Refactored Venv command mappings using a new pathForGitBash helper and modernized the code style
src/managers/conda/condaUtils.ts Added GitBash-specific shell activation/deactivation using pathForGitBash and updated environment creation logic
src/common/utils/unsafeEntries.ts Removed the unsafeEntries file now replaced by a custom _entries function in shellDetector

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/test/features/common/shellDetector.unit.test.ts:133

  • Ensure that comparing the test string 'unknown' to TerminalShellType.unknown is type-safe; verify that both values are compatible (e.g., if TerminalShellType.unknown is a string literal) to avoid unexpected mismatches.
if (shellType === TerminalShellType.unknown) {

src/features/common/shellDetector.ts:171

  • Review the reordering of fallback detection functions; using identifyShellFromVSC before identifyShellFromTerminalName may change the intended detection logic—ensure this new order still meets all edge cases.
shellType = identifyShellFromVSC(terminal);

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

@karthiknadig karthiknadig marked this pull request as ready for review February 11, 2025 00:08
@vs-code-engineering
Copy link

⚠️ This PR originates from a fork. Due to security restrictions, pipelines from forks are no longer triggered automatically. Learn more.

If the changes appear safe, you can manually trigger the pipeline by commenting /AzurePipelines run.

@karthiknadig karthiknadig enabled auto-merge (squash) February 11, 2025 02:09
@karthiknadig karthiknadig merged commit ba76736 into microsoft:main Feb 12, 2025
6 checks passed
@karthiknadig karthiknadig deleted the git-bash1 branch February 12, 2025 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Git Bash Shell windows path parsing error

2 participants