From 6945d1805cce83876a6da478e58dc04bc53c7f19 Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Tue, 18 Feb 2025 11:33:33 -0400 Subject: [PATCH 1/4] Cleaned up the scopes flows and added the ability to defer the loading of the local scope --- src/LaunchConfiguration.ts | 7 + src/adapters/DebugProtocolAdapter.ts | 1 - src/adapters/TelnetAdapter.ts | 5 +- src/debugSession/BrightScriptDebugSession.ts | 311 +++++++++++-------- 4 files changed, 193 insertions(+), 131 deletions(-) diff --git a/src/LaunchConfiguration.ts b/src/LaunchConfiguration.ts index 035457f4..5317057b 100644 --- a/src/LaunchConfiguration.ts +++ b/src/LaunchConfiguration.ts @@ -161,9 +161,16 @@ export interface LaunchConfiguration extends DebugProtocol.LaunchRequestArgument /** * Enables automatic population of the debug variable panel on a breakpoint or runtime errors. + * @deprecated Use `deferScopeLoading` instead */ enableVariablesPanel: boolean; + /** + * Will defer the population of the 'Local' scope variables until the user expands it in the variables panel. + * @default false + */ + deferScopeLoading: boolean; + /** * Enables automatic population of the virtual variables. */ diff --git a/src/adapters/DebugProtocolAdapter.ts b/src/adapters/DebugProtocolAdapter.ts index 9a744cdc..2d14d1d0 100644 --- a/src/adapters/DebugProtocolAdapter.ts +++ b/src/adapters/DebugProtocolAdapter.ts @@ -581,7 +581,6 @@ export class DebugProtocolAdapter { * @param expression the expression for the specified variable (i.e. `m`, `someVar.value`, `arr[1][2].three`). If empty string/undefined is specified, all local variables are retrieved instead */ private async getVariablesResponse(expression: string, frameId: number) { - const isScopesRequest = expression === ''; const logger = this.logger.createLogger('[getVariable]'); logger.info('begin', { expression }); if (!this.isAtDebuggerPrompt) { diff --git a/src/adapters/TelnetAdapter.ts b/src/adapters/TelnetAdapter.ts index d8e2be1b..c3c1189d 100644 --- a/src/adapters/TelnetAdapter.ts +++ b/src/adapters/TelnetAdapter.ts @@ -588,8 +588,9 @@ export class TelnetAdapter { /** * Given an expression, evaluate that statement ON the roku * @param expression + * @param frameId unused but added to match signature of DebugProtocolAdapter */ - public async getVariable(expression: string) { + public async getVariable(expression: string, frameId = -1) { const logger = this.logger.createLogger('[getVariable]'); logger.info('begin', { expression }); if (!this.isAtDebuggerPrompt) { @@ -1167,6 +1168,6 @@ interface BrightScriptRuntimeError { errorCode: string; } -export function isTelnetAdapterAdapter(adapter: TelnetAdapter | DebugProtocolAdapter): adapter is TelnetAdapter { +export function isTelnetAdapter(adapter: TelnetAdapter | DebugProtocolAdapter): adapter is TelnetAdapter { return adapter?.constructor.name === TelnetAdapter.name; } diff --git a/src/debugSession/BrightScriptDebugSession.ts b/src/debugSession/BrightScriptDebugSession.ts index 49215c16..f068cf7a 100644 --- a/src/debugSession/BrightScriptDebugSession.ts +++ b/src/debugSession/BrightScriptDebugSession.ts @@ -6,7 +6,6 @@ import type { DeviceInfo, RokuDeploy, RokuDeployOptions } from 'roku-deploy'; import { BreakpointEvent, DebugSession as BaseDebugSession, - Handles, InitializedEvent, InvalidatedEvent, OutputEvent, @@ -26,7 +25,7 @@ import { ComponentLibraryServer } from '../ComponentLibraryServer'; import { ProjectManager, Project, ComponentLibraryProject } from '../managers/ProjectManager'; import type { EvaluateContainer } from '../adapters/DebugProtocolAdapter'; import { isDebugProtocolAdapter, DebugProtocolAdapter } from '../adapters/DebugProtocolAdapter'; -import { TelnetAdapter } from '../adapters/TelnetAdapter'; +import { isTelnetAdapter, TelnetAdapter } from '../adapters/TelnetAdapter'; import type { BSDebugDiagnostic } from '../CompileErrorProcessor'; import { RendezvousTracker } from '../RendezvousTracker'; import { @@ -141,8 +140,6 @@ export class BrightScriptDebugSession extends BaseDebugSession { private variables: Record = {}; - private variableHandles = new Handles(); - private rokuAdapter: DebugProtocolAdapter | TelnetAdapter; private rendezvousTracker: RendezvousTracker; @@ -362,6 +359,12 @@ export class BrightScriptDebugSession extends BaseDebugSession { config.rewriteDevicePathsInLogs ??= true; config.autoResolveVirtualVariables ??= false; config.enhanceREPLCompletions ??= true; + + // migrate the old `enableVariablesPanel` setting to the new `deferScopeLoading` setting + if (typeof config.enableVariablesPanel !== 'boolean') { + config.enableVariablesPanel = true; + } + config.deferScopeLoading ??= config.enableVariablesPanel === false; return config; } @@ -1195,58 +1198,52 @@ export class BrightScriptDebugSession extends BaseDebugSession { } } - protected async scopesRequest(response: DebugProtocol.ScopesResponse, args: DebugProtocol.ScopesArguments) { + protected scopesRequest(response: DebugProtocol.ScopesResponse, args: DebugProtocol.ScopesArguments) { const logger = this.logger.createLogger(`scopesRequest ${this.idCounter}`); logger.info('begin', { args }); try { const scopes = new Array(); + let v: AugmentedVariable; - if (isDebugProtocolAdapter(this.rokuAdapter)) { - let refId = this.getEvaluateRefId('', args.frameId); - let v: AugmentedVariable; - //if we already looked this item up, return it - if (this.variables[refId]) { - v = this.variables[refId]; - } else { - let result = await this.rokuAdapter.getLocalVariables(args.frameId); - if (!result) { - throw new Error(`Could not get scopes`); - } - v = await this.getVariableFromResult(result, args.frameId); - //TODO - testing something, remove later - // eslint-disable-next-line camelcase - v.request_seq = response.request_seq; - v.frameId = args.frameId; - } - - scopes.push({ - name: 'Local', - variablesReference: v.variablesReference, - expensive: false, - presentationHint: 'locals' - }); + // create the locals scope + let localsRefId = this.getEvaluateRefId('', args.frameId); + if (this.variables[localsRefId]) { + v = this.variables[localsRefId]; } else { - // NOTE: Legacy telnet support - scopes.push({ - name: 'Local', - variablesReference: this.variableHandles.create('local'), - expensive: false, - presentationHint: 'locals' - }); + v = { + variablesReference: localsRefId, + name: 'Locals', + value: '', + type: '$$Locals', + frameId: args.frameId, + isScope: true, + childVariables: [] + }; + this.variables[localsRefId] = v; } - let refId = this.getEvaluateRefId('$$registry', Infinity); + scopes.push({ + name: 'Local', + variablesReference: v.variablesReference, + // Flag the locals scope as expensive if the client asked that it be loaded lazily + expensive: this.launchConfiguration.deferScopeLoading, + presentationHint: 'locals' + }); + + // create the registry scope + let registryRefId = this.getEvaluateRefId('$$registry', Infinity); scopes.push({ name: 'Registry', - variablesReference: refId, + variablesReference: registryRefId, expensive: true }); - this.variables[refId] = { - variablesReference: refId, + this.variables[registryRefId] = { + variablesReference: registryRefId, name: 'Registry', value: '', type: '$$Registry', + isScope: true, childVariables: [] }; @@ -1387,100 +1384,80 @@ export class BrightScriptDebugSession extends BaseDebugSession { response.message = 'Debug session is not paused'; return this.sendResponse(response); } - const reference = this.variableHandles.get(args.variablesReference); - if (reference) { - logger.log('reference', reference); - // NOTE: Legacy telnet support for local vars - if (this.launchConfiguration.enableVariablesPanel) { - const vars = await (this.rokuAdapter as TelnetAdapter).getScopeVariables(); - - for (const varName of vars) { - let { evalArgs } = await this.evaluateExpressionToTempVar({ expression: varName, frameId: -1 }, util.getVariablePath(varName)); - let result = await this.rokuAdapter.getVariable(evalArgs.expression, -1); - let tempVar = await this.getVariableFromResult(result, -1); - updatedVariables.push(tempVar); - } - } else { - updatedVariables.push(new Variable('variables disabled by launch.json setting', 'enableVariablesPanel: false')); - } - this.variables[args.variablesReference] = new Variable('', '', args.variablesReference, 0, updatedVariables.length); - this.variables[args.variablesReference].childVariables = updatedVariables; - } else { - //find the variable with this reference - let v = this.variables[args.variablesReference]; - if (!v) { - response.success = false; - response.message = `Variable reference has expired`; - return this.sendResponse(response); - } - logger.log('variable', v); - if (v.type === '$$Registry' && v.childVariables.length === 0) { - // This is a special scope variable used to load registry data via an ECP call - // Send the registry ECP call for the `dev` app as side loaded apps are always `dev` - await populateVariableFromRegistryEcp({ host: this.launchConfiguration.host, remotePort: this.launchConfiguration.remotePort, appId: 'dev' }, v, this.variables, this.getEvaluateRefId.bind(this)); - } - - //query for child vars if we haven't done it yet or DAP is asking to resolve a lazy variable - if (v.childVariables.length === 0 || v.isResolved) { - let tempVar: AugmentedVariable; - if (!v.isResolved) { - // Evaluate the variable - try { - let { evalArgs } = await this.evaluateExpressionToTempVar({ expression: v.evaluateName, frameId: v.frameId }, util.getVariablePath(v.evaluateName)); - let result = await this.rokuAdapter.getVariable(evalArgs.expression, v.frameId); - tempVar = await this.getVariableFromResult(result, v.frameId); - tempVar.frameId = v.frameId; - // Determine if the variable has changed - sendInvalidatedEvent = v.type !== tempVar.type || v.indexedVariables !== tempVar.indexedVariables; - } catch (error) { - logger.error('Error getting variables', error); - tempVar = new Variable('Error', `❌ Error: ${error.message}`); - tempVar.type = ''; - tempVar.childVariables = []; - sendInvalidatedEvent = true; - response.success = false; - response.message = error.message; - } + //find the variable with this reference + let v = this.variables[args.variablesReference]; + if (!v) { + response.success = false; + response.message = `Variable reference has expired`; + return this.sendResponse(response); + } + logger.log('variable', v); - // Merge the resulting updates together - v.childVariables = tempVar.childVariables; - v.value = tempVar.value; - v.type = tempVar.type; - v.indexedVariables = tempVar.indexedVariables; - v.namedVariables = tempVar.namedVariables; - } - frameId = v.frameId; + // Populate scope level values if needed + if (v.isScope) { + await this.populateScopeVariables(v, args); + } - if (v?.presentationHint?.lazy || v.isResolved) { - // If this was a lazy variable we need to respond with the updated variable and not the children - if (v.isResolved && v.childVariables.length > 0) { - updatedVariables = v.childVariables; - } else { - updatedVariables = [v]; - } - v.isResolved = true; - } else { - updatedVariables = v.childVariables; + //query for child vars if we haven't done it yet or DAP is asking to resolve a lazy variable + if (v.childVariables.length === 0 || v.isResolved) { + let tempVar: AugmentedVariable; + if (!v.isResolved) { + // Evaluate the variable + try { + let { evalArgs } = await this.evaluateExpressionToTempVar({ expression: v.evaluateName, frameId: v.frameId }, util.getVariablePath(v.evaluateName)); + let result = await this.rokuAdapter.getVariable(evalArgs.expression, v.frameId); + tempVar = await this.getVariableFromResult(result, v.frameId); + tempVar.frameId = v.frameId; + // Determine if the variable has changed + sendInvalidatedEvent = v.type !== tempVar.type || v.indexedVariables !== tempVar.indexedVariables; + } catch (error) { + logger.error('Error getting variables', error); + tempVar = new Variable('Error', `❌ Error: ${error.message}`); + tempVar.type = ''; + tempVar.childVariables = []; + sendInvalidatedEvent = true; + response.success = false; + response.message = error.message; } - // If the variable has no children, set the reference to 0 - // so it does not look expandable in the Ui - if (v.childVariables.length === 0) { - v.variablesReference = 0; - } + // Merge the resulting updates together + v.childVariables = tempVar.childVariables; + v.value = tempVar.value; + v.type = tempVar.type; + v.indexedVariables = tempVar.indexedVariables; + v.namedVariables = tempVar.namedVariables; + } + frameId = v.frameId; - // If the variable was resolve in the past we may not have fetched a new temp var - tempVar ??= v; - if (v?.presentationHint) { - v.presentationHint.lazy = tempVar.presentationHint?.lazy; + if (v?.presentationHint?.lazy || v.isResolved) { + // If this was a lazy variable we need to respond with the updated variable and not the children + if (v.isResolved && v.childVariables.length > 0) { + updatedVariables = v.childVariables; } else { - v.presentationHint = tempVar.presentationHint; + updatedVariables = [v]; } - + v.isResolved = true; } else { updatedVariables = v.childVariables; } + + // If the variable has no children, set the reference to 0 + // so it does not look expandable in the Ui + if (v.childVariables.length === 0) { + v.variablesReference = 0; + } + + // If the variable was resolve in the past we may not have fetched a new temp var + tempVar ??= v; + if (v?.presentationHint) { + v.presentationHint.lazy = tempVar.presentationHint?.lazy; + } else { + v.presentationHint = tempVar.presentationHint; + } + + } else { + updatedVariables = v.childVariables; } // Only send the updated variables if we are not going to trigger an invalidated event. @@ -1547,6 +1524,79 @@ export class BrightScriptDebugSession extends BaseDebugSession { return filteredUpdatedVariables; } + /** + * Takes a scope variable and populates its child variables based on the scope type and the current adapter type. + * @param v scope variable to populate + * @param args + */ + private async populateScopeVariables(v: AugmentedVariable, args: DebugProtocol.VariablesArguments) { + if (v.childVariables.length === 0) { + let tempVar: AugmentedVariable; + try { + if (v.type === '$$Locals') { + if (isDebugProtocolAdapter(this.rokuAdapter)) { + let result = await this.rokuAdapter.getLocalVariables(v.frameId); + tempVar = await this.getVariableFromResult(result, v.frameId); + } else if (isTelnetAdapter(this.rokuAdapter)) { + // NOTE: Legacy telnet support + let variables: AugmentedVariable[] = []; + const varNames = await this.rokuAdapter.getScopeVariables(); + + // Fetch each variable individually + for (const varName of varNames) { + let { evalArgs } = await this.evaluateExpressionToTempVar({ expression: varName, frameId: -1 }, util.getVariablePath(varName)); + let result = await this.rokuAdapter.getVariable(evalArgs.expression, -1); + let tempLocalsVar = await this.getVariableFromResult(result, -1); + variables.push(tempLocalsVar); + } + tempVar = { + ...v, + childVariables: variables, + namedVariables: variables.length, + indexedVariables: 0 + }; + } + + // Merge the resulting updates together onto the original variable + v.childVariables = tempVar.childVariables; + v.namedVariables = tempVar.namedVariables; + v.indexedVariables = tempVar.indexedVariables; + } else if (v.type === '$$Registry') { + // This is a special scope variable used to load registry data via an ECP call + // Send the registry ECP call for the `dev` app as side loaded apps are always `dev` + await populateVariableFromRegistryEcp({ host: this.launchConfiguration.host, remotePort: this.launchConfiguration.remotePort, appId: 'dev' }, v, this.variables, this.getEvaluateRefId.bind(this)); + } + } catch (error) { + logger.error(`Error getting variables for scope ${v.type}`, error); + tempVar = { + name: '', + value: `❌ Error: ${error.message}`, + variablesReference: 0, + childVariables: [] + }; + v.childVariables = [tempVar]; + v.namedVariables = 1; + v.indexedVariables = 0; + } + + // Mark the scope as resolved so we don't re-fetch the variables + v.isResolved = true; + + // If the scope has no children, add a single child to indicate there are no values + if (v.childVariables.length === 0) { + tempVar = { + name: '', + value: `No values for scope '${v.name}'`, + variablesReference: 0, + childVariables: [] + }; + v.childVariables = [tempVar]; + v.namedVariables = 1; + v.indexedVariables = 0; + } + } + } + private evaluateRequestPromise = Promise.resolve(); private evaluateVarIndexByFrameId = new Map(); @@ -1564,12 +1614,12 @@ export class BrightScriptDebugSession extends BaseDebugSession { await this.getRokuAdapter(); let deferred = defer(); - if (args.context === 'repl' && !this.enableDebugProtocol && args.expression.trim().startsWith('>')) { + if (args.context === 'repl' && isTelnetAdapter(this.rokuAdapter) && args.expression.trim().startsWith('>')) { this.clearState(); this.rokuAdapter.clearCache(); const expression = args.expression.replace(/^\s*>\s*/, ''); this.logger.log('Sending raw telnet command...I sure hope you know what you\'re doing', { expression }); - (this.rokuAdapter as TelnetAdapter).requestPipeline.client.write(`${expression}\r\n`); + this.rokuAdapter.requestPipeline.client.write(`${expression}\r\n`); this.sendResponse(response); return deferred.promise; } @@ -2204,7 +2254,7 @@ export class BrightScriptDebugSession extends BaseDebugSession { let v: AugmentedVariable; if (result) { - if (this.enableDebugProtocol) { + if (isDebugProtocolAdapter(this.rokuAdapter)) { let refId = this.getEvaluateRefId(result.evaluateName, frameId); if (result.isCustom && !result.presentationHint?.lazy && result.evaluateNow) { try { @@ -2262,7 +2312,7 @@ export class BrightScriptDebugSession extends BaseDebugSession { v = new Variable(result.name, value, result?.presentationHint?.lazy ? refId : 0); } this.variables[refId] = v; - } else { + } else if (isTelnetAdapter(this.rokuAdapter)) { if (result.highLevelType === 'primative' || result.highLevelType === 'uninitialized') { v = new Variable(result.name, `${result.value}`); } else if (result.highLevelType === 'array') { @@ -2272,7 +2322,7 @@ export class BrightScriptDebugSession extends BaseDebugSession { } else if (result.highLevelType === 'object') { let refId: number; //handle collections - if ((this.rokuAdapter as TelnetAdapter).isScrapableContainObject(result.type)) { + if (this.rokuAdapter.isScrapableContainObject(result.type)) { refId = this.getEvaluateRefId(result.evaluateName, frameId); } v = new Variable(result.name, result.type, refId, 0, result.children?.length ?? 0); @@ -2506,4 +2556,9 @@ export interface AugmentedVariable extends DebugProtocol.Variable { * only used for lazy variables */ isResolved?: boolean; + /** + * used to indicate that this variable is a scope variable + * and may require special handling + */ + isScope?: boolean; } From b8371304c7f1b7820d0360a1a227dcb0a8483ccc Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Tue, 18 Feb 2025 13:00:46 -0400 Subject: [PATCH 2/4] Fixed some tests --- src/adapters/DebugProtocolAdapter.ts | 12 +++++--- src/adapters/TelnetAdapter.ts | 12 +++++--- .../BrightScriptDebugSession.spec.ts | 30 +++++++++++++++++-- src/debugSession/BrightScriptDebugSession.ts | 14 ++++----- 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/adapters/DebugProtocolAdapter.ts b/src/adapters/DebugProtocolAdapter.ts index 2d14d1d0..795fa7b2 100644 --- a/src/adapters/DebugProtocolAdapter.ts +++ b/src/adapters/DebugProtocolAdapter.ts @@ -1011,6 +1011,14 @@ export class DebugProtocolAdapter { } } } + + public isTelnetAdapter(): this is TelnetAdapter { + return false; + } + + public isDebugProtocolAdapter(): this is DebugProtocolAdapter { + return true; + } } export interface StackFrame { @@ -1063,7 +1071,3 @@ interface BrightScriptRuntimeError { message: string; errorCode: string; } - -export function isDebugProtocolAdapter(adapter: TelnetAdapter | DebugProtocolAdapter): adapter is DebugProtocolAdapter { - return adapter?.constructor.name === DebugProtocolAdapter.name; -} diff --git a/src/adapters/TelnetAdapter.ts b/src/adapters/TelnetAdapter.ts index c3c1189d..8f1d2ea5 100644 --- a/src/adapters/TelnetAdapter.ts +++ b/src/adapters/TelnetAdapter.ts @@ -1113,6 +1113,14 @@ export class TelnetAdapter { public async syncBreakpoints() { //we can't send dynamic breakpoints to the server...so just do nothing } + + public isTelnetAdapter(): this is TelnetAdapter { + return true; + } + + public isDebugProtocolAdapter(): this is DebugProtocolAdapter { + return true; + } } export interface StackFrame { @@ -1167,7 +1175,3 @@ interface BrightScriptRuntimeError { message: string; errorCode: string; } - -export function isTelnetAdapter(adapter: TelnetAdapter | DebugProtocolAdapter): adapter is TelnetAdapter { - return adapter?.constructor.name === TelnetAdapter.name; -} diff --git a/src/debugSession/BrightScriptDebugSession.spec.ts b/src/debugSession/BrightScriptDebugSession.spec.ts index 4ddf9130..845d681b 100644 --- a/src/debugSession/BrightScriptDebugSession.spec.ts +++ b/src/debugSession/BrightScriptDebugSession.spec.ts @@ -22,6 +22,7 @@ import { RendezvousTracker } from '../RendezvousTracker'; import { ClientToServerCustomEventName, isCustomRequestEvent, LogOutputEvent } from './Events'; import { EventEmitter } from 'eventemitter3'; import type { EvaluateContainer } from '../adapters/DebugProtocolAdapter'; +import { VariableType } from '../debugProtocol/events/responses/VariablesResponse'; const sinon = sinonActual.createSandbox(); const tempDir = s`${__dirname}/../../.tmp`; @@ -117,6 +118,8 @@ describe('BrightScriptDebugSession', () => { getScopeVariables: (a) => { }, setExceptionBreakpoints: (a) => { }, isScrapableContainObject: () => { }, + isTelnetAdapter: () => false, + isDebugProtocolAdapter: () => true, getThreads: () => { return []; }, @@ -334,6 +337,21 @@ describe('BrightScriptDebugSession', () => { sinon.stub(rokuAdapter, 'getScopeVariables').callsFake(() => { return Promise.resolve(['m', 'top', `${session.tempVarPrefix}eval`]); }); + sinon.stub(session as any, 'populateScopeVariables').callsFake((v: AugmentedVariable, args: DebugProtocol.VariablesArguments) => { + v.childVariables = [{ + name: `${session.tempVarPrefix}eval`, + value: 'true', + variablesReference: 0 + }, { + name: 'top', + value: 'roSGNode:GetSubReddit', + variablesReference: 3 + }, { + name: 'm', + value: VariableType.AssociativeArray, + variablesReference: 0 + }]; + }); sinon.stub(rokuAdapter, 'getVariable').callsFake(x => { return Promise.resolve( { @@ -359,7 +377,7 @@ describe('BrightScriptDebugSession', () => { session['dispatchRequest']({ command: 'scopes', arguments: { frameId: 0 }, type: 'request', seq: 8 }); await session.variablesRequest( response, - { variablesReference: 1000, filter: 'named', start: 0, count: 0, format: '' } as DebugProtocol.VariablesArguments + { variablesReference: 1, filter: 'named', start: 0, count: 0, format: '' } as DebugProtocol.VariablesArguments ); expect( @@ -369,7 +387,7 @@ describe('BrightScriptDebugSession', () => { session['launchConfiguration'].showHiddenVariables = true; await session.variablesRequest( response, - { variablesReference: 1000, filter: 'named', start: 0, count: 0, format: '' } as DebugProtocol.VariablesArguments + { variablesReference: 1, filter: 'named', start: 0, count: 0, format: '' } as DebugProtocol.VariablesArguments ); expect( response.body.variables.find(x => x.name.startsWith(session.tempVarPrefix)) @@ -654,6 +672,8 @@ describe('BrightScriptDebugSession', () => { it('returns the correct boolean variable', async () => { session['rokuAdapterDeferred'].resolve(session['rokuAdapter']); + sinon.stub(session['rokuAdapter'], 'isTelnetAdapter').callsFake(() => true); + sinon.stub(session['rokuAdapter'], 'isDebugProtocolAdapter').callsFake(() => false); let expression = 'someBool'; getVariableValue = getBooleanEvaluateContainer(expression); @@ -673,6 +693,8 @@ describe('BrightScriptDebugSession', () => { //this fails on TravisCI for some reason. TODO - fix this it('returns the correct indexed variables count', async () => { session['rokuAdapterDeferred'].resolve(session['rokuAdapter']); + sinon.stub(session['rokuAdapter'], 'isTelnetAdapter').callsFake(() => true); + sinon.stub(session['rokuAdapter'], 'isDebugProtocolAdapter').callsFake(() => false); let expression = 'someArray'; getVariableValue = { @@ -699,6 +721,8 @@ describe('BrightScriptDebugSession', () => { it('returns the correct named variables count', async () => { session['rokuAdapterDeferred'].resolve(session['rokuAdapter']); + sinon.stub(session['rokuAdapter'], 'isTelnetAdapter').callsFake(() => true); + sinon.stub(session['rokuAdapter'], 'isDebugProtocolAdapter').callsFake(() => false); let expression = 'someObject'; getVariableValue = { @@ -1020,6 +1044,8 @@ describe('BrightScriptDebugSession', () => { beforeEach(() => { session['rokuAdapterDeferred'].resolve(session['rokuAdapter']); + sinon.stub(session['rokuAdapter'], 'isTelnetAdapter').callsFake(() => true); + sinon.stub(session['rokuAdapter'], 'isDebugProtocolAdapter').callsFake(() => false); rokuAdapter.isAtDebuggerPrompt = true; evalStub = sinon.stub(rokuAdapter, 'evaluate').callsFake((args) => { diff --git a/src/debugSession/BrightScriptDebugSession.ts b/src/debugSession/BrightScriptDebugSession.ts index f068cf7a..9957fc01 100644 --- a/src/debugSession/BrightScriptDebugSession.ts +++ b/src/debugSession/BrightScriptDebugSession.ts @@ -24,8 +24,8 @@ import { fileUtils, standardizePath as s } from '../FileUtils'; import { ComponentLibraryServer } from '../ComponentLibraryServer'; import { ProjectManager, Project, ComponentLibraryProject } from '../managers/ProjectManager'; import type { EvaluateContainer } from '../adapters/DebugProtocolAdapter'; -import { isDebugProtocolAdapter, DebugProtocolAdapter } from '../adapters/DebugProtocolAdapter'; -import { isTelnetAdapter, TelnetAdapter } from '../adapters/TelnetAdapter'; +import { DebugProtocolAdapter } from '../adapters/DebugProtocolAdapter'; +import { TelnetAdapter } from '../adapters/TelnetAdapter'; import type { BSDebugDiagnostic } from '../CompileErrorProcessor'; import { RendezvousTracker } from '../RendezvousTracker'; import { @@ -1534,10 +1534,10 @@ export class BrightScriptDebugSession extends BaseDebugSession { let tempVar: AugmentedVariable; try { if (v.type === '$$Locals') { - if (isDebugProtocolAdapter(this.rokuAdapter)) { + if (this.rokuAdapter.isDebugProtocolAdapter()) { let result = await this.rokuAdapter.getLocalVariables(v.frameId); tempVar = await this.getVariableFromResult(result, v.frameId); - } else if (isTelnetAdapter(this.rokuAdapter)) { + } else if (this.rokuAdapter.isTelnetAdapter()) { // NOTE: Legacy telnet support let variables: AugmentedVariable[] = []; const varNames = await this.rokuAdapter.getScopeVariables(); @@ -1614,7 +1614,7 @@ export class BrightScriptDebugSession extends BaseDebugSession { await this.getRokuAdapter(); let deferred = defer(); - if (args.context === 'repl' && isTelnetAdapter(this.rokuAdapter) && args.expression.trim().startsWith('>')) { + if (args.context === 'repl' && this.rokuAdapter.isTelnetAdapter() && args.expression.trim().startsWith('>')) { this.clearState(); this.rokuAdapter.clearCache(); const expression = args.expression.replace(/^\s*>\s*/, ''); @@ -2254,7 +2254,7 @@ export class BrightScriptDebugSession extends BaseDebugSession { let v: AugmentedVariable; if (result) { - if (isDebugProtocolAdapter(this.rokuAdapter)) { + if (this.rokuAdapter.isDebugProtocolAdapter()) { let refId = this.getEvaluateRefId(result.evaluateName, frameId); if (result.isCustom && !result.presentationHint?.lazy && result.evaluateNow) { try { @@ -2312,7 +2312,7 @@ export class BrightScriptDebugSession extends BaseDebugSession { v = new Variable(result.name, value, result?.presentationHint?.lazy ? refId : 0); } this.variables[refId] = v; - } else if (isTelnetAdapter(this.rokuAdapter)) { + } else if (this.rokuAdapter.isTelnetAdapter()) { if (result.highLevelType === 'primative' || result.highLevelType === 'uninitialized') { v = new Variable(result.name, `${result.value}`); } else if (result.highLevelType === 'array') { From e19d0cffd10297fcef21638e14284c1e41ad6bf6 Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Tue, 18 Feb 2025 13:09:57 -0400 Subject: [PATCH 3/4] Update src/debugSession/BrightScriptDebugSession.ts --- src/debugSession/BrightScriptDebugSession.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/debugSession/BrightScriptDebugSession.ts b/src/debugSession/BrightScriptDebugSession.ts index 9957fc01..61ea7707 100644 --- a/src/debugSession/BrightScriptDebugSession.ts +++ b/src/debugSession/BrightScriptDebugSession.ts @@ -1206,7 +1206,7 @@ export class BrightScriptDebugSession extends BaseDebugSession { let v: AugmentedVariable; // create the locals scope - let localsRefId = this.getEvaluateRefId('', args.frameId); + let localsRefId = this.getEvaluateRefId('$$locals', args.frameId); if (this.variables[localsRefId]) { v = this.variables[localsRefId]; } else { From e82c76cfe884c97f85bd34247de4f88c9ba7f1db Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Tue, 18 Feb 2025 13:13:19 -0400 Subject: [PATCH 4/4] pr feedback to exit earily in populateScopeVariables --- src/debugSession/BrightScriptDebugSession.ts | 121 ++++++++++--------- 1 file changed, 62 insertions(+), 59 deletions(-) diff --git a/src/debugSession/BrightScriptDebugSession.ts b/src/debugSession/BrightScriptDebugSession.ts index 61ea7707..9851dfb3 100644 --- a/src/debugSession/BrightScriptDebugSession.ts +++ b/src/debugSession/BrightScriptDebugSession.ts @@ -1530,70 +1530,73 @@ export class BrightScriptDebugSession extends BaseDebugSession { * @param args */ private async populateScopeVariables(v: AugmentedVariable, args: DebugProtocol.VariablesArguments) { - if (v.childVariables.length === 0) { - let tempVar: AugmentedVariable; - try { - if (v.type === '$$Locals') { - if (this.rokuAdapter.isDebugProtocolAdapter()) { - let result = await this.rokuAdapter.getLocalVariables(v.frameId); - tempVar = await this.getVariableFromResult(result, v.frameId); - } else if (this.rokuAdapter.isTelnetAdapter()) { - // NOTE: Legacy telnet support - let variables: AugmentedVariable[] = []; - const varNames = await this.rokuAdapter.getScopeVariables(); - - // Fetch each variable individually - for (const varName of varNames) { - let { evalArgs } = await this.evaluateExpressionToTempVar({ expression: varName, frameId: -1 }, util.getVariablePath(varName)); - let result = await this.rokuAdapter.getVariable(evalArgs.expression, -1); - let tempLocalsVar = await this.getVariableFromResult(result, -1); - variables.push(tempLocalsVar); - } - tempVar = { - ...v, - childVariables: variables, - namedVariables: variables.length, - indexedVariables: 0 - }; - } + if (v.childVariables.length > 0) { + // Already populated + return; + } - // Merge the resulting updates together onto the original variable - v.childVariables = tempVar.childVariables; - v.namedVariables = tempVar.namedVariables; - v.indexedVariables = tempVar.indexedVariables; - } else if (v.type === '$$Registry') { - // This is a special scope variable used to load registry data via an ECP call - // Send the registry ECP call for the `dev` app as side loaded apps are always `dev` - await populateVariableFromRegistryEcp({ host: this.launchConfiguration.host, remotePort: this.launchConfiguration.remotePort, appId: 'dev' }, v, this.variables, this.getEvaluateRefId.bind(this)); + let tempVar: AugmentedVariable; + try { + if (v.type === '$$Locals') { + if (this.rokuAdapter.isDebugProtocolAdapter()) { + let result = await this.rokuAdapter.getLocalVariables(v.frameId); + tempVar = await this.getVariableFromResult(result, v.frameId); + } else if (this.rokuAdapter.isTelnetAdapter()) { + // NOTE: Legacy telnet support + let variables: AugmentedVariable[] = []; + const varNames = await this.rokuAdapter.getScopeVariables(); + + // Fetch each variable individually + for (const varName of varNames) { + let { evalArgs } = await this.evaluateExpressionToTempVar({ expression: varName, frameId: -1 }, util.getVariablePath(varName)); + let result = await this.rokuAdapter.getVariable(evalArgs.expression, -1); + let tempLocalsVar = await this.getVariableFromResult(result, -1); + variables.push(tempLocalsVar); + } + tempVar = { + ...v, + childVariables: variables, + namedVariables: variables.length, + indexedVariables: 0 + }; } - } catch (error) { - logger.error(`Error getting variables for scope ${v.type}`, error); - tempVar = { - name: '', - value: `❌ Error: ${error.message}`, - variablesReference: 0, - childVariables: [] - }; - v.childVariables = [tempVar]; - v.namedVariables = 1; - v.indexedVariables = 0; + + // Merge the resulting updates together onto the original variable + v.childVariables = tempVar.childVariables; + v.namedVariables = tempVar.namedVariables; + v.indexedVariables = tempVar.indexedVariables; + } else if (v.type === '$$Registry') { + // This is a special scope variable used to load registry data via an ECP call + // Send the registry ECP call for the `dev` app as side loaded apps are always `dev` + await populateVariableFromRegistryEcp({ host: this.launchConfiguration.host, remotePort: this.launchConfiguration.remotePort, appId: 'dev' }, v, this.variables, this.getEvaluateRefId.bind(this)); } + } catch (error) { + logger.error(`Error getting variables for scope ${v.type}`, error); + tempVar = { + name: '', + value: `❌ Error: ${error.message}`, + variablesReference: 0, + childVariables: [] + }; + v.childVariables = [tempVar]; + v.namedVariables = 1; + v.indexedVariables = 0; + } - // Mark the scope as resolved so we don't re-fetch the variables - v.isResolved = true; + // Mark the scope as resolved so we don't re-fetch the variables + v.isResolved = true; - // If the scope has no children, add a single child to indicate there are no values - if (v.childVariables.length === 0) { - tempVar = { - name: '', - value: `No values for scope '${v.name}'`, - variablesReference: 0, - childVariables: [] - }; - v.childVariables = [tempVar]; - v.namedVariables = 1; - v.indexedVariables = 0; - } + // If the scope has no children, add a single child to indicate there are no values + if (v.childVariables.length === 0) { + tempVar = { + name: '', + value: `No values for scope '${v.name}'`, + variablesReference: 0, + childVariables: [] + }; + v.childVariables = [tempVar]; + v.namedVariables = 1; + v.indexedVariables = 0; } }