-
Notifications
You must be signed in to change notification settings - Fork 169
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
Make output visible for delve #505
Conversation
This pull request introduces 1 alert when merging 77f21dd into 9e833b2 - view on LGTM.com new alerts:
|
77f21dd
to
268ae97
Compare
The delve-dap debug adapter doesn't send its output to the correct channel via dap, rather prints to its own stdout. This behaviour is buggy, but it seems that it's the job of every DAP client to fix it rather than fixing it at source. My solutin is to allow adapter specs to say that they need to be run in a terminal rather than a background job. This would in theory allow for stdin to work too. An alternative, or perhaps complementary, solution would be to attach the stdin/out of the 'channel-based server job' to a buffer and inlcude that in the Output window.
268ae97
to
7bb4754
Compare
This pull request introduces 1 alert when merging 9e70896 into 9e833b2 - view on LGTM.com new alerts:
|
For cpptools, just use x86 binraies, as that apparently works. For lua and c-sharp, skip.
b317417
to
4fcd07d
Compare
4fcd07d
to
b8504d8
Compare
I had to drop support for the output window because of some weird deadlock where the channel isn't closing properly on shutdown, although the job is stopped. I think the terminal version is better anyway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 6 files at r1, 15 of 17 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @puremourning)
Fixes #504
There are two mechanisms implemented here - using a terminal buffer and using the Output window.
I'm actually not sure which is better. For now tty is the default for delve. This can be decided by using the following:
Setting "tty" to
false
will instead put the output in a new "Adapter" tab in the Output window.