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

Resolve command variables contributed by debuggers #11170

Merged
merged 1 commit into from
May 26, 2022

Conversation

alvsan09
Copy link
Contributor

@alvsan09 alvsan09 commented May 17, 2022

What it does

Fixes: #10710, Fixes: #2819

The debuggers contribution point provides the possibility to specify
command id variables which can therefore be referenced within debug
configurations.

This change reads these contributed variables and feeds them to the
variable resolver so they can be used just before resolving values
via commands.

See "variables" at:
https://code.visualstudio.com/api/references/contribution-points#contributes.debuggers

In addition, the 'VariableResolverOptions' includes an option to
indicate if all variables were successfully resolved, this enables
to stop the processing and prevent the possibility of error
notifications e.g. in case a user did not select a valid option.

How to test

The following test uses the command "PickProcess" contributed by the node-debug extension.
We will need to start a node application that we can attach to i.e. using the --inspect option.

  1. You can use the following test application, created for this purpose:
    https://github.com/alvsan09/hello_express.git

  2. Build the application by running

yarn

  1. Run the application

node --inspect hello_express.js

  1. Open a browser pointing to localhost:3099, make sure you see the Hello world! message and a counter incrementing every refresh.
  2. Open the folder holding your application with theia
  3. From the debug view, start the provided configuration i.e. Attach to node process,
    NOTE: This configuration uses the following variable that needs to be resolved "${command:PickProcess}"
  4. From the list of node processes select the one hello_express.js
    NOTE: The fact that a list of node processes is presented for selection is an indication of the successful resolution of the command variable mentioned in the previous step.
  5. Open the file hello_express.js in editor and place a break point inside the get block. The break point shall become active.
  6. In the browser, Refresh the application and make sure the break point is hit.
  7. Repeat steps 1-6 above but don't select any of the node processes e.g. pressing escape or clicking outside of the options area.
  8. Note that no debug session is started and no errors are reported.

pickProcess

Review checklist

Reminder for reviewers

@alvsan09 alvsan09 added vscode issues related to VSCode compatibility debug issues that related to debug functionality variable-resolver issues related to the variable-resolver extension labels May 17, 2022
@colin-grant-work colin-grant-work self-requested a review May 19, 2022 14:29
@msujew msujew self-requested a review May 23, 2022 11:36
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that the change allows to resolve command variables 👍

  • Variables such as ${command:PickProcess} are resolved correctly
    • Selecting such a debug config opens the quick input to select a process id
    • Afterwards, the debugger runs as expected
  • Other debug configs continue to work as expected

I have some discussions on the code that I'd like to see resolved before approving.

@alvsan09 alvsan09 force-pushed the asl/resolve_debugger_command_variables branch from 2b7096d to a721460 Compare May 24, 2022 18:42
@alvsan09
Copy link
Contributor Author

I have some discussions on the code that I'd like to see resolved before approving.

@msujew, please take a look to the recent updates which address your remarks,
Thanks !!

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

packages/plugin-ext/package.json Outdated Show resolved Hide resolved
The debuggers contribution point provides the possibility to specify
command id variables which can therefore be referenced within debug
configurations.

This change reads these contributed variables and feeds them to the
variable resolver so they can be used just before resolving values
via commands.

See "variables" at:
https://code.visualstudio.com/api/references/contribution-points#contributes.debuggers

In addition, the 'VariableResolverOptions' includes an option to
indicate if all variables were successfully resolved, this enables
to stop the processing and prevent the possibility of error
notifications e.g. in case a user did not select a valid option.

Signed-off-by: Alvaro Sanchez-Leon <[email protected]>
@alvsan09 alvsan09 force-pushed the asl/resolve_debugger_command_variables branch from 11eba5d to e7edc77 Compare May 25, 2022 17:59
@alvsan09 alvsan09 merged commit 116a40c into master May 26, 2022
@alvsan09 alvsan09 deleted the asl/resolve_debugger_command_variables branch May 26, 2022 12:03
@github-actions github-actions bot added this to the 1.26.0 milestone May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug issues that related to debug functionality variable-resolver issues related to the variable-resolver extension vscode issues related to VSCode compatibility
Projects
None yet
2 participants