GH-7909: Inline variable values in editor.

[kittaakos]

Closes #7909.

Signed-off-by: Akos Kitta [email protected]

What it does

How to test

• Create a JS module, set a few breakpoints here and there, steps through your module, you can see the inline value decorations,
• Start another debug session while the previous is up and running, you can see the value decorators get updated when you alter the active debug session,

• Terminate the debug session, decorators are removed.

• Also, close the editor you debug, but do not terminate the debug session. When you reopen the file, the inline value decorations are there.

Review checklist

• 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

[kittaakos]

This PR is ready for the review/preview.

[kittaakos]

I am happy to drop the fix for #7918.

[akosyakov]

I am happy to drop the fix for #7918.

I think this state does not belong to the widget, but DebugConfigurationManager. Otherwise there can be race conditions when proper current from the manager is ignored because there is state to restore in the widget. Could we rather implement it there, i.e. store when the current is changed and read at proper time when current is accessed. Maybe with another PR?

[kittaakos]

I think this state does not belong to the widget, but DebugConfigurationManager. Otherwise there can be race conditions when proper current from the manager is ignored because there is state to restore in the widget. Could we rather implement it there, i.e. store when the current is changed and read at proper time when current is accessed. Maybe with another PR?

Sure. I hated to reset the desired launch config during the development, so that's why the messy fix.

Regarding the required changes; it is a little bit tricky. There is no clear event on the DebugSessionManager when were the configs loaded. First, we have a config for the current workspace root without any config (we make initialized resolve), then we have a preference change event and we populate the configs for the workspace root.

[akosyakov]

I meant we could preserve current in https://github.com/eclipse-theia/theia/blob/master/packages/debug/src/browser/debug-configuration-manager.ts and restore it on the initialization, i.e. on first updateModels.

[kittaakos]

I meant we could preserve current in https://github.com/eclipse-theia/theia/blob/master/packages/debug/src/browser/debug-configuration-manager.ts and restore it on the initialization, i.e. on first updateModels.

Sure sure, I understood it, as we do for the editor navigation history. I will open a separate PR for that.

[akosyakov]

Code changes look good. I've tried to debug our Mocha tests and inline variable values are properly rendered as I step over.

The only question whether we need a CQ for copied code.

[kittaakos]

The only question whether we need a CQ for copied code.

I am going to take care of it.

[kittaakos]

@marcdumais-work, can you please confirm whether we need a new CQ for this PR or not? Thank you!

theia/packages/debug/src/browser/model/debug-stack-frame.tsx

Line 99 in 9a29154

// https://github.com/theia-ide/vscode/blob/standalone/0.19.x/src/vs/workbench/contrib/debug/common/debugModel.ts#L324-L335

theia/packages/debug/src/browser/editor/debug-inline-value-decorator.ts

Line 21 in 9a29154

// Based on https://github.com/theia-ide/vscode/blob/standalone/0.19.x/src/vs/workbench/contrib/debug/browser/debugEditorContribution.ts

[marcdumais-work]

@marcdumais-work, can you please confirm whether we need a new CQ for this PR or not?

Hi @kittaakos ,

I believe so - AFAIK we've never requested these files examined for "forking" (CQ type B). Please use links to the upstream vscode repo when opening the CQ.

[marcdumais-work]

Also, please add the Microsoft copyright header in file: debug-stack-frame.tsx like you did for the other file.

[kittaakos]

Also, please add the Microsoft copyright header in file: debug-stack-frame.tsx like you did for the other file.

Good point, thanks!

[kittaakos]

CQ: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22247

[marcdumais-work]

I gave this PR a quick try on Gitpod. I installed vscode-clangd and cdt-gdb-vscode from the extensions view and used the test workspace and launch config from here.

It worked nicely!

[kittaakos]

@marcdumais-work, how long does it usually take to get a CQ approved? I have seen an almost prompt approval (with the + sign) on Bugzilla, but nothing has happened since then. The workflow state is the same. I do not have much experience with CQs 😕 could you please check whether the one for this PR is going in the good direction. Thank you!

[marcdumais-work]

@marcdumais-work, how long does it usually take to get a CQ approved? I have seen an almost prompt approval (with the + sign) on Bugzilla, but nothing has happened since then. The workflow state is the same. I do not have much experience with CQs could you please check whether the one for this PR is going in the good direction. Thank you!

The CQ above looks good - it has one approval so far from a PMC member. It has been opened 2 work days ago, so I'd give it a couple more, and then "ping" on the CQ itself if still unresolved.

[kittaakos]

@marcdumais-work, does it make sense to ask about the CQ on Bugzilla?

[marcdumais-work]

@marcdumais-work, does it make sense to ask about the CQ on Bugzilla?

I am not sure that there is a better way. I remember @akosyakov having some success with pinging. Let me give it a try.

[akosyakov]

Maybe already vacation time 🌴

[kittaakos]

We have an update, I can see:

You may proceed to check the content into the Eclipse repository.

And also

We will perform a comprehensive analysis at a later date based on IP work queue.

Is it yes or no? 😕

[akosyakov]

We can merge when CQ has checkin label. It does, so you can proceed!

[kittaakos]

Thank you all for the help on this PR.