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.

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

Expose a stopDebugging API to extensions #101883

Closed
connor4312 opened this issue Jul 7, 2020 · 7 comments
Closed

Expose a stopDebugging API to extensions #101883

connor4312 opened this issue Jul 7, 2020 · 7 comments
Assignees
Labels
api api-finalization debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan
Milestone

Comments

@connor4312
Copy link
Member

We have vscode.debug.startDebugging to start a debug sessions, which is quite useful.

I wanted to write some tests for teardown behavior of js-debug, and when trying to do so I ran into the problem that there's no equivalent stopDebugging API. I could manually send some "terminate" request, but this doesn't cover the full scenario as effectively.

I'd like to propose the addition of a simple new API:

export function stopDebugging(session: DebugSession): Promise<void>

cc @weinand

@connor4312 connor4312 added debt Code quality issues under-discussion Issue is under discussion for relevance, priority, approach api-proposal labels Jul 7, 2020
@connor4312 connor4312 self-assigned this Jul 7, 2020
@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label Jul 7, 2020
@weinand weinand self-assigned this Jul 7, 2020
@weinand weinand added the feature-request Request for new features or functionality label Jul 7, 2020
@weinand weinand added this to the July 2020 milestone Jul 7, 2020
@weinand weinand removed debt Code quality issues under-discussion Issue is under discussion for relevance, priority, approach labels Jul 7, 2020
@weinand
Copy link
Contributor

weinand commented Jul 8, 2020

IMO there are two APIs possible:

A new terminate method on the DebugSession:

	export interface DebugSession {

		/**
		 * Terminates the session.
		 */
		terminate(): Thenable<void>;
	}

This approach is similar to TaskExecution.terminate() and the name "terminate" matches the already existing onDidTerminateDebugSession
.

Or a new function vscode.debug.stopDebugging:

	export namespace debug {

		/**
		 * Stop the given debug session or if no session is given stop all sessions.
		 */
		export function stopDebugging(session?: DebugSession): Thenable<void>;
	}

stopDebugging matches startDebugging and the optional session matches our internal semantics of IDebugService.stopSession.

@connor4312 @isidorn opinions?

@weinand weinand added the api label Jul 8, 2020
@connor4312
Copy link
Member Author

This approach is similar to TaskExecution.terminate() and the name "terminate" matches the already existing onDidTerminateDebugSession

One point of confusion here is that "stop debugging" for attach configurations will send disconnect rather than terminate. We could alternately rename it something like stop().

Personally I like the symmetry between vscode.debug.startDebugging and stopDebugging. But I don't feel strongly.

weinand added a commit that referenced this issue Jul 8, 2020
gjsjohnmurray pushed a commit to gjsjohnmurray/vscode that referenced this issue Jul 8, 2020
@weinand
Copy link
Contributor

weinand commented Jul 9, 2020

I've released both variants of the API to proposed.

@connor4312 I agree that "terminate" does not convey the semantics in a good way. And I like the symmetry between startDebugging and stopDebugging too.

So I will push vscode.debug.stopDebugging through the API process...

@weinand
Copy link
Contributor

weinand commented Jul 21, 2020

Proposal:

Similar to the already existing vscode.debug.startDebugging we introduce a new function vscode.debug.stopDebugging:

	export namespace debug {

		/**
		 * Stop the given debug session or if no session is given stop all sessions.
		 */
		export function stopDebugging(session?: DebugSession): Thenable<void>;
	}

stopDebugging matches startDebugging and the optional session matches our internal semantics of IDebugService.stopSession.

@isidorn
Copy link
Contributor

isidorn commented Jul 27, 2020

The proposal makes sense to me. Making it symetric to startDebugging feels like the way to go for me.

@weinand weinand modified the milestones: July 2020, August 2020 Aug 3, 2020
@weinand
Copy link
Contributor

weinand commented Aug 3, 2020

Testing of the proposed API will be done in July, finalization will take place in August.

@weinand
Copy link
Contributor

weinand commented Aug 11, 2020

After incorporating the feedback from the test pass the final proposed API has become this:

		/**
		 * Stop the given debug session or stop all debug sessions if session is omitted.
		 * @param session The [debug session](#DebugSession) to stop; if omitted all sessions are stopped.
		 */
		export function stopDebugging(session?: DebugSession): Thenable<void>;

@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-finalization debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan
Projects
None yet
Development

No branches or pull requests

4 participants
@weinand @isidorn @connor4312 and others