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

Close debug panel tabs when client stopped #20

Merged
merged 6 commits into from
Aug 22, 2024

Conversation

RemcoSmitsDev
Copy link
Owner

No description provided.

Instead of unwrapping send client stopped event to close all the panes that belong to that client
@RemcoSmitsDev RemcoSmitsDev changed the title Close debug panel when client stopped Close debug panes when client stopped Aug 18, 2024
@RemcoSmitsDev RemcoSmitsDev force-pushed the close-debug-panel-when-client-stopped branch from 854e505 to e95f9b6 Compare August 21, 2024 11:54
@RemcoSmitsDev RemcoSmitsDev marked this pull request as ready for review August 21, 2024 16:30
@RemcoSmitsDev RemcoSmitsDev changed the title Close debug panes when client stopped Close debug panel tabs when client stopped Aug 21, 2024
Copy link
Collaborator

@Anthony-Eid Anthony-Eid left a comment

Choose a reason for hiding this comment

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

Overall this is looking good!!

@@ -94,6 +94,9 @@ impl DebugPanelItem {
DebugPanelEvent::Output((client_id, event)) => {
Self::handle_output_event(this, client_id, event, cx)
}
DebugPanelEvent::ClientStopped(client_id) => {
Self::handle_client_stopped_event(this, client_id, cx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be more readable to call the function using this e.g this.handle_client_stopped_event(client_id, cx)

Not a blocking issue for a PR though

Copy link
Owner Author

Choose a reason for hiding this comment

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

We can change them, will do in another pr

client_id: &DebugAdapterClientId,
cx: &mut ViewContext<Self>,
) {
if Self::should_skip_event(this, client_id, this.thread_id) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as earlier this.should_skip_event(client_id, this.thread_id) is more readable. Also, I'm not sure it's necessary to pass thread_id to a struct's method when the struct is already aware of that value.

Copy link
Owner Author

Choose a reason for hiding this comment

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

The method requires it, but i could just check only client is myself. But i think this is more self explaining and its inline with the other event handles. So its more consistent

@RemcoSmitsDev RemcoSmitsDev merged commit 149116e into debugger Aug 22, 2024
0 of 4 checks passed
@RemcoSmitsDev RemcoSmitsDev deleted the close-debug-panel-when-client-stopped branch September 10, 2024 20:43
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.

2 participants