Skip to content
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

Handle ProcessPicker via resolveDebugConfiguration #4509

Merged
merged 4 commits into from Apr 21, 2021

Conversation

@WardenGnaw
Copy link
Member

@WardenGnaw WardenGnaw commented Apr 20, 2021

VS Code commands are limited to only being able to have a single output.
We will handle having an empty processId and show the dialog for
ProcessPicker in resolveDebugConfigurationWithSubstitutedVariables.
We need multiple outputs to handle the latest macOS on Apple M1.

For Apple Silicon M1 (ARM64), we need to determine if we need to use the
x86_64 or arm64 debugger. We are able to detect if the process is using
S_TRANSLATED using the ps commandline with 'flags', if it is set with
0x20000, it is emulated.

VS Code commands are limited to only being able to have a single output.
We will handle having an empty processId and show the dialog for
ProcessPicker in resolveDebugConfigurationWithSubstitutedVariables.
We need multiple outputs to handle the latest macOS on Apple M1.

For Apple Silicon M1 (ARM64), we need to determine if we need to use the
x86_64 or arm64 debugger. We are able to detect if the process is using
S_TRANSLATED using the ps commandline with 'flags', if it is set with
0x20000, it is emulated.
@WardenGnaw WardenGnaw requested a review from gregg-miskelly Apr 20, 2021
src/features/commands.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@gregg-miskelly gregg-miskelly left a comment

Otherwise LGTM

Migrate remoteProcessPicker to also be in resolve configuration

Handle 'clr' type for remote process picking.

Addressing PR issues.

Added comment about 0x20000
@WardenGnaw
Copy link
Member Author

@WardenGnaw WardenGnaw commented Apr 20, 2021

Tested:

Type Platform Scenario Result
clr Windows Launch
clr Windows Attach
coreclr Windows Attach
coreclr Windows Attach
coreclr Windows Attach to WSL
coreclr macOS (ARM64) Launch x86_64 dotnet
coreclr macOS (ARM64) Attach x86_64 dotnet
coreclr macOS (ARM64) Launch arm64 dotnet
coreclr macOS (ARM64) Attach arm64 dotnet
coreclr Windows input promptString
coreclr macOS -> Linux Attach x86_64 dotnet
src/assets.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@gregg-miskelly gregg-miskelly left a comment

Aside from maybe removing 'processId' from the templates looks good to me.

@WardenGnaw WardenGnaw merged commit 5497895 into master Apr 21, 2021
3 checks passed
3 checks passed
@github-actions
build
Details
license/cla All CLA requirements met.
Details
security/snyk (david-driscoll) 1 security test has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants