Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Supoort attach/detach remote headless server without kill process #2125

Merged
merged 6 commits into from
Jun 14, 2019

Conversation

BetaXOi
Copy link
Contributor

@BetaXOi BetaXOi commented Nov 14, 2018

Related: #1599 #1609
The reason is that sending halt request twice when close delve session.

This PR add support for "request": "attach" in launch.json. You can attach a running process from dlv --headless remote server, and detach without kill the running process.

I'm not a TypeScript beginner even, so the code modified maybe ugly, but it works. It's wonderful if anyone improve the code.

@msftclas
Copy link

msftclas commented Nov 14, 2018

CLA assistant check
All CLA requirements met.

@ramya-rao-a
Copy link
Contributor

Thanks for the PR @BetaXOi! This is a very much needed feature.
Unfortunately, I am little tight on time, so it will take a while for me to look and test the changes. But I'll surely get to this in the next 2 weeks

@jhendrixMSFT
Copy link
Member

Rebased on top of master.

src/debugAdapter/goDebug.ts Show resolved Hide resolved
src/debugAdapter/goDebug.ts Outdated Show resolved Hide resolved
src/debugAdapter/goDebug.ts Show resolved Hide resolved
src/debugAdapter/goDebug.ts Outdated Show resolved Hide resolved
src/debugAdapter/goDebug.ts Outdated Show resolved Hide resolved
src/debugAdapter/goDebug.ts Show resolved Hide resolved
src/goDebugConfiguration.ts Outdated Show resolved Hide resolved
src/debugAdapter/goDebug.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
debugConfiguration['useApiV1'] = dlvConfig['useApiV1'];
let useApiV1 = false;
if (debugConfiguration.hasOwnProperty('useApiV1')) {
useApiV1 = debugConfiguration['useApiV1'] === true;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This and the line below can also be useApiV1 = !!debugConfiguration['useApiV1'].

@quoctruong
Copy link
Contributor

@jhendrixMSFT @ramya-rao-a Any update on this PR?

@jhendrixMSFT
Copy link
Member

Sorry for the delay, been busy with Go SDK work. I've rebased on top of master.

@minkir014
Copy link

Why not merging this PR?? It's very perfect.

@jhendrixMSFT
Copy link
Member

@ramya-rao-a how do you want to proceed?

@minkir014
Copy link

What's the full goal of this PR? When will it be merged?

@jhendrixMSFT
Copy link
Member

Ramya and I discussed this earlier today, I'm going to make a few small changes. Should get merged later today/tomorrow. Sorry for the delay.

Added template for attaching to process.
Fixed template for connecting to a server.
Removed unnecessary fields from AttachRequestArguments
Cleaned up Delve.constructor to remove unnecessary parameters.
Removed duplicated code from attachRequest().
Cleaned up handling of legacy 'useApiV1' setting.
Ensure that property 'cwd' is set in debug configuration.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants