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

Allow debug widgets to get updated when not visible #8645

Merged

Conversation

federicobozzini
Copy link
Contributor

@federicobozzini federicobozzini commented Oct 16, 2020

What it does

This adds a property alwaysUpdate to TreeWidget that can be used to force a widget to update even when not visible or attached. This property is used by the Widgets showing debug information to get rendered even when not visible.

Fixes: #8639.

How to test

The easiest way to test this, would be to switch the active panel right after a debug session is terminated. This would normally prevent the debug information to get cleared, because of debug widgets not being visible. With this PR the debug widgets are correctly cleared when the debug session is terminated.

Review checklist

Reminder for reviewers

@kittaakos
Copy link
Contributor

kittaakos commented Oct 19, 2020

VS Code does it differently. When the debug session is over and the debug view was not active, Code leaves the trees untouched. The trees are cleaned up only when the debug view is activated again and there is not active debug session. See the screencast

screencast 2020-10-19 14-31-38

Why do not we do the same? If not necessary, let's not introduce another property on the tree-widget just because you want to work around something. (Especially, you can customize this behavior in your extension.) Can we handle this issue with onAfterShow on the DebugSessionWidget?

@kittaakos
Copy link
Contributor

TreeWidgets should be customized via TreeProps and not the way you did.

@federicobozzini
Copy link
Contributor Author

Why do not we do the same? If not necessary, let's not introduce another property on the tree-widget just because you want to work around something. (Especially, you can customize this behavior in your extension.)

Actually I've noticed that this issue is also present in vanilla Theia. All it takes to see it is too start a debug session, switch the active panel and then stop the active debug session by using Shift + F5. Once you go back to the Debug panel you can see that the debug panel is not correctly cleared.

Can we handle this issue with onAfterShow on the DebugSessionWidget?

@kittaakos I think this is a brilliant idea.

@federicobozzini federicobozzini force-pushed the debug-info-clearing branch 3 times, most recently from ae918b9 to b33bc25 Compare October 19, 2020 14:14
Signed-off-by: Federico Bozzini <federico.bozzini@gmail.com>
@federicobozzini
Copy link
Contributor Author

Before this change:
theia-debug-not-cleared

After this change:
theia-debug-cleared

@vince-fugnitto vince-fugnitto added the debug issues that related to debug functionality label Oct 28, 2020
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.

@federicobozzini the changes work very well for me and provide a much better debugging experience, similarly to vscode:

  1. open a *spec.ts file.
  2. start debugging with the run mocha test configuration.
  3. open the explorer.
  4. return to the debug view once the session has ended.
  5. the debug view is properly updated, while on master it is not.

@vince-fugnitto
Copy link
Member

@federicobozzini do you need help merging?

@federicobozzini
Copy link
Contributor Author

@vince-fugnitto Yes, please. I don't have the permissions to merge this PR.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Debug] Add the possibility to know when the debug widget has been cleared
3 participants