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

Associate root folder to dynamic debug configurations #12482

Merged
merged 1 commit into from
May 12, 2023

Conversation

alvsan09
Copy link
Contributor

@alvsan09 alvsan09 commented May 3, 2023

What it does

Fixes: #12352

The methods in the vscode interface DebugConfigurationProvider specify the parameter `folder: WorksapceFolder | undefined'.

The parameter can be undefined in the context of a folderless setup, however some plugins expect it to be present always (e.g. ms-python.python). The original implementation of dynamic debug configurations did not provide this parameter.

This change associates the root folder to the contributed configurations directly after the response for the call to 'provideDebugConfigurations' and it makes sure to preserve this parameter within the pick items presented to the user.

With the above, dynamic configurations are able to execute programs located inside the same root.

How to test

  1. Test that the plugin ms-python.python gets loaded and activated
  1. Run programs via extension provided dynamic debug configurations within the same root folder
    hello-multi-root-js.zip
  • Download the zip file above and using theia open the multi root workspace file included in the main folder
  • There are two root folders named original and copy, each root have its own src folder with js files ready to run
  • Follow the following recording to validate that you can run programs via extension provided dynamic configurations as long as they are located in the same selected root folder.
  • Verify that using a provided dynamic configuration to run a program in a different root folder fails.
    https://user-images.githubusercontent.com/76971376/236050594-6a7f0907-6e52-40ef-a3ee-83ded4ecad80.mp4
  1. Using quick access (e.g., typing ? from command palette), Verify the successful execution of programs via extension provided dynamic debug configurations (follow the video clip below),
    https://user-images.githubusercontent.com/76971376/236050617-59f796d5-eabf-4a9f-a01b-d31a07cec58c.mp4

Review checklist

Reminder for reviewers

@alvsan09 alvsan09 added vscode issues related to VSCode compatibility debug issues that related to debug functionality python issues related to the python language / extension plug-in system issues related to the plug-in system and removed vscode issues related to VSCode compatibility labels May 3, 2023
@alvsan09 alvsan09 force-pushed the asl/fixing_dynamic_debug branch from a04182b to f9112ae Compare May 3, 2023 21:38
@alvsan09 alvsan09 marked this pull request as draft May 3, 2023 22:19
@alvsan09 alvsan09 marked this pull request as ready for review May 4, 2023 00:03
@alvsan09 alvsan09 requested a review from martin-fleck-at May 4, 2023 16:37
@alvsan09 alvsan09 force-pushed the asl/fixing_dynamic_debug branch from f9112ae to 31a99fb Compare May 9, 2023 21:10
@alvsan09
Copy link
Contributor Author

alvsan09 commented May 9, 2023

The updated commit 31a99fb is just a re-base to latest master.

@alvsan09 alvsan09 requested a review from vince-fugnitto May 11, 2023 13:53
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirm that the changes work as expected 👍
I only have some minor feedback.

CHANGELOG.md Outdated Show resolved Hide resolved
packages/debug/src/common/debug-service.ts Outdated Show resolved Hide resolved
packages/debug/src/common/debug-service.ts Outdated Show resolved Hide resolved
packages/debug/src/node/debug-service-impl.ts Outdated Show resolved Hide resolved
@alvsan09 alvsan09 force-pushed the asl/fixing_dynamic_debug branch from 31a99fb to 26b2a6c Compare May 12, 2023 16:21
@alvsan09 alvsan09 requested a review from vince-fugnitto May 12, 2023 16:29
The methods is the `vscode` interface `DebugConfigurationProvider`
specify the parameter `folder: WorksapceFolder | undefined'.

The parameter can be `undefined` in the context of a folderless setup
however some plugins expect it to be present always
(e.g. ms-python.python). The original implementation of dynamic debug
configurations did not provide this parameter.

This change associates the root folder to the contributed
configurations directly after the response for the call to
'provideDebugConfigurations' and it makes sure to preserve this
parameter within the pick items presented to the user.

Signed-off-by: Alvaro Sanchez-Leon <alvaro.sanchez-leon@ericsson.com>
@alvsan09 alvsan09 force-pushed the asl/fixing_dynamic_debug branch from 26b2a6c to aad3b80 Compare May 12, 2023 16:32
@alvsan09
Copy link
Contributor Author

The latest commit, is just squashing of the two related commits

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

  • confirmed that the code looks good, builds, lints
  • confirmed that the python use-case works well
  • confirmed that the multi-root use-case works well

@alvsan09 alvsan09 merged commit 23e3df5 into master May 12, 2023
@alvsan09 alvsan09 deleted the asl/fixing_dynamic_debug branch May 12, 2023 17:27
@github-actions github-actions bot added this to the 1.38.0 milestone May 12, 2023
@alvsan09 alvsan09 mentioned this pull request May 16, 2023
11 tasks
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 plug-in system issues related to the plug-in system python issues related to the python language / extension
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Python plugin fails to activate (2022.16.1)
2 participants