Skip to content

Enhance NativePythonFinderImpl with improved logging and configuration comparison#1149

Merged
karthiknadig merged 1 commit intomainfrom
plain-dormouse
Feb 3, 2026
Merged

Enhance NativePythonFinderImpl with improved logging and configuration comparison#1149
karthiknadig merged 1 commit intomainfrom
plain-dormouse

Conversation

@karthiknadig
Copy link
Member

@karthiknadig karthiknadig commented Feb 2, 2026

Summary

This PR implements code quality improvements for ativePythonFinder.ts as identified in #1142.

Changes

1. Type Guard for isNativeEnvInfo()

Changed return type from boolean to info is NativeEnvInfo for proper TypeScript type narrowing.

2. Robust Configuration Comparison

Replaced fragile JSON.stringify() comparison with a dedicated configurationEquals() method that:

  • Compares properties individually
  • Uses sorted array comparison to handle order differences
  • Avoids issues with undefined value serialization

3. Cache Key Collision Prevention

Changed the separator for cache keys from , to \0 (null character) to prevent collisions with paths containing commas.

4. Standardized Logging

Converted traceVerbose calls within NativePythonFinderImpl to use outputChannel.debug() for consistent instance-level logging. Removed unused import.

5. Graceful Shutdown

Added a graceful shutdown mechanism that closes stdin before killing the process, giving it 500ms to clean up.

6. Documentation

Added explanatory comment for the intentional duplicate venvFolders fetch explaining why it's needed when searchPaths may override environmentDirectories.

Fixes #1142

@eleanorjboyd
Copy link
Member

@copilot add testing for functionality introduced here and also to confirm the Cache Key Collision Prevention and Type Guard for isNativeEnvInfo() is working.

Copy link
Contributor

Copilot AI commented Feb 2, 2026

@eleanorjboyd I've opened a new pull request, #1151, to work on those changes. Once the pull request is ready, I'll request review from you.

@karthiknadig karthiknadig reopened this Feb 2, 2026
@karthiknadig karthiknadig reopened this Feb 3, 2026
@karthiknadig karthiknadig self-assigned this Feb 3, 2026
@karthiknadig karthiknadig marked this pull request as ready for review February 3, 2026 00:52
@karthiknadig karthiknadig enabled auto-merge (squash) February 3, 2026 00:52
@vs-code-engineering vs-code-engineering bot added this to the February 2026 milestone Feb 3, 2026
@karthiknadig karthiknadig merged commit ac8abbd into main Feb 3, 2026
11 of 26 checks passed
@karthiknadig karthiknadig deleted the plain-dormouse branch February 3, 2026 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NativePythonFinder: Code quality improvements - type safety, logging, and robustness

4 participants