-
Notifications
You must be signed in to change notification settings - Fork 237
Support breakpoints in untitled files in WinPS #2248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Adds support for setting breakpoints in untitled/unsaved files for Windows PowerShell 5.1. This aligns the breakpoint validation behaviour with the PowerShell 7.x API so that a breakpoint can be set for any ScriptBlock with a filename if it aligns with the client's filename.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for setting breakpoints in untitled/unsaved files for Windows PowerShell 5.1, aligning the breakpoint validation behavior with PowerShell 7.x. This enables debugging of untitled files across all supported PowerShell versions.
- Replaced direct
Set-PSBreakpoint
calls with custom logic that bypasses file path validation - Removed PowerShell Core version checks that previously limited untitled file breakpoint support
- Updated tests to verify both command and line breakpoints work in untitled files
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
BreakpointService.cs | Implements custom breakpoint creation logic using reflection to bypass file validation |
BreakpointHandlers.cs | Removes PowerShell version checks and simplifies untitled file support logic |
ConfigurationDoneHandler.cs | Removes unused dependencies and PowerShell version-specific parsing logic |
DebugServiceTests.cs | Enhances test coverage with parameterized tests and removes constructor parameter |
Comments suppressed due to low confidence (1)
src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs:21
- [nitpick] The constant name '_setPSBreakpointLegacy' uses an underscore prefix which is typically reserved for private fields, but this is a constant. Consider renaming to 'SetPSBreakpointLegacyScript' for clarity.
private const string _setPSBreakpointLegacy = @"
psCommand | ||
.AddCommand(@"Microsoft.PowerShell.Utility\Set-PSBreakpoint") | ||
.AddScript(_setPSBreakpointLegacy, useLocalScope: true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using reflection to access non-public constructors and executing dynamic PowerShell scripts could introduce security risks. Consider adding input validation or restricting the scope where this custom script can be executed.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case the code is only run on WinPS 5.1 which is mostly set in stone. PowerShell 7+ which is a rolling target uses a newer public API not available in the older version.
If for whatever reason WinPS is updated and the reflection code is unable to find the ctors, the latest commit has it falling back to Set-PSBreakpoint
which is the original code anyway.
) | ||
|
||
if ($PSCmdlet.ParameterSetName -eq 'Command') { | ||
$cmdCtor = [System.Management.Automation.CommandBreakpoint].GetConstructor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The embedded PowerShell script uses reflection to access internal constructors which makes the code brittle to PowerShell internal API changes. Consider adding error handling for cases where these constructors might not be available in future versions.
Copilot uses AI. Check for mistakes.
PR Summary
Adds support for setting breakpoints in untitled/unsaved files for Windows PowerShell 5.1. This aligns the breakpoint validation behaviour with the PowerShell 7.x API so that a breakpoint can be set for any ScriptBlock with a filename if it aligns with the client's filename.
PR Context
Fixes: #2243
This also means that breakpoints in untitled files will work in Windows PowerShell 5.1 and not just PowerShell 7+.