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

Fix output that was produced before initial stop was not visible at first stop #57

Merged
merged 5 commits into from
Nov 15, 2024

Conversation

Anthony-Eid
Copy link
Collaborator

@Anthony-Eid Anthony-Eid commented Oct 30, 2024

This PR allows you to see the output of your program that was produced before the initial breakpoint stop.
Before this change, the output was not visible because we only stored the output for threads that have stopped once. Because we store the output inside the debug_panel_item that is only created when the debug session has stopped.

So by storing the output events and emitting the events when the thread stopped, allows you to see all the output that was produced before the stop.

@RemcoSmitsDev
Copy link
Owner

One thing we might, should consider:
when the client never stopped, should we push the logs to the debug/RPC messages panel?
We have to think about a good solution here, so we can always see the logs without needing to open another panel.

@Anthony-Eid
Copy link
Collaborator Author

@RemcoSmitsDev

That's a good point! I know that vscode has their debug console as a separate pane than it's debugging pane, but that's not necessary the best solution.

I like your idea better! We should send a toast notification to workspace with a url/button that opens the DAP logs editor that was recently merged. We could also add the ability to filter log breakpoint's to the log editor and set that behavior as the default when a user opens DAP logs from the toast notification.

@RemcoSmitsDev
Copy link
Owner

Yeah one more thing that came up, we have to support viewing deb logs from an adapter that has already exited/terminated. Right now we remove the debug log entries.

@Anthony-Eid
Copy link
Collaborator Author

What if we merge this in now then I could work on setting up a history for the DAP logs?

item.client_id() == *client_id
});

if existing_item {
Copy link
Owner

Choose a reason for hiding this comment

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

Can we pls just always send the event and push the message queue? And when the client has stopped (shutdown) we should remove the queue again. Because you could have multiple debug sessions and each debug session should show the same output log

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes! I'll make the change.

Copy link
Owner

@RemcoSmitsDev RemcoSmitsDev left a comment

Choose a reason for hiding this comment

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

Can we pls just always send the event and push the message queue? And when the client has stopped (shutdown) we should remove the queue again. Because you could have multiple debug sessions and each debug session should show the same output log

We still want all the output in new threads
@RemcoSmitsDev RemcoSmitsDev changed the title Fix log breakpoint output bug when hit before any breakpoints Fix output was not visible before a thread initial stop Nov 15, 2024
@RemcoSmitsDev RemcoSmitsDev changed the title Fix output was not visible before a thread initial stop Fix output that was produced before initial stop was not visible at first stop Nov 15, 2024
@RemcoSmitsDev RemcoSmitsDev merged commit 3a77d7a into RemcoSmitsDev:debugger Nov 15, 2024
9 checks passed
@RemcoSmitsDev RemcoSmitsDev deleted the log-breakpoints branch November 15, 2024 20:45
cx.emit(DebugPanelEvent::Output((client_id, output)));
if let Some(message_queue) = this.message_queue.get(&client_id) {
while let Some(output) = message_queue.iter().next() {
cx.emit(DebugPanelEvent::Output((client_id, output.clone())));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need a way to filter messages by thread_id to stop log messages from one thread outputting to another thread.

I also think it could be worth looking into sharing the same View<Console> between thread's of the same client_id

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah so that is not really possible because the output does not belong to a thread it belongs to a client. We could add some indicator so you can see the logs since the thread stopped.

Yeah wanted to do that as well.

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