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.

Sign up for GitHub

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

Jump to bottom

debug: escape hatch for arbitrary dlv flags #978

Closed
polinasok opened this issue Dec 1, 2020 · 4 comments
Closed

debug: escape hatch for arbitrary dlv flags #978

polinasok opened this issue Dec 1, 2020 · 4 comments
Assignees
@hyangah @suzmue
Labels
Debug Issues related to the debugging functionality of the extension. FrozenDueToAge
Milestone
v0.21.0

Comments

@polinasok
Copy link
Contributor

polinasok commented Dec 1, 2020 •
edited
Loading

Dlv has the following command-line arguments: https://github.com/go-delve/delve/blob/master/Documentation/usage/dlv.md#options. For some we have launch.json equivalents that are mapped by the debug adapter. In particular,

  • showLog => --log
  • logOutput => --log-output
  • init => --init
  • backend => --backend
  • output => --output

For the rest, the only option the users have is to launch an instance of --headless delve from the terminal and then vscode and launch.json to connect to it. This is especially annoying for newly added flags based on popular demand (e.g. #810). We might or might not want to keep up with all the flags in our mapping, but in addition we should add an arbitrary string field to specify whatever flags users want to add to the dlv command as-is.

Related points that probably deserve their own issues:

  • The above property-to-dlv-flag mapping should be made clear by the property comments in package.json, especially if we add an arbitrary option, so there are no surprises when users specify the same flag via both ways.
  • With dlv-dap, we have an opportunity to revisit the names and conventions - should we follow the same pattern as we do for the existing adapter?
@polinasok polinasok added Debug Issues related to the debugging functionality of the extension. DelveDAP labels Dec 1, 2020
@polinasok polinasok added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 7, 2020
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/280697 mentions this issue: src/debugAdapter: add dlvFlags to accept arbitrary delve flags

@hyangah
Copy link
Contributor

hyangah commented Dec 30, 2020

I encountered this issue while testing go1.16 beta - we need to pass --check-go-version=false to test go1.16 beta now.

gopherbot pushed a commit that referenced this issue Jan 8, 2021
@hyangah
src/debugAdapter: add dlvFlags to accept arbitrary delve flags
cda36be 
Added the same to src/debugAdapter2. We don't do any flag validation
- that will be done by dlv. We place the user-specified flags before
the flags internally set up, so, when users specify flags that are used
internally (e.g. --listen) or that are configured through the debug
configuration properties (e.g. --log-output) as well, the flag values
internally set up can be picked up unless dlv complains about the
duplicated flags.

This flag property and other properties related to dlv flags are
currently passed as part of launch request args because the legacy DA
launches the delve process upon receiving the launch request. However,
I am not sure if that's still true when we switch to the delve DAP mode
and we can launch the delve DAP directly from the extension. For now,
I added dlvFlags to the delve DAP mode thin adapter too (like other
flag-related properties).

Fixes #810
Updates #978

Change-Id: Iaab6097d3357ccafc274cb0a4fc970f2b6945b66
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/280697
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>
Trust: Hyang-Ah Hana Kim <[email protected]>
Trust: Suzy Mueller <[email protected]>
Reviewed-by: Polina Sokolova <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
@hyangah hyangah added this to the v0.21.0 milestone Jan 14, 2021
@hyangah
Copy link
Contributor

hyangah commented Jan 15, 2021

v0.21.0 will have dlvFlags attributes.

@hyangah hyangah closed this as completed Jan 15, 2021
@polinasok polinasok added FixedInBothDAs and removed NeedsFix The path to resolution is known, but the work has not been done. labels Feb 3, 2021
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/288954 mentions this issue: src/goDebugFactory.ts: run dlv-dap as a server and connect as server

gopherbot pushed a commit that referenced this issue Feb 8, 2021
@suzmue
src/goDebugFactory.ts: run dlv-dap as a server and connect as server
df853b9 
Connect to the dlv dap server directly from vscode, bypassing the
thin adapter.

This is accomplished using the vscode.DebugAdapterDescriptorFactory
for launch configurations of type "godlvdap".

Updates #23
Updates #978

Change-Id: I877a1c1b0cf0c40a2ba0602ed1e90a27d8f0159e
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/288954
Trust: Suzy Mueller <[email protected]>
Trust: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Suzy Mueller <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
@golang golang locked and limited conversation to collaborators Feb 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@hyangah hyangah

@suzmue suzmue

Labels
Debug Issues related to the debugging functionality of the extension. FrozenDueToAge
Projects
None yet
Milestone
v0.21.0
Development

No branches or pull requests
4 participants
@hyangah @suzmue @gopherbot @polinasok