Add options for handling errors encountered during debugging #717
Conversation
|
@microsoft-github-policy-service agree |
|
…dling-errors-encountered
There was a problem hiding this comment.
Pull request overview
This PR adds error handling options for debugging and refactors Python environment integration to support both the legacy Python extension API and a new Python Environments extension. The PR includes significant changes to how Python interpreters are resolved, terminal quote character computation, and comprehensive test updates.
Changes:
- Added
onErrorsconfiguration option to control behavior when encountering diagnostic errors before debugging - Refactored Python API integration to support dual extension paths (legacy and new environments extension)
- Added automatic terminal quote character detection based on shell type
- Improved PATH environment variable handling to avoid double separators
- Updated test infrastructure and added comprehensive test coverage
Reviewed changes
Copilot reviewed 70 out of 72 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension/common/onErrorsAction.ts | New file implementing error handling actions before debugging |
| src/extension/extensionInit.ts | Added error checking before debug commands execute |
| src/extension/debugger/configuration/debugConfigurationService.ts | Integrated error handling and improved interpreter resolution |
| src/extension/common/python.ts | Refactored to support both legacy and new Python environment extensions |
| src/extension/common/legacyPython.ts | New file implementing legacy Python extension API compatibility |
| src/extension/envExtApi.ts | New file with comprehensive Python Environments extension API types |
| src/extension/debugger/configuration/resolvers/base.ts | Enhanced Python path resolution with better documentation |
| src/extension/debugger/configuration/resolvers/launch.ts | Added terminal quote character computation |
| src/extension/noConfigDebugInit.ts | Fixed PATH separator handling |
| package.json | Added onErrors setting and terminalQuoteCharacter configuration |
| Multiple test files | Updated tests for new API structures and added comprehensive coverage |
| Localization files | Added translations for new error message |
| Build files | Updated Node.js version and build configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (platform === 'win32') { | ||
| return "'"; // Default is powershell | ||
| } else { | ||
| return '"'; // Default is bash/zsh | ||
| } |
There was a problem hiding this comment.
The comment on line 213 states "Default is powershell" but this assumption may not be accurate for all Windows systems. Windows 10/11 now defaults to PowerShell, but older systems or user configurations may use cmd.exe as default. Consider updating the comment to be more accurate: "Defaults to single quote (PowerShell default on modern Windows)" or add additional logic to detect the actual default shell.
| { title: 'Debug Anyway', id: OnErrorsActions.debugAnyway }, | ||
| { title: 'Show Errors', id: OnErrorsActions.showErrors }, | ||
| { title: 'Abort', id: OnErrorsActions.abort, isCloseAffordance: true }, | ||
| ]; |
There was a problem hiding this comment.
The action button titles are hardcoded English strings instead of using l10n for localization. These should be wrapped with l10n.t() for consistency with the rest of the codebase and to support multiple languages.
| if (err) { | ||
| traceError(`Error reading debuggerAdapterEndpoint.txt file: ${err}`); | ||
| return; | ||
| } | ||
| try { | ||
| // parse the client port | ||
| const dataParse = data.toString(); |
There was a problem hiding this comment.
The variable dataParse is declared after the error check but it was previously declared before (line 104 in old code). While this change improves code by only parsing when needed, the variable name could be more descriptive. Consider renaming to fileContent or endpointData for better clarity.
| return; | ||
| } | ||
| // If program is a valid file, get interpreter for that file | ||
| if (fs.existsSync(debugConfiguration.program) && fs.statSync(debugConfiguration.program).isFile()) { |
There was a problem hiding this comment.
The code calls fs.statSync() after already checking fs.existsSync(). This results in two filesystem calls. Consider using only fs.statSync() wrapped in a try-catch, as statSync will throw if the file doesn't exist, eliminating the redundant existsSync check.
| "format-check": "prettier --check \"src/**/*.ts\" \"build/**/*.yml\" \".github/**/*.yml\"", | ||
| "format-fix": "prettier --write \"src/**/*.ts\" \"build/**/*.yml\" \".github/**/*.yml\"", |
There was a problem hiding this comment.
The quotes in the glob patterns were changed from single quotes to double quotes. While this works, it's inconsistent with the previous style and may cause issues on some shells. Consider keeping the original single quote style or documenting why the change was necessary.
| "format-check": "prettier --check \"src/**/*.ts\" \"build/**/*.yml\" \".github/**/*.yml\"", | |
| "format-fix": "prettier --write \"src/**/*.ts\" \"build/**/*.yml\" \".github/**/*.yml\"", | |
| "format-check": "prettier --check 'src/**/*.ts' 'build/**/*.yml' '.github/**/*.yml'", | |
| "format-fix": "prettier --write 'src/**/*.ts' 'build/**/*.yml' '.github/**/*.yml'", |
| } | ||
| } | ||
|
|
||
| const profile = defaultProfile ? profiles[defaultProfile] : profiles[0]; |
There was a problem hiding this comment.
This use of variable 'defaultProfile' always evaluates to true.
2 participants
Based on #714
Added options for handling errors encountered during debugging.
Change Description
Add an
On Errorsoption to the settings (highlighted in orange in the image below).This option checks the diagnostics list for any items with severity set to "error" before starting debugging.
If any are found, it handles them according to the selected setting.
Options
Since
debugAnyWayis the default value, adding this feature does not change the current debugging behavior.Example
Take this code with a spelling error as an example.
,
With
On Errorsset toprompt,when pressing
F5ora dialog box will appear asking the user how to proceed
When
showErrorsis selected (highlighted in orange in the image above), the error list will be shown and debugging will stop.