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 extra prompting and manual debugger commands #1778

Merged
merged 2 commits into from
Apr 27, 2022

Conversation

andyleejordan
Copy link
Member

This requires that we set the REPL to exit correctly. Note that we don't
have a notion of "disconnect" so we removed that logic. We are assuming
the debugger is "active" a little earlier now, that is, also when we've
received an LSP notification to start it (via a launch or attach
handler). Because we receive a notification on "disconnect" even for a
script just finishing, we need to know that the server is not active and
so not cancel the current prompt.

This requires that we set the REPL to exit correctly. Note that we don't
have a notion of "disconnect" so we removed that logic. We are assuming
the debugger is "active" a little earlier now, that is, also when we've
received an LSP notification to start it (via a launch or attach
handler). Because we receive a notification on "disconnect" even for a
script just finishing, we need to know that the server is not active and
so not cancel the current prompt.
@andyleejordan
Copy link
Member Author

andyleejordan commented Apr 27, 2022

@SeeminglyScience Just added the commit that removes the WriteWithPrompt workaround from the andschwa/improvements branch (and originally explored in #1769). It's just cleaner now, the first commit is obviously the actual fix. Now the existing prompt gets used correctly by whatever we're about to run.

@andyleejordan
Copy link
Member Author

Kind of funny cosmetic bug: if you run a script, and then type input at the next prompt, and then run the script again, then PSReadLine doesn't save that input. But if you've invoked anything else in between (or type that text before running the script) it works fine. Strange, we'll fix it later.

Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM but one thing. HMU if you want to keep it, doing request changes to not trigger auto merge

@andyleejordan andyleejordan force-pushed the andschwa/fix-debugger branch from 2eb92ae to e968d8b Compare April 27, 2022 22:45
@andyleejordan andyleejordan force-pushed the andschwa/fix-debugger branch from e968d8b to 3cba30c Compare April 27, 2022 22:47
@SeeminglyScience SeeminglyScience self-requested a review April 27, 2022 23:00
@andyleejordan andyleejordan merged commit 381e7fe into master Apr 27, 2022
@andyleejordan andyleejordan deleted the andschwa/fix-debugger branch April 27, 2022 23:16
@SydneyhSmith SydneyhSmith mentioned this pull request Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants