debug: support suspendDebuggee

[polinasok]

microsoft/debug-adapter-protocol/issues/177 added suspendDebuggee to the protocol, but as far as I can tell it is not supported by the vscode UI. I can toggle to enable/disabled terminateDebuggee only.

[roblourens]

cc @weinand

[weinand]

Yes, VS Code does not yet surface suspendDebuggee in the UI.

[polinasok]

You could have more options in the toggle: Stop (default for launch), Disconnect+Resume (default for attach), Disconnect+Suspend (new option).

[polinasok]

This has come up again for us. Anything in the works by any chance to add the UI option for suspending or restarting the debuggee on disconnect?

[weinand]

@polinasok you are talking about this, right?

[polinasok]

This is the control you added to support terminateDebuggee, and it's been great. Stop = terminate debuggee (default for launch). Disconnect = don't terminate (default for attach). Our users also need to be able to specify leaving the debuggee and having it resume execution vs stay suspended once the debug session disconnects.

[polinasok]

Just wanted to add that with three different options (and possibly even with two) it might be a good idea to have the end-session options side-by-side instead of having the users toggle with a key.

EDIT: Writing this made me realize that adapters like ours might need to communicate supportsTerminateDebuggee and supportsSuspendDebuggee not at initialization, but after launching/attaching. Not all of them will be supported depending on how things are launched/attached, so not all of them should be offered to the user whether in a side-by-side or toggle form.

[weinand]

@polinasok thanks for the additional info.

Here are the three possible disconnect options for the two debug configuration request types:

• "launch" request type:
  • Stop (default)
  • Disconnect + Resume
  • Disconnect + Suspend
• "attach" request type:
  • Stop
  • Disconnect + Resume (default)
  • Disconnect + Suspend

Now we have to find a new UX because the existing alt-modifier approach doesn't work for three options.
Just using a second modifier for the resume/suspend toggle is a bit awkward...

@roblourens @connor4312 @polinasok any ideas?

[polinasok]

Yes. The extra symbols should only be displayed displayed in response to supportsXYZ capabilities. So in some cases, there might be only one or two.

How about 1-3 side-by-side action controls? Maybe represent the disconnect options with hybrid icon combining "disconnect" and "pause"/"continue"?

[connor4312]

Hm, I suppose having a new icon appear when alt is pressed is out of the question. Maybe have the stop button be one of the new dropdown buttons with actions?

[weinand]

Yes, a dropdown button could work.

@polinasok should the button remember the last choice or should it always fall back to the default action?

[polinasok]

Hmm, that's a good question. I can think of pros and cons for both...
Out of curiosity, what's the rationale for favoring drop downs and key triggers as opposed to adding more option to the action bar?

[polinasok]

Thank you for adding this to the April rotation. We are very much looking forward to this feature instead of defining private clunky workarounds.

[hyangah]

Here are the three possible disconnect options for the two debug configuration request types:

• "launch" request type:

  • Stop (default)
  • Disconnect + Resume
  • Disconnect + Suspend

What are these launch + Disconnect options for?
Are we supposed to leave the debugee process (go program) and the debugger (e.g. dlv) being orphaned?

[weinand]

@hyangah "Disconnect on launch" means the same as the "Disconnect on attach": don't kill the debuggee but just disconnect the debugger from it. With the "... Resume" and "... Suspend" variants you can control whether the debuggee will continue to run or stays in the suspended state.

What to do with the debugger I don't know because VS Code knows nothing about debuggers. Debuggers are an implementation detail of a debug adapter or debug session. I suggest that you ask @polinasok for details.

[hyangah]

Thanks @weinand - Yes, I understand VS Code knows nothing about debuggers.

@polinasok do you plan to enable this capability only for "attach" request type in delve? (maybe send the capability update event?) I don't think this is a viable option for launch requests in Go extension but causes user confusion.

[polinasok]

@hyangah There are already two options for launch, regardless of supportsTerminateDebuggee value, - Stop and Disconnect, which is not right for our case. I have already updated delve to only enable this capability for attach (go-delve/delve#2940) and filed an issue against VS Code to tailor the UI to the capabilities as opposed to always offering all options (#134412). That issue confirms that this will be addressed as part of revisiting all these controls. I plan to handle supportsTerminateDebuggee customization the same way - enabling in remote attach only.

[hyangah]

Thanks @polinasok. Great to hear that this is enabled only on attach.

One more UX question:

• Unlike command line-based debugging tool like dlv or gdb that I believe this suspendDebugee feature was based on, vscode debugging UI allows to disconnect while the debugee is running. If the debugee is running, can users assume the disconnect+suspend request will pause the debugee's execution?
  • If yes, I think "Suspend & Disconnect" is clearer if there is any text description.
  • If not, I think it's better not to present the "disconnect+suspend" option when the debugee is running.

@weinand It looks like DAP spec's "Disconnect Request" section needs update to reflect this change #134412 (comment) . Currently the spec says:

In addition, the debug adapter must terminate the debuggee if it was started with the ‘launch’ request. If an ‘attach’ request was used to connect to the debuggee, then the debug adapter must not terminate the debuggee.

[weinand]

@hyangah I think the spec covers that already:

[weinand]

@roblourens Looks great - ship it!
and yes: let's keep the alt behavior.

[polinasok]

What will alt do? How will one get to Stop in this case?

Also will the options differ, when state is running vs stopped?
Running showing:

• Disconnect
• Suspend & Disconnect

Stopped showing

• Resume & Disconnect
• Disconnect

[polinasok]

Oh, an what will this look like if suspendDebuggee is not enabled? And what if it is, but terminateDebugee isn't?

[weinand]

What will alt do? How will one get to Stop in this case?

The "Alt" modifier key will temporarily flip the button between Stop and Disconnect without the need for selecting from the drop down. It is basically a shortcut and exists for backward compatibility (with user's muscle memory).

Also will the options differ, when state is running vs stopped?

No, we do not want to track suspend state in VS Code.
If the user selects "suspend", the value "true" will be passed for the corresponding flag.

... what will this look like if suspendDebuggee is not enabled? And what if it is, but terminateDebugee isn't?

if there are no options enabled, then the button will work like before: no dropdown.

[polinasok]

What will alt do? How will one get to Stop in this case?

The "Alt" modifier key will temporarily flip the button between Stop and Disconnect without the need for selecting from the drop down. It is basically a shortcut and exists for backward compatibility (with user's muscle memory).

I am clearly missing something. Isn't square used for Stop and plug used for disconnect with alt?
For attach, I would therefore expect a plug with drop-down options by default, and square with no drop-down with alt.
Or is the square the 3rd option in addition to drop-down values? Then I don't even know what alt would do.

Also will the options differ, when state is running vs stopped?

No, we do not want to track suspend state in VS Code.

You do to decide when to enable pause and when to enable continue and step controls, don't you?

If the user selects "suspend", the value "true" will be passed for the corresponding flag.

I understand that. But without the extra details, it is not clear that one option guarantees suspended state while the other option does the opposite as opposed to preserving whatever state things are in and only disconnecting.

if there are no options enabled, then the button will work like before: no dropdown.

Can you please help me populate this, so we are all on the samee page?

• supportsTerminateDebuggee=false supportsSuspendDebuggee=false

  • launch:
    • stop (square)
  • attach
    • disconnect (plug)

• supportsTerminateDebuggee=true supportsSuspendDebuggee=false

  • launch:
    • stop (default)
    • alt => disconnect
  • attach
    • disconnect (default)
    • alt => stop

• supportsTerminateDebuggee=false supportsSuspendDebuggee=true

  • ???

• supportsTerminateDebuggee=true supportsSuspendDebuggee=true

  • launch:
    • stop (default)
    • alt => disconnect with drop down
  • attach
    • disconnect with drop-down (default)
    • alt => stop

[weinand]

... Isn't square used for Stop and plug used for disconnect with alt?

with supportsTerminateDebuggee == false:

• "red square" is disconnect() in "launch" mode. DA should interpret this as "stop debuggee"
• "red plug" is for disconnect() in "attach" mode. DA should interpret this as "disconnect from debuggee"
• VS Code has bug #145650 here: the "Alt" modifier key should not be honoured if supportsTerminateDebuggee == false or missing.

with supportsTerminateDebuggee == true:

• "red square" is disconnect(terminate=true). DA must interpret this as "stop debuggee"
• "red plug" is for disconnect(terminate=false). DA must interpret this as "disconnect from debuggee"
• VS Code assigns "red square" in "launch" mode and "red plug" in "attach" mode.
• Pressing the "Alt" modifier key flips this assignment temporarily (only while the "Alt" key is pressed).

Can you please help me populate this, so we are all on the same page?

• supportsTerminateDebuggee=false supportsSuspendDebuggee=false

  • launch:
    • square: -> disconnect()
    • no dropdown
  • attach:
    • plug: -> disconnect()
    • no dropdown
  • Alt modifier key: not honored

• supportsTerminateDebuggee=true supportsSuspendDebuggee=false

  • launch:
    • button(=dflt): shows "square" icon -> disconnect(terminate=true)
    • dropdown:
      • "stop" -> disconnect(terminate=true)
      • "disconnect" -> disconnect(terminate=false)
  • attach:
    • button(=dflt): shows "plug" icon -> disconnect(terminate=false)
    • dropdown:
      • "disconnect" -> disconnect(terminate=false)
      • "stop" -> disconnect(terminate=true)
  • Alt modifier key: flips the button's icon and semantics between "launch" and "attach"

• supportsTerminateDebuggee=false supportsSuspendDebuggee=true

  • launch:
    • button(=dflt): shows "square" icon -> disconnect(suspend=false)
  • attach:
    • button(=dflt): shows "plug" icon -> disconnect(suspend=false)
    • dropdown:
      • "disconnect" -> disconnect(suspend=false)
      • "disconnect and suspend" -> disconnect(suspend=true)
  • Alt modifier key: not honored

• supportsTerminateDebuggee=true supportsSuspendDebuggee=true

  • launch:
    • button(=dflt): shows "square" icon -> disconnect(terminate=true, suspend=false)
    • dropdown:
      • "stop" -> disconnect(terminate=true, suspend=false)
      • "disconnect" -> disconnect(terminate=false, suspend=false)
      • "disconnect and suspend" -> disconnect(terminate=false, suspend=true)
  • attach:
    • button(=dflt): shows "plug" icon -> disconnect(terminate=false, suspend=false)
    • dropdown:
      • "disconnect" -> disconnect(terminate=false, suspend=false)
      • "disconnect and suspend" -> disconnect(terminate=false, suspend=true)
      • "stop" -> disconnect(terminate=true, suspend=false)
  • Alt modifier key: flips the button's icon and semantics between "launch" and "attach"

Please note: I'm assuming that the dropdown always shows all available actions and that the first actions is the same as the default.

[roblourens]

Thanks @weinand, I wrote a similar table for myself yesterday 😅. I just pushed yesterday's change before I looked at the thread but it's mostly there now.

• What do you mean by (=dflt)?
• In the dropdown for "launch", you've listed "stop" -> disconnect(terminate=true), which is the same as the top-level stop button. I am not putting the same option in the dropdown, it might be confusing to show it twice. Same for "attach" and "disconnect"
• I assumed that for supportsTerminateDebuggee=false supportsSuspendDebuggee=true, for "launch", I would show "Disconnect and Suspend". But I guess you have to support the Terminate flag in order to get that option, because otherwise the disconnect request would always terminate, right?

[polinasok]

Another follow up about "Disconnect" and "Disconnect and Suspend" labels. Imagine a user that is paused on a breakpoint. What should they expect from "Disconnect"? Should they expect it to resume execution or leave things as is? I think it is important to highlight what state the user can expect execution to be in when they are done clicking. I think "Disconnect" is too ambitious. The options should be really be something like "Disconnect Resumed" and "Disconnect Suspended".

[roblourens]

"Suspend" is actually interesting because we don't use that term anywhere else, how did we pick "Suspend" and not "Pause" or "Stop"? Of course the DAP and UI terms diverge already and we can use something else in the UI if it makes sense.

[roblourens]

For me, I guess I assume that if I disconnect a debugger, the debuggee will resume by default. It seems enough that the opposite case ("Disconnect and Suspend") is spelled out, and I haven't heard of users being confused by that yet.

[weinand]

@roblourens answers for your questions:

What do you mean by (=dflt)?

I mean the "default" action provided by the top-level button part of the split button (as opposed to the dropdown part).

In the dropdown for "launch", you've listed "stop" -> disconnect(terminate=true), which is the same as the top-level stop button. I am not putting the same option in the dropdown, it might be confusing to show it twice. Same for "attach" and "disconnect"

Yes, that's ok.
In the table I just wanted to list all the available actions in the drop down (and ideally flag one as the "default")

I assumed that for supportsTerminateDebuggee=false supportsSuspendDebuggee=true, for "launch", I would show "Disconnect and Suspend". But I guess you have to support the Terminate flag in order to get that option, because otherwise the disconnect request would always terminate, right?

Yes, correct. For backward compatibility we should not assume that a DA supports "disconnectdisconnect(terminate=false)" for the "launch" case if supportsTerminateDebuggee is missing or false.

"Suspend" is actually interesting because we don't use that term anywhere else, how did we pick "Suspend" and not "Pause" or "Stop"?

Good observation: I agree we use "Pause" elsewhere but actually I prefer "Suspend" over "Pause". So if we want to unify terms I would rename "Pause" to "Suspend"... (but that's my personal preference).

@polinasok answers for your comment:

... Imagine a user that is paused on a breakpoint. What should they expect from "Disconnect"? Should they expect it to resume execution or leave things as is? ...

I fully agree with @roblourens:
I'm using debuggers since almost 40 years, and I have never seen a debugger where "disconnecting from a debuggee" means to "suspend execution". So I think we should not change VS Code's "Disconnect" terminology here: "Disconnect" means "resume execution".

[roblourens]

Pushed another change so that the behavior should align with the chart above.

[polinasok]

Just tried this with 1.67.0-insider:

With launch request and supportsTerminateDebuggee=false

This is working as expected.

With attach request and supportTerminateDebuggee=true

{"seq":0,"type":"event","event":"capabilities","body":{"capabilities":{"supportTerminateDebuggee":true}}}

This is unexpected. Based on the previous discussion, I thought the drop-down would show both Stop and Disconnect and so will the legacy alt hover. Also, oddly the keyboard shortcut is displayed as the same for both options.

With attach request and supportTerminateDebuggee=true and supportSuspendDebuggee=true

{"seq":0,"type":"event","event":"capabilities","body":{"capabilities":{"supportTerminateDebuggee":true,"supportSuspendDebuggee":true}}}

Same as attach above. I don't see a clickable option to suspend.

[roblourens]

Based on the previous discussion, I thought the drop-down would show both Stop and Disconnect and so will the legacy alt hover

Andre listed both actions in the dropdown but I said that I wouldn't duplicate the top-level button's action in the dropdown.

I don't see a clickable option to suspend.

Thanks for checking, I accidentally left Disconnect and Suspend out of the attach event.

[roblourens]

And the keyboard shortcut thing is just a limitation of how we look up keybindings - they do have the same shortcut, just in different contexts. Will see if I can clean it up.

[polinasok]

Thanks for the quick response, let me know when I can give this another try.

[roblourens]

The changes will be in Monday's insiders

[polinasok]

Tried with new insiders. I am glad there is a working solution. Thank you. Some additional usability thoughts below.

With launch request and supportsTerminateDebuggee=false

Same as before. Working as expected.

With attach request and supportTerminateDebuggee=true

The drop down is same as before minus erroneous F5.

The hover is back.

Having both may be redundant since they fully overlap. I personally find the drop down easier to use then messing with my keyboard, but I maybe there are users out there who feel differently and this gives them options.

I do think that lack of symmetry is unfortunate. It is not obvious that both controls are equivalent. You see two choices with hover, so it is clear what the icon represents and what the other option is. But that is not the case with the drop down.

With attach request and supportTerminateDebuggee=true and supportSuspendDebuggee=true

Hover is the same. Lacking the new option.

With this one I miss the drop-down label for Disconnect even more. Sorry. I know this was a conscious decision. But since the default is a name-less icon and the alternatives are words and off to the side, something feels off/confusing. Something like this would be more clear, in my opinion

[roblourens2]

Thanks for the feedback, you make good points

But since the default is a name-less icon and the alternatives are words and off to the side, something feels off/confusing. Something like this would be more clear, in my opinion

If I could have icons in the context menu, then I might like to duplicate the Disconnect action into the dropdown, like you suggest. I can't do that, and without that, I'm worried that it would make it more confusing, since the user might think there is a difference between the icon button and the Disconnect label command. Having the same icon would tie it together.

I think I will wait for user feedback before trying anything else, let's see whether people are confused.