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

Add support for theia.DebugSession.parentSession #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tsmaeder
Copy link
Collaborator

@tsmaeder tsmaeder commented Nov 24, 2022

What it does

Add suppport vor DebugSession.parentSession. Fixes eclipse-theia#11512

Adds support for the parentSession property in theia.d.ts. Includes some minor changes to make sure we're not sending theia.DebugSession object over the wire, but only session ids.

Contributed on behalf of ST Microelectronics

How to test

  1. Install the vscode extension from here: https://github.com/tsmaeder/castletest (I've attached a pre-built copy to this issue).
  2. Run Theia and open a copy of the theia source code
  3. Run the "Run Browser Backend and Frontend" launch configuration
  4. Select various debug sesssions and run the "Show active debug session" command from the command palette.
  5. Observe: you should see the name of the correct parent session printed to the output of the Theia instance under test

Review checklist

Reminder for reviewers

Adds support for the parentSession property in theia.d.ts. Includes
some minor changes to make sure we're not sending theia.DebugSession
object over the wire, but only session ids.

Contributed on behalf of ST Microelectronics

Signed-off-by: Thomas Mäder <[email protected]>
@tsmaeder
Copy link
Collaborator Author

@planger how about a review?

@tsmaeder tsmaeder closed this Nov 28, 2022
@tsmaeder tsmaeder reopened this Nov 28, 2022
@tsmaeder
Copy link
Collaborator Author

@sgraband this is an updated version of this PR: #53. I noticed a problem (debug session objects being sent over the wire) and did a better version. Could you have another look before I open a PR on the main Theia repo?

@sgraband sgraband self-requested a review November 28, 2022 11:17
@planger
Copy link

planger commented Nov 28, 2022

Since @sgraband is looking at this change anyway, I think it makes more sense that he reviews this change rather than me.
Thanks @tsmaeder and @sgraband!

@sgraband
Copy link

Install the vscode extension from here: https://github.com/tsmaeder/castletest (I've attached a pre-built copy to this issue).

@tsmaeder I cannot see a pre-built copy of the extension on this PR or the issue. Am i not seeing something? 👀

Copy link

@sgraband sgraband left a comment

Choose a reason for hiding this comment

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

Thank you!

Apart from the missing pre built extension i mentioned above this looks and works great!

@tsmaeder
Copy link
Collaborator Author

I cannot see a pre-built copy of the extension on this PR or the issue. Am i not seeing something?

Promises, promises! Here you go:
castletest-0.0.1.zip

@sdirix sdirix changed the base branch from master to 0.6.1 November 28, 2022 12:09
@sdirix sdirix changed the base branch from 0.6.1 to master November 28, 2022 12:09
Copy link

@sgraband sgraband left a comment

Choose a reason for hiding this comment

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

Great! Thank you! I already tested it, as i still had the extension from my earlier review, but the theia main repo reviewer will probably not have it 😉

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vscode] Support optional property DebugSession.parentSession
3 participants