Rob's comments and update variable view when done debugging #7527
Conversation
in a single block
-when the debugging manager is done debugging, debuggerVariables calls for a variable view refresh
| public readonly config: DebugConfiguration, | ||
| options?: DebugSessionOptions | ||
| ) { | ||
| this.session = new Promise<DebugSession>((resolve, reject) => { |
There was a problem hiding this comment.
You might try using a deferred object for this idea of remembering the promise resolve/reject.
If you create a deferred object, you can implement resolve/reject with it.
| const sf = stResponse.stackFrames[0]; | ||
| const mode = this.debuggingManager.getDebugMode(doc); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| let scopesResponse: any | undefined; |
| // Call variables | ||
| if (scopesResponse) { | ||
| // Keep track of variablesReference because "hover" requests also try to update variables | ||
| const newVariablesReference = scopesResponse.scopes[0].variablesReference; |
There was a problem hiding this comment.
Are you sure you only want to update the first scope?
| @@ -300,10 +267,6 @@ export class DebuggerVariables extends DebugLocationTracker | |||
|
|
|||
| this.updateVariables(undefined, message as DebugProtocol.VariablesResponse); | |||
There was a problem hiding this comment.
Can't this stuff be deleted, it seems like you copied the code to line 469
There was a problem hiding this comment.
I thought it was needed, but after testing, its not, I'll remove it.
src/client/debugger/types.ts
Outdated
| /** | ||
| * Called for every event sent from the debug adapter to the client. Returns true to signal that sending the message is vetoed. | ||
| */ | ||
| willSendEvent(msg: DebugProtocolMessage): Promise<boolean>; | ||
| onWillSendEvent(msg: DebugProtocolMessage): Promise<boolean>; |
There was a problem hiding this comment.
Usually I feel that events start with on..., this is a method and not an event. I don't know if your codebase has the same pattern, it's up to you
There was a problem hiding this comment.
true, that would be confusing, I'll put it back
-remove duplicated code
Codecov Report
@@ Coverage Diff @@
## main #7527 +/- ##
=====================================
- Coverage 66% 66% -1%
=====================================
Files 364 365 +1
Lines 22618 22638 +20
Branches 3446 3442 -4
=====================================
+ Hits 15072 15081 +9
- Misses 6248 6259 +11
Partials 1298 1298
|
| @@ -41,7 +41,7 @@ export class DebugCellController implements DebuggingDelegate { | |||
| } | |||
| } | |||
|
|
|||
| export class RunByLineController implements DebuggingDelegate { | |||
| export class RunByLineController implements IDebuggingDelegate { | |||
There was a problem hiding this comment.
Curious, do we need all of these interfaces? Why can't we just use classes
There was a problem hiding this comment.
We were following the extension patterns.
| } | ||
|
|
||
| async stop() { | ||
| void debug.stopDebugging(await this.session); |
There was a problem hiding this comment.
Why is this async if we're ignoring the promise returned by debug.stopDebuggin
There was a problem hiding this comment.
because we have to wait to get the session
For #7323 and Rob's comments here
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).