-
Notifications
You must be signed in to change notification settings - Fork 797
debug: implement new prototype thin DA for delve #267
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
Conversation
This PR (HEAD: b979615) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: e200bcb) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
@polinasok FYI I'm still cleaning up this PR. Reverted package-lock, and now looking at some extra configs that sneaked into package.json |
After looking deeper into the |
This PR (HEAD: 37df287) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
|
This PR (HEAD: a0dedd0) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: b2f6ab4) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
Message from Polina Sokolova: Patch Set 6: (3 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Hyang-Ah Hana Kim: Patch Set 6: Run-TryBot+1 (5 comments) Thanks! Wonder how you like shorter directory/file names. Something like debugAdapter2 instead of debugAdapterDlvDap (and document it means the version that uses dlv dap), Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Eli Bendersky: Patch Set 6:
I don't mind the name very much. One issue with goDebug.ts is that it's the same name as the current DA, so two files with the same name, in different dirs. This makes it hard to distinguish them in open tabs in the editor, for example. Since the long-term maintenance of this DA will be in your team, I'll completely defer to your choice of file names here. If what I mentioned in the previous paragraph is not a concern, I'll rename to that. Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Hyang-Ah Hana Kim: Patch Set 6:
Thanks, let's rename the directory to debugAdapter2 for sure. If goDebug.ts is too confusing, I am fine with other name of your choice as long as it's not too long. Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
This PR (HEAD: 30a337d) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: 41d6bad) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: a547234) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
Message from Eli Bendersky: Patch Set 9: (6 comments)
Renamed Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Eli Bendersky: Patch Set 10: Thank you for all the comments. I hope I covered everything - with the Gerrit/GitHub split and all the renamed files it's becoming a bit challenging to track. Please let me know if I missed anything Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Hyang-Ah Hana Kim: Patch Set 11: (4 comments) I will let Polina do the final LGTM :-) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Polina Sokolova: Patch Set 11: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
This PR (HEAD: 25734e2) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
Message from Eli Bendersky: Patch Set 11: (3 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Polina Sokolova: Patch Set 12: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Eli Bendersky: Patch Set 12: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
This PR (HEAD: 0eef564) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
Message from Eli Bendersky: Patch Set 12: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
This PR (HEAD: ddd68dc) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/vscode-go/+/240417 to see it. Tip: You can toggle comments from me using the |
Message from Eli Bendersky: Patch Set 13: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Eli Bendersky: Patch Set 14: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Polina Sokolova: Patch Set 14: Code-Review+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Polina Sokolova: Patch Set 14: LGTM Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Message from Hyang-Ah Hana Kim: Patch Set 14: Code-Review+2 Please don’t reply on this GitHub thread. Visit golang.org/cl/240417. |
Updates #23 This DA serves as a DAP proxy between the editor and a DAP-capable Delve. Right now all it does is launch Delve (in DAP mode) when LaunchRequest is received from the editor, and then relays all messages between the editor and Delve back and forth. package.json section is copied from the current DA as-is. Some of its options will be removed and cleaned up in the subsequent PRs to make the diffs more obvious (see #271). Change-Id: I3493c5ee1d9e80071a17ea32c04a15eb021c8006 GitHub-Last-Rev: ddd68dc GitHub-Pull-Request: #267 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/240417 Reviewed-by: Polina Sokolova <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
This PR is being closed because golang.org/cl/240417 has been merged. |
These were copied over from the original package.json debug configuration in #267 Fixes #271 Change-Id: I424a5e7d28cade39b6defa4cdef57b5bedc0c30d GitHub-Last-Rev: 0e92c08 GitHub-Pull-Request: #290 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/241097 Reviewed-by: Polina Sokolova <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Take the waiting code from the existing adapter - try to connect after server emits to stdout. Also add comment in dapClient.ts for clarity. Updates #23 Follow up on #267 Change-Id: I29d0c87504464ce2ad0297ec0a5d7ad67c707b88 GitHub-Last-Rev: 13c423a GitHub-Pull-Request: #291 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/241117 Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Updates #23
This DA serves as a DAP proxy between the editor and a DAP-capable Delve. Right now all it does is launch Delve (in DAP mode) when LaunchRequest is received from the editor, and then relays all messages between the editor and Delve back and forth.
package.json section is copied from the current DA as-is. Some of its options will be removed and cleaned up in the subsequent PRs to make the diffs more obvious (see #271).