Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add support for one-step debugging for Blazor #1885
Add support for one-step debugging for Blazor #1885
Changes from all commits
88e4fad
6757fca
ce06cf1
dbaabe5
76fc7a9
d239662
ea2e7b4
79475e0
b6795c7
2ed3e96
146d0dd
0fd5492
ea7b144
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Just remembered, you'll also need to make sure this is tracked in component governance as well: https://dev.azure.com/dnceng/internal/_componentGovernance/dotnet-aspnetcore-tooling?_a=alerts&typeId=1981311&alerts-view-option=active
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.
Missed this in the first pass. @gregg-miskelly do you all do anything special in the case
dotnet
is not on the path? Wondering if we should be notifying users if it's not.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.
For our implementation, the error will be visible in the terminal in the workspace. Not sure if we want to check for
dotnet
in the path ahead of time and do something prominent like show an error message.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.
Assuming the C# extension is installed (I think it always will be?) the user will get a prompt telling them that dotnet could not be located. That said, lots of people ignore errors. So if you are just showing a generic 'file not found' error, it might be helpful to print out a bit extra. You could link to: https://aka.ms/VSCode-CS-DotnetNotFoundHelp
In reply to: 424869410 [](ancestors = 424869410)
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.
This will fire when any debug session finishes -- you want to look at the session to make sure it's the one we care about. (e.g. I might have an API server written in some other language, stopping/restarting that shouldn't cause my blazor debugging to stop)
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.
Seems sensible. The
type
property on the DebugSession seemed like the best thing to check against but let me know if there is a better approach here.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.
Rather than using
map
I think you want to instead loop throughprocesses
to findtargetPid
. Then loop through just the remaining entries in the map to find any child processes. This way you will avoid the process id reuse problem with ppid.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.
I'll add this in a future PR.