Add support for compound launches

[martin-fleck-at]

What it does

Fixes https://github.com/eclipse-theia/theia/issues/11302

• Extend debug model for compound launches -- Support pre-launch task, stopAll, list of configurations -- Properly parse compounds into configuration model -- Support mapping to custom session option type -- Support search of options by configuration, compound, and name

• Show compound launches in configuration dropdown -- Query all available options -- Use JSON (de-)serialization to allow multiple options with same name --- Previously custom string was used for identification

• Extend session manager to start compound session options

• Ensure compound sessions can also be found from DebugMain

I tried to keep backwards compatibility with interfaces and parameters as good as possible.

How to test

• If you start Theia with Theia itself as a workspace, you should be able to see the already present compound launches Launch Electron Backend & Frontend and Launch Browser Backend & Frontend. But you can also add your own compound launches and play around with the options (stopAll, preLaunchTask).
• To test the call from within a plugin, I added a small plugin that has a few commands to trigger the existing launch configurations (with or without workspace) or provide a custom launch configuration: https://github.com/martin-fleck-at/theia/tree/issue-11302-test

Review checklist

• [x] As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[JonasHelming]

@colin-grant-work It would help us a lot, if this could make it to the upcoming release. So if you have some cycles, we would be very thankful for a review. Thanks!

[martin-fleck-at]

@msujew Thank you very much for your fast and detailed review! I added another commit to it so you can see the changes but I can also squash them if you feel everything is taken care of.

[martin-fleck-at]

@colin-grant-work Thank you very much for your review! I pushed it as a separate commit so you can see the changes at a glance. If need be, I can squash the changes and push the whole thing again.

[martin-fleck-at]

@vince-fugnitto @alvsan09 Thank you very much for your input!

I tested it a few times with Launch Electron Backend & Frontend and for me the debug sessions do seem to stay interactive, especially when I cancel the debug session early on:

The only one that sometimes causes problems is the Attach to Plugin Host which sometimes remains in the debug view. This can happen when I try to stop the debug sessions by stopping any of the debug sessions but also happens when I simply close the started Electron window. I can also see the following warning in the output sometimes: Did not receive terminated event in time.. @vince-fugnitto Is this what you are also experiencing? Do you experience the same problems when quickly starting one debug session after another manually?

For simple cases everything seems to work as expected, e.g.,

{
  "configurations": [{
      "command": "top",
      "name": "Run top",
      "request": "launch",
      "type": "node-terminal"
  }],
  "compounds": [{
      "name": "Multi-Run",
      "configurations": [
        "Run top",
        "Run top"
      ],
      "stopAll": false
    }]
}

I think it may has to do with how the debug adapters and their environment are set up or maybe it is a timing issue and some launch configurations need to wait for others to be completely started? I checked the VSCode code and there they also don't have any special waiting mechanism, they simply use Promise.all like we do.

@alvsan09 Unfortunately, I cannot reproduce the TextEditor is disposed message but I think my setup might not be exactly the same. Does it cause any failures on using any of the text editors? Do you experience the same problems when quickly starting one debug session after another manually?

[martin-fleck-at]

@colin-grant-work Could you please have a look if you believe the reported issues are connected to this PR or some other setup issue that is only revealed through this PR? I already tested it with the pre-built Gitpod but I am not sure how else I can proceed here so any help would be greatly appreciated.

[martin-fleck-at]

@alvsan09 Thank you for your in-depth analysis! I agree that starting another Theia instance from Electron seems to cause a problem with the plugin host.

When I start the debug session I can see that the new Electron window opens normally and everything seems fine but after some time the 'Attach to Plugin Host' reports the following problem in the original window: Debug session initialization failed. See console for details.. Since we have the stopAll option set to true that stops all other debug sessions as well and the whole started Electron application closes/crashes. However, if we set stopAll to false, we can see that the window remains open as the other two launch configurations Launch Electron Backend and Attach to Electron Frontend do not have a problem. I know to little about the Plugin host to actually know how the attachment works and how that problem could be fixed if it has some deep underlying issue. I tried changing the port from 9339 to 9340 but that didn't solve the issue.

Still I would argue that this is a problem specifically to the plugin host, so maybe this could be considered as a separate bug?

[martin-fleck-at]

I just checked and the problem with 'Attach to Plugin Host' also happens when I start all three launch configurations manually. I therefore created a new bug for it here: https://github.com/eclipse-theia/theia/issues/11475. I think I addressed all other comments so far so is there any chance we can still get this merged before the release? An adopter would really appreciate that.

[alvsan09]

Hi @martin-fleck-at , Thanks for writing the issue above.

I just saw the following problem, when toggling stopAll in the editor, in the following, the Auto save option is on, but changes to it does not always take effect on the next launch.

https://github.com/alvsan09/hello_express

[martin-fleck-at]

@alvsan09 Oh wow, great catch, thank you! Seems like I removed something when updating the current configuration and so changed properties were not correctly applied.

[alvsan09]

@martin-fleck-at, The latest change is breaking non compound configs, see:

[martin-fleck-at]

@alvsan09 Oh wow, you are right! I am very sorry for missing that! Seems like the typeguards were not good enough to detect a dynamic configuration vs a regular configuration. I just checked whether the property was there but not if that property was set.

[alvsan09]

@martin-fleck-at , I saw one more problem, we are no longer able to select dynamic configurations, see gif below:

[martin-fleck-at]

@alvsan09 I updated the search for the current configuration again, I think now all cases should be covered. Thank you very much for taking the time!

[alvsan09]

@martin-fleck-at , The latest change fixes the previous issues, unfortunately, I found new ones:

1. When starting a workspace used in a previous version of theia where that workspace had registered dynamic configurations in local storage, the configurations will be restored with a label of undefined.

2. From the debug view, selecting Add Configuration has no effect

3. When reloading the browser, sometimes the selection box in the debug view go out of bounds, I tried reproducing this one on master but I couldn't, with the new change I had to try two or three times and it will happen.

See the gif below:

[martin-fleck-at]

@alvsan09 Thank you again for your feedback!

1. I fixed the naming issue, there was indeed a change as we now use a different serialization so the name was not properly recognized.
2. For me 'Add Configuration...' opens the launch.json file as expected both in Chrome and in Firefox.
3. I also cannot reproduce the select rendering issue. I tried several times in Chrome and in Firefox but it always renders to the bottom.

Could you please give it another try? I don't know what could have caused this but I cannot reproduce it at all. What browser are you using?

[alvsan09]

@martin-fleck-at , I tried the correction for 'undefined' for the names on restored dynamic configs and it works fine now, :+1:

Could you please give it another try? I don't know what could have caused this but I cannot reproduce it at all. What browser are you using?

For the two remaining issues, I managed to reproduce them using a mult-root workspace, and the amount of entries in the configuration is larger e.g.. all the ones provided by theia + all the ones provided by the hello_express example mentioned above.

The following clip shows the difference when having a folder vs a workspace project.

https://user-images.githubusercontent.com/76971376/181771509-74032bfb-10bf-40c5-9154-9f2e3d6230e1.mp4

[martin-fleck-at]

@alvsan09 With the multi-root workspace service I can indeed reproduce the problem! Seems like when we need to select a workspace the update of the configurations dropdown takes focus and automatically closes the workspace select, I now keep it open even on focus lost.

As for the rendering "issue": This really depends on how much space is available at the top or bottom. Previously, if there was not enough space at the bottom, it was rendered to the top. I changed that so that it prefers bottom rendering unless more content can be shown when rendering to the top.

Hopefully this will resolve the remaining issues. In any case, I really appreciate you sticking with me on this ;-)

[alvsan09]

@martin-fleck-at I have done a regression test and almost everything seems to work as far as I can tell, Except the following issues (related to recent dynamic configurations from old format)

The video starts showing a set of dynamic launch configurations which were created on a previous session with Theia master, then we launch each of them and duplicates can sometimes be created.

https://user-images.githubusercontent.com/76971376/182202236-6ca35688-2631-45f5-9b64-d356e77788a2.mp4

I saw a strange behaviour once before where a non dynamic config. managed to get inserted among the recent dynamic configurations, I am not sure how to reproduce it, although I managed to get a screen shot of the serialized data. See Attach to node process

[alvsan09]

As for the rendering "issue": This really depends on how much space is available at the top or bottom. Previously, if there was not enough space at the bottom, it was rendered to the top. I changed that so that it prefers bottom rendering unless more content can be shown when rendering to the top.

@msujew, Do you agree with the change of approach affecting the custom Select Component widget ?

[msujew]

@alvsan09 @martin-fleck-at Yeah, the changes look reasonable to me. Everything in regards to the select component still works as expected or even better 👍

[martin-fleck-at]

@alvsan09 I hope I managed to remove the last issues now. When restoring the old dynamic configurations I now ensure that the options name is properly derived if it is not set already. I also filter for dynamic options to ensure nothing else gets into that list (even though I was not able to reproduce that locally).

Interestingly, the fix showed an issue which probably was the reason for the initial custom serialization: When we set the current value in the debug session manager, the value we have in current does not necessarily match exactly one of the options we have on the configuration drop down, mostly due to the fact that properties are in different order. So I now aim to match the current value to the current option before when updating the select. As far as I can tell, this now works for all option types and updates in the launch.json are also properly considered.

[alvsan09]

Using the last commit I have verified that the functionality works as expected ! :+1:

@vince-fugnitto and @colin-grant-work, any pending items on your side ?

I have successfully run the tests below:

• Storing / Restoring configurations as current for (dynamic, not dynamic & compound)
• Storing / Restoring recently used dynamic configurations
• Restoring configurations from previous theia version
• Launching configuration by 'debug ' command
• Launching compound configurations - stopAll [true, false] - toggling stopAll takes effect at the next launch - with a preLaunchTask
• Launching not compound configurations
• "Add configuration" works
• The above works for multi-root as well as for project folders

[martin-fleck-at]

@colin-grant-work @msujew Any objections to merging this?

[colin-grant-work]

Any objections?

None from me.