From 79385fa56f392fd25207ac11171febd9444199b5 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Fri, 27 Oct 2023 13:00:27 -0400 Subject: [PATCH 1/7] Fix and enhance the device picker --- package-lock.json | 45 ++++- package.json | 1 + src/ActiveDeviceManager.ts | 88 ++++++++-- src/DebugConfigurationProvider.spec.ts | 3 +- src/DebugConfigurationProvider.ts | 161 +++++++++++++----- src/util.spec.ts | 12 ++ src/util.ts | 31 ++++ .../OnlineDevicesViewProvider.ts | 39 +---- 8 files changed, 280 insertions(+), 100 deletions(-) diff --git a/package-lock.json b/package-lock.json index b8f0072a..d8165d47 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,6 +17,7 @@ "brighterscript-formatter": "^1.6.34", "debounce": "^1.2.0", "dotenv": "^6.2.0", + "eventemitter3": "^5.0.1", "fast-xml-parser": "^3.12.16", "fs-extra": "^7.0.1", "get-port": "^5.0.0", @@ -2570,6 +2571,11 @@ "node": ">=0.8.0" } }, + "node_modules/brighterscript/node_modules/eventemitter3": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", + "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==" + }, "node_modules/brighterscript/node_modules/fs-extra": { "version": "8.1.0", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-8.1.0.tgz", @@ -4511,9 +4517,9 @@ } }, "node_modules/eventemitter3": { - "version": "4.0.7", - "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", - "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==" + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-5.0.1.tgz", + "integrity": "sha512-GWkBvjiSZK87ELrYOSESUYeVIc9mvLLf/nXalMOS5dYrgZq9o5OVkbZAVM06CVxYsCwH9BDZFPlQTlPA1j4ahA==" }, "node_modules/expand-template": { "version": "2.0.3", @@ -5772,6 +5778,11 @@ "node": ">=8.0.0" } }, + "node_modules/http-proxy/node_modules/eventemitter3": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", + "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==" + }, "node_modules/iconv-lite": { "version": "0.4.24", "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.24.tgz", @@ -8865,6 +8876,11 @@ "xml2js": "^0.5.0" } }, + "node_modules/roku-debug/node_modules/eventemitter3": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", + "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==" + }, "node_modules/roku-debug/node_modules/fs-extra": { "version": "10.1.0", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-10.1.0.tgz", @@ -13143,6 +13159,11 @@ "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz", "integrity": "sha512-vbRorB5FUQWvla16U8R/qgaFIya2qGzwDrNmCZuYKrbdSUMG6I1ZCGQRefkRVhuOkIGVne7BQ35DSfo1qvJqFg==" }, + "eventemitter3": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", + "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==" + }, "fs-extra": { "version": "8.1.0", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-8.1.0.tgz", @@ -14676,9 +14697,9 @@ } }, "eventemitter3": { - "version": "4.0.7", - "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", - "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==" + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-5.0.1.tgz", + "integrity": "sha512-GWkBvjiSZK87ELrYOSESUYeVIc9mvLLf/nXalMOS5dYrgZq9o5OVkbZAVM06CVxYsCwH9BDZFPlQTlPA1j4ahA==" }, "expand-template": { "version": "2.0.3", @@ -15622,6 +15643,13 @@ "eventemitter3": "^4.0.0", "follow-redirects": "^1.0.0", "requires-port": "^1.0.0" + }, + "dependencies": { + "eventemitter3": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", + "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==" + } } }, "http-proxy-middleware": { @@ -17993,6 +18021,11 @@ "xml2js": "^0.5.0" }, "dependencies": { + "eventemitter3": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", + "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==" + }, "fs-extra": { "version": "10.1.0", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-10.1.0.tgz", diff --git a/package.json b/package.json index 942c35da..50e4b69d 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,7 @@ "brighterscript-formatter": "^1.6.34", "debounce": "^1.2.0", "dotenv": "^6.2.0", + "eventemitter3": "^5.0.1", "fast-xml-parser": "^3.12.16", "fs-extra": "^7.0.1", "get-port": "^5.0.0", diff --git a/src/ActiveDeviceManager.ts b/src/ActiveDeviceManager.ts index 8d292c72..3c951c99 100644 --- a/src/ActiveDeviceManager.ts +++ b/src/ActiveDeviceManager.ts @@ -1,5 +1,5 @@ import * as backoff from 'backoff'; -import { EventEmitter } from 'events'; +import { EventEmitter } from 'eventemitter3'; import * as xmlParser from 'fast-xml-parser'; import * as http from 'http'; import * as NodeCache from 'node-cache'; @@ -8,13 +8,13 @@ import { Client } from 'node-ssdp'; import { URL } from 'url'; import { util } from './util'; import * as vscode from 'vscode'; +import { firstBy } from 'thenby'; const DEFAULT_TIMEOUT = 10000; -export class ActiveDeviceManager extends EventEmitter { +export class ActiveDeviceManager { constructor() { - super(); this.isRunning = false; this.firstRequestForDevices = true; @@ -38,23 +38,70 @@ export class ActiveDeviceManager extends EventEmitter { this.deviceCache = new NodeCache({ stdTTL: 3600, checkperiod: 120 }); //anytime a device leaves the cache (either expired or manually deleted) this.deviceCache.on('del', (deviceId, device) => { - this.emit('expiredDevice', deviceId, device); + void this.emit('device-expire', device); }); this.processEnabledState(); } + private emitter = new EventEmitter(); + + public on(eventName: 'device-expire', handler: (device: RokuDeviceDetails) => void); + public on(eventName: 'device-found', handler: (device: RokuDeviceDetails) => void); + public on(eventName: string, handler: (payload: any) => void) { + this.emitter.on(eventName, handler); + return () => { + if (this.emitter !== undefined) { + this.emitter.removeListener(eventName, handler); + } + }; + } + + private async emit(eventName: 'device-expire', device: RokuDeviceDetails); + private async emit(eventName: 'device-found', device: RokuDeviceDetails); + private async emit(eventName: string, data?: any) { + //emit these events on next tick, otherwise they will be processed immediately which could cause issues + await util.sleep(0); + this.emitter?.emit(eventName, data); + } + public firstRequestForDevices: boolean; - public lastUsedDevice: string; - private enabled: boolean; + public lastUsedDevice: RokuDeviceDetails; + public enabled: boolean; private showInfoMessages: boolean; private deviceCache: NodeCache; private exponentialBackoff: any; private isRunning: boolean; - // Returns an object will all the active devices by device id - public getActiveDevices() { + /** + * Get a list of all devices discovered on the network + */ + public getActiveDevices(): RokuDeviceDetails[] { this.firstRequestForDevices = false; - return this.deviceCache.mget(this.deviceCache.keys()); + const devices = Object.values( + this.deviceCache.mget(this.deviceCache.keys()) as Record + ).sort(firstBy((a: RokuDeviceDetails, b: RokuDeviceDetails) => { + return this.getPriorityForDeviceFormFactor(a) - this.getPriorityForDeviceFormFactor(b); + }).thenBy((a: RokuDeviceDetails, b: RokuDeviceDetails) => { + if (a.id < b.id) { + return -1; + } + if (a.id > b.id) { + return 1; + } + // ids must be equal + return 0; + })); + return devices; + } + + private getPriorityForDeviceFormFactor(device: RokuDeviceDetails): number { + if (device.deviceInfo['is-stick']) { + return 0; + } + if (device.deviceInfo['is-tv']) { + return 2; + } + return 1; } // Returns the device cache statistics. @@ -114,6 +161,18 @@ export class ActiveDeviceManager extends EventEmitter { } } + /** + * The number of milliseconds since a new device was discovered + */ + public get timeSinceLastDiscoveredDevice() { + if (!this.lastDiscoveredDeviceDate) { + return 0; + } + return Date.now() - this.lastDiscoveredDeviceDate.getTime(); + } + private lastDiscoveredDeviceDate: Date; + + // Discover all Roku devices on the network and watch for new ones that connect private discoverAll(timeout: number = DEFAULT_TIMEOUT): Promise { return new Promise((resolve, reject) => { @@ -122,13 +181,16 @@ export class ActiveDeviceManager extends EventEmitter { finder.on('found', (device: RokuDeviceDetails) => { if (!devices.includes(device.id)) { - if (this.showInfoMessages && this.deviceCache.get(device.id) === undefined) { - // New device found - void vscode.window.showInformationMessage(`Device found: ${device.deviceInfo['default-device-name']}`); + if (this.deviceCache.get(device.id) === undefined) { + this.lastDiscoveredDeviceDate = new Date(); + if (this.showInfoMessages) { + // New device found + void vscode.window.showInformationMessage(`Device found: ${device.deviceInfo['default-device-name']}`); + } } this.deviceCache.set(device.id, device); devices.push(device.id); - this.emit('foundDevice', device.id, device); + this.emit('device-found', device); } }); diff --git a/src/DebugConfigurationProvider.spec.ts b/src/DebugConfigurationProvider.spec.ts index 3a3f55a9..569014a9 100644 --- a/src/DebugConfigurationProvider.spec.ts +++ b/src/DebugConfigurationProvider.spec.ts @@ -10,6 +10,7 @@ import { BrightScriptDebugConfigurationProvider } from './DebugConfigurationProv import { vscode } from './mockVscode.spec'; import { standardizePath as s } from 'brighterscript'; import * as fsExtra from 'fs-extra'; +import type { ActiveDeviceManager } from './ActiveDeviceManager'; const sinon = createSandbox(); const Module = require('module'); @@ -50,7 +51,7 @@ describe('BrightScriptConfigurationProvider', () => { let activeDeviceManager = { getActiveDevices: () => [] - }; + } as any as ActiveDeviceManager; configProvider = new BrightScriptDebugConfigurationProvider(context, activeDeviceManager, null, vscode.window.createOutputChannel('Extension')); }); diff --git a/src/DebugConfigurationProvider.ts b/src/DebugConfigurationProvider.ts index c4d681a4..36e3429a 100644 --- a/src/DebugConfigurationProvider.ts +++ b/src/DebugConfigurationProvider.ts @@ -1,4 +1,4 @@ -import { util as bslangUtil } from 'brighterscript'; +import { Deferred, util as bslangUtil } from 'brighterscript'; import * as dotenv from 'dotenv'; import * as path from 'path'; import * as fsExtra from 'fs-extra'; @@ -8,6 +8,7 @@ import type { CancellationToken, DebugConfigurationProvider, ExtensionContext, + QuickPickItem, WorkspaceFolder } from 'vscode'; import * as vscode from 'vscode'; @@ -15,12 +16,13 @@ import type { LaunchConfiguration } from 'roku-debug'; import { fileUtils } from 'roku-debug'; import { util } from './util'; import type { TelemetryManager } from './managers/TelemetryManager'; +import type { ActiveDeviceManager, RokuDeviceDetails } from './ActiveDeviceManager'; export class BrightScriptDebugConfigurationProvider implements DebugConfigurationProvider { public constructor( private context: ExtensionContext, - private activeDeviceManager: any, + private activeDeviceManager: ActiveDeviceManager, private telemetryManager: TelemetryManager, private extensionOutputChannel: vscode.OutputChannel ) { @@ -399,57 +401,20 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio if (config.host.trim() === '${promptForHost}' || (config?.deepLinkUrl?.includes('${promptForHost}'))) { if (this.activeDeviceManager.enabled) { - if (this.activeDeviceManager.firstRequestForDevices && !this.activeDeviceManager.getCacheStats().keys) { - let deviceWaitTime = 5000; - if (this.showDeviceInfoMessages) { - await vscode.window.showInformationMessage(`Device Info: Allowing time for device discovery (${deviceWaitTime} ms)`); - } - - await util.delay(deviceWaitTime); - } - - let activeDevices = this.activeDeviceManager.getActiveDevices(); - - if (activeDevices && Object.keys(activeDevices).length) { - let items = []; - - // Create the Quick Picker option items - for (const key of Object.keys(activeDevices)) { - let device = activeDevices[key]; - let itemText = `${device.ip} | ${device.deviceInfo['user-device-name']} - ${device.deviceInfo['serial-number']} - ${device.deviceInfo['model-number']}`; - - if (this.activeDeviceManager.lastUsedDevice && device.deviceInfo['default-device-name'] === this.activeDeviceManager.lastUsedDevice) { - items.unshift(itemText); - } else { - items.push(itemText); - } - } - - // Give the user the option to type their own IP incase the device they want has not yet been detected on the network - let manualIpOption = 'Other'; - items.push(manualIpOption); - - let host = await vscode.window.showQuickPick(items, { placeHolder: `Please Select a Roku or use the "${manualIpOption}" option to enter a IP` }); - - if (host === manualIpOption) { - showInputBox = true; - } else if (host) { - let defaultDeviceName = host.substring(host.toLowerCase().indexOf(' | ') + 3, host.toLowerCase().lastIndexOf(' - ')); - let deviceIP = host.substring(0, host.toLowerCase().indexOf(' | ')); - if (defaultDeviceName) { - this.activeDeviceManager.lastUsedDevice = defaultDeviceName; - } - config.host = deviceIP; - } else { - // User canceled. Give them one more change to enter an ip - showInputBox = true; - } + const host = await this.promptForHost(); + if (host === 'Enter manually') { + showInputBox = true; + } else if (host) { + config.host = host; } else { + // User canceled. Give them one more change to enter an ip showInputBox = true; } } else { showInputBox = true; } + } else { + showInputBox = true; } if (showInputBox) { @@ -467,6 +432,108 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio return config; } + /** + * Prompt the user to pick a host from a list of devices + */ + private async promptForHost() { + const deferred = new Deferred(); + + const discoveryTime = 5_000; + const manualLabel = 'Enter manually'; + + //create the quickpick item + const quickPick = vscode.window.createQuickPick(); + quickPick.placeholder = `Please Select a Roku or manually type an IP address`; + quickPick.keepScrollPosition = true; + + //allow the user to manually type an IP address + quickPick.onDidAccept(() => { + deferred.resolve(quickPick.value); + }); + + //create a text-based spinner factory for use in the "loading ..." label + const generateSpinnerText = util.createTextSpinner(3); + + const refreshList = () => { + let itemsRefreshed: Array = this.activeDeviceManager.getActiveDevices().map(device => ({ + label: `${device.ip} | ${device.deviceInfo['user-device-name']} - ${device.deviceInfo['serial-number']} - ${device.deviceInfo['model-number']}`, + device: device + })); + + const devicesLabel: QuickPickItem = { + label: this.activeDeviceManager.lastUsedDevice ? 'other devices' : 'devices', + kind: vscode.QuickPickItemKind.Separator + }; + itemsRefreshed.unshift(devicesLabel); + + //move the the most recently used device to the top + if (this.activeDeviceManager.lastUsedDevice) { + const idx = itemsRefreshed.findIndex(x => x.device?.id === this.activeDeviceManager.lastUsedDevice?.id); + const [item] = itemsRefreshed.splice(idx, 1); + itemsRefreshed.unshift(item); + + itemsRefreshed.unshift({ + label: 'last used', + kind: vscode.QuickPickItemKind.Separator + }); + } + + if (this.activeDeviceManager.timeSinceLastDiscoveredDevice < discoveryTime) { + devicesLabel.label += ` (searching ${generateSpinnerText()})`; + setTimeout(() => { + refreshList(); + }, 500); + } + + // allow user to manually type an IP address + itemsRefreshed.push( + { label: '', kind: vscode.QuickPickItemKind.Separator }, + { label: manualLabel, device: { id: Number.MAX_SAFE_INTEGER } } as any + ); + + //find the active item from our list (if there is one) + const activeItem = itemsRefreshed.find(x => { + return x.device?.id === ((quickPick.activeItems?.[0] as any)?.device as RokuDeviceDetails)?.id; + }); + quickPick.items = itemsRefreshed; + if (activeItem) { + quickPick.activeItems = [activeItem]; + } + quickPick.show(); + }; + + //anytime the device picker adds/removes a device, update the list + this.activeDeviceManager.on('device-found', refreshList); + this.activeDeviceManager.on('device-expire', refreshList); + quickPick.onDidHide(() => { + deferred.reject(new Error('No host was selected')); + quickPick.dispose(); + }); + + quickPick.onDidChangeSelection(selection => { + const selectedItem = selection[0]; + if (selectedItem) { + if (selectedItem.kind === vscode.QuickPickItemKind.Separator) { + // Handle separator selection + } else { + if (selectedItem.label === manualLabel) { + deferred.resolve(manualLabel); + } else { + const device = (selectedItem as any).device as RokuDeviceDetails; + this.activeDeviceManager.lastUsedDevice = device; + deferred.resolve(device.ip); + } + quickPick.dispose(); + } + } + }); + //run the list refresh once to show the popup + refreshList(); + const result = await deferred.promise; + quickPick.dispose(); + return result; + } + /** * Validates the password parameter in the config and opens an input ui if set to ${promptForPassword} * @param config current config object diff --git a/src/util.spec.ts b/src/util.spec.ts index 67b07cf7..9d522155 100644 --- a/src/util.spec.ts +++ b/src/util.spec.ts @@ -328,4 +328,16 @@ describe('Util', () => { ]); }); }); + + describe('generateLoadingText', () => { + it('generates properly', () => { + const spin = util.createTextSpinner(3, '-', '*'); + expect(spin()).to.eql('--*'); + expect(spin()).to.eql('-*-'); + expect(spin()).to.eql('*--'); + expect(spin()).to.eql('--*'); + expect(spin()).to.eql('-*-'); + expect(spin()).to.eql('*--'); + }); + }); }); diff --git a/src/util.ts b/src/util.ts index 447b8333..49f20dae 100644 --- a/src/util.ts +++ b/src/util.ts @@ -381,6 +381,37 @@ class Util { public escapeRegex(text: string) { return text?.toString().replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'); } + + /** + * Returns a function that generates a string of off+on chars for showing "animated" loading in text-based locations + * @param max the total number of chars to show + * @param offChar the char to show when that index is "off" + * @param onChar the char to show when that index is "on" + * @returns function that generates loading strings + */ + public createTextSpinner(max: number, offChar = '◦', onChar = '•') { + let current = 0; + const fullText = offChar.repeat(max).split(''); + /** + * Generate the next text + */ + return function spinner() { + const text = [...fullText]; + text[current++ % max] = onChar; + return text.reverse().join(''); + }; + } + + /** + * Set an interval + * @returns a function that can be called to clear the interval + */ + public setInterval(callback: () => any, duration: number) { + const handle = setInterval(callback, duration); + return () => { + clearInterval(handle); + }; + } } const util = new Util(); diff --git a/src/viewProviders/OnlineDevicesViewProvider.ts b/src/viewProviders/OnlineDevicesViewProvider.ts index 4ee6f5a1..30228186 100644 --- a/src/viewProviders/OnlineDevicesViewProvider.ts +++ b/src/viewProviders/OnlineDevicesViewProvider.ts @@ -20,21 +20,21 @@ export class OnlineDevicesViewProvider implements vscode.TreeDataProvider { - if (!this.findDeviceById(newDeviceId)) { + this.activeDeviceManager.on('device-found', newDevice => { + if (!this.findDeviceById(newDevice.id)) { // Add the device to the list this.devices.push(newDevice); this._onDidChangeTreeData.fire(null); } else { // Update the device - const foundIndex = this.devices.findIndex(device => device.id === newDeviceId); + const foundIndex = this.devices.findIndex(device => device.id === newDevice.id); this.devices[foundIndex] = newDevice; } }); - this.activeDeviceManager.on('expiredDevice', (deviceId: string, device: RokuDeviceDetails) => { + this.activeDeviceManager.on('device-expire', device => { // Remove the device from the list - const foundIndex = this.devices.findIndex(x => x.id === deviceId); + const foundIndex = this.devices.findIndex(x => x.id === device.id); this.devices.splice(foundIndex, 1); this._onDidChangeTreeData.fire(null); }); @@ -49,38 +49,11 @@ export class OnlineDevicesViewProvider implements vscode.TreeDataProvider; - private getPriorityForDeviceFormFactor(device: RokuDeviceDetails): number { - if (device.deviceInfo['is-stick']) { - return 0; - } - if (device.deviceInfo['is-tv']) { - return 2; - } - return 1; - } - getChildren(element?: DeviceTreeItem | DeviceInfoTreeItem): vscode.ProviderResult { if (!element) { if (this.devices) { - - // Process the root level devices in order by id - - let devices = this.devices.sort( - firstBy((a: RokuDeviceDetails, b: RokuDeviceDetails) => { - return this.getPriorityForDeviceFormFactor(a) - this.getPriorityForDeviceFormFactor(b); - }).thenBy((a: RokuDeviceDetails, b: RokuDeviceDetails) => { - if (a.id < b.id) { - return -1; - } - if (a.id > b.id) { - return 1; - } - // ids must be equal - return 0; - })); - let items: DeviceTreeItem[] = []; - for (const device of devices) { + for (const device of this.devices) { // Make a rook item for each device let treeItem = new DeviceTreeItem( device.deviceInfo['user-device-name'] + ' - ' + this.concealString(device.deviceInfo['serial-number']), From 9702b2fc9cb43553f11d59199d302d6963a3ed10 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Fri, 27 Oct 2023 15:47:49 -0400 Subject: [PATCH 2/7] Fix loading message --- src/DebugConfigurationProvider.ts | 27 ++++++++++++++++++--------- src/util.spec.ts | 22 ++++++++++++++++++++++ src/util.ts | 10 +++++++--- 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/DebugConfigurationProvider.ts b/src/DebugConfigurationProvider.ts index 36e3429a..3d32a109 100644 --- a/src/DebugConfigurationProvider.ts +++ b/src/DebugConfigurationProvider.ts @@ -17,6 +17,7 @@ import { fileUtils } from 'roku-debug'; import { util } from './util'; import type { TelemetryManager } from './managers/TelemetryManager'; import type { ActiveDeviceManager, RokuDeviceDetails } from './ActiveDeviceManager'; +import { debounce } from 'debounce'; export class BrightScriptDebugConfigurationProvider implements DebugConfigurationProvider { @@ -437,6 +438,7 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio */ private async promptForHost() { const deferred = new Deferred(); + const disposables: Array<() => any> = []; const discoveryTime = 5_000; const manualLabel = 'Enter manually'; @@ -454,8 +456,11 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio //create a text-based spinner factory for use in the "loading ..." label const generateSpinnerText = util.createTextSpinner(3); - const refreshList = () => { - let itemsRefreshed: Array = this.activeDeviceManager.getActiveDevices().map(device => ({ + const refreshListDebounced = debounce(() => refreshList(true), 400); + + const refreshList = (updateSpinnerText = false) => { + const devices = this.activeDeviceManager.getActiveDevices(); + let itemsRefreshed: Array = devices.map(device => ({ label: `${device.ip} | ${device.deviceInfo['user-device-name']} - ${device.deviceInfo['serial-number']} - ${device.deviceInfo['model-number']}`, device: device })); @@ -479,15 +484,13 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio } if (this.activeDeviceManager.timeSinceLastDiscoveredDevice < discoveryTime) { - devicesLabel.label += ` (searching ${generateSpinnerText()})`; - setTimeout(() => { - refreshList(); - }, 500); + devicesLabel.label += ` (searching ${generateSpinnerText(updateSpinnerText)})`; + refreshListDebounced(); } // allow user to manually type an IP address itemsRefreshed.push( - { label: '', kind: vscode.QuickPickItemKind.Separator }, + { label: ' ', kind: vscode.QuickPickItemKind.Separator }, { label: manualLabel, device: { id: Number.MAX_SAFE_INTEGER } } as any ); @@ -503,8 +506,11 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio }; //anytime the device picker adds/removes a device, update the list - this.activeDeviceManager.on('device-found', refreshList); - this.activeDeviceManager.on('device-expire', refreshList); + disposables.push( + this.activeDeviceManager.on('device-found', () => refreshList()), + this.activeDeviceManager.on('device-expire', () => refreshList()) + ); + quickPick.onDidHide(() => { deferred.reject(new Error('No host was selected')); quickPick.dispose(); @@ -531,6 +537,9 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio refreshList(); const result = await deferred.promise; quickPick.dispose(); + for (const disposable of disposables) { + disposable(); + } return result; } diff --git a/src/util.spec.ts b/src/util.spec.ts index 9d522155..331d9ca9 100644 --- a/src/util.spec.ts +++ b/src/util.spec.ts @@ -339,5 +339,27 @@ describe('Util', () => { expect(spin()).to.eql('-*-'); expect(spin()).to.eql('*--'); }); + + it('returns same value when passed false', () => { + const spin = util.createTextSpinner(3, '-', '*'); + expect(spin()).to.eql('--*'); + expect(spin(false)).to.eql('--*'); + + expect(spin()).to.eql('-*-'); + expect(spin(false)).to.eql('-*-'); + + expect(spin()).to.eql('*--'); + expect(spin(false)).to.eql('*--'); + + expect(spin()).to.eql('--*'); + expect(spin(false)).to.eql('--*'); + + expect(spin()).to.eql('-*-'); + expect(spin(false)).to.eql('-*-'); + + expect(spin()).to.eql('*--'); + expect(spin(false)).to.eql('*--'); + + }); }); }); diff --git a/src/util.ts b/src/util.ts index 49f20dae..44d032b3 100644 --- a/src/util.ts +++ b/src/util.ts @@ -390,14 +390,18 @@ class Util { * @returns function that generates loading strings */ public createTextSpinner(max: number, offChar = '◦', onChar = '•') { - let current = 0; + let current = 2; const fullText = offChar.repeat(max).split(''); /** * Generate the next text + * @param increment if false, will return the same value as last time */ - return function spinner() { + return function spinner(increment = true) { const text = [...fullText]; - text[current++ % max] = onChar; + if (increment) { + current++; + } + text[current % max] = onChar; return text.reverse().join(''); }; } From 956e6c5f71656fa923b27bffe0498c74fd1d1b37 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 30 Oct 2023 10:25:47 -0400 Subject: [PATCH 3/7] Separate list building from list showing/picking --- src/DebugConfigurationProvider.spec.ts | 149 ++++++++++++++++++++++++- src/DebugConfigurationProvider.ts | 107 +++++++++++------- src/mockVscode.spec.ts | 44 +++++++- 3 files changed, 255 insertions(+), 45 deletions(-) diff --git a/src/DebugConfigurationProvider.spec.ts b/src/DebugConfigurationProvider.spec.ts index 569014a9..8b16f1d9 100644 --- a/src/DebugConfigurationProvider.spec.ts +++ b/src/DebugConfigurationProvider.spec.ts @@ -4,13 +4,15 @@ import { assert, expect } from 'chai'; import * as path from 'path'; import { createSandbox } from 'sinon'; import type { WorkspaceFolder } from 'vscode'; +import { QuickPickItemKind } from 'vscode'; import Uri from 'vscode-uri'; import type { BrightScriptLaunchConfiguration } from './DebugConfigurationProvider'; import { BrightScriptDebugConfigurationProvider } from './DebugConfigurationProvider'; import { vscode } from './mockVscode.spec'; import { standardizePath as s } from 'brighterscript'; import * as fsExtra from 'fs-extra'; -import type { ActiveDeviceManager } from './ActiveDeviceManager'; +import type { RokuDeviceDetails } from './ActiveDeviceManager'; +import { ActiveDeviceManager } from './ActiveDeviceManager'; const sinon = createSandbox(); const Module = require('module'); @@ -49,10 +51,16 @@ describe('BrightScriptConfigurationProvider', () => { index: 0 }; - let activeDeviceManager = { - getActiveDevices: () => [] - } as any as ActiveDeviceManager; - configProvider = new BrightScriptDebugConfigurationProvider(context, activeDeviceManager, null, vscode.window.createOutputChannel('Extension')); + //prevent the 'start' method from actually running + sinon.stub(ActiveDeviceManager.prototype as any, 'start').callsFake(() => { }); + let activeDeviceManager = new ActiveDeviceManager(); + + configProvider = new BrightScriptDebugConfigurationProvider( + context, + activeDeviceManager, + null, + vscode.window.createOutputChannel('Extension') + ); }); afterEach(() => { @@ -322,4 +330,135 @@ describe('BrightScriptConfigurationProvider', () => { expect(config.rootDir).to.eql('./somePath/123'); }); }); + + describe('createHostQuickPickList', () => { + const devices: Array = [{ + deviceInfo: { + 'user-device-name': 'roku1', + 'serial-number': 'alpha', + 'model-number': 'model1' + }, + id: '1', + ip: '1.1.1.1', + location: '???' + }, { + deviceInfo: { + 'user-device-name': 'roku2', + 'serial-number': 'beta', + 'model-number': 'model2' + }, + id: '2', + ip: '1.1.1.2', + location: '???' + }, { + deviceInfo: { + 'user-device-name': 'roku3', + 'serial-number': 'charlie', + 'model-number': 'model3' + }, + id: '3', + ip: '1.1.1.3', + location: '???' + }]; + function label(device: RokuDeviceDetails) { + return `${device.ip} | ${device.deviceInfo['user-device-name']} - ${device.deviceInfo['serial-number']} - ${device.deviceInfo['model-number']}`; + } + + it('includes "manual', () => { + expect( + configProvider['createHostQuickPickList']([], undefined, '') + ).to.eql([{ + label: 'Enter manually', + device: { + id: Number.MAX_SAFE_INTEGER + } + }]); + }); + + it('includes separators for devices and manual options', () => { + expect( + configProvider['createHostQuickPickList']([devices[0]], undefined, '') + ).to.eql([ + { + kind: QuickPickItemKind.Separator, + label: 'devices' + }, + { + label: '1.1.1.1 | roku1 - alpha - model1', + device: devices[0] + }, + { + kind: QuickPickItemKind.Separator, + label: ' ' + }, { + label: 'Enter manually', + device: { + id: Number.MAX_SAFE_INTEGER + } + }] + ); + }); + + it('moves active device to the top', () => { + expect( + configProvider['createHostQuickPickList']([devices[0], devices[1], devices[2]], devices[1], '').map(x => x.label) + ).to.eql([ + 'last used', + label(devices[1]), + 'other devices', + label(devices[0]), + label(devices[2]), + ' ', + 'Enter manually' + ]); + }); + + it('includes the spinner text when "last used" and "other devices" separators are both present', () => { + expect( + configProvider['createHostQuickPickList'](devices, devices[1], ' (searching ...)').map(x => x.label) + ).to.eql([ + 'last used', + label(devices[1]), + 'other devices', + label(devices[0]), + label(devices[2]), + '(searching ...)', + 'Enter manually' + ]); + }); + + it('includes the spinner text if "devices" separator is present', () => { + expect( + configProvider['createHostQuickPickList'](devices, null, ' (searching ...)').map(x => x.label) + ).to.eql([ + 'devices', + label(devices[0]), + label(devices[1]), + label(devices[2]), + '(searching ...)', + 'Enter manually' + ]); + }); + + it('includes the spinner text if only "last used" separator is present', () => { + expect( + configProvider['createHostQuickPickList']([devices[0]], devices[0], ' (searching ...)').map(x => x.label) + ).to.eql([ + 'last used', + label(devices[0]), + '(searching ...)', + 'Enter manually' + ]); + }); + + it('includes the spinner text when no other device entries are present', () => { + expect( + configProvider['createHostQuickPickList']([], null, ' (searching ...)').map(x => x.label) + ).to.eql([ + '(searching ...)', + 'Enter manually' + ]); + }); + + }); }); diff --git a/src/DebugConfigurationProvider.ts b/src/DebugConfigurationProvider.ts index 3d32a109..14ef66b5 100644 --- a/src/DebugConfigurationProvider.ts +++ b/src/DebugConfigurationProvider.ts @@ -459,49 +459,18 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio const refreshListDebounced = debounce(() => refreshList(true), 400); const refreshList = (updateSpinnerText = false) => { - const devices = this.activeDeviceManager.getActiveDevices(); - let itemsRefreshed: Array = devices.map(device => ({ - label: `${device.ip} | ${device.deviceInfo['user-device-name']} - ${device.deviceInfo['serial-number']} - ${device.deviceInfo['model-number']}`, - device: device - })); - - const devicesLabel: QuickPickItem = { - label: this.activeDeviceManager.lastUsedDevice ? 'other devices' : 'devices', - kind: vscode.QuickPickItemKind.Separator - }; - itemsRefreshed.unshift(devicesLabel); - - //move the the most recently used device to the top - if (this.activeDeviceManager.lastUsedDevice) { - const idx = itemsRefreshed.findIndex(x => x.device?.id === this.activeDeviceManager.lastUsedDevice?.id); - const [item] = itemsRefreshed.splice(idx, 1); - itemsRefreshed.unshift(item); - - itemsRefreshed.unshift({ - label: 'last used', - kind: vscode.QuickPickItemKind.Separator - }); - } - + const { activeItems } = quickPick; + let spinnerText = ''; if (this.activeDeviceManager.timeSinceLastDiscoveredDevice < discoveryTime) { - devicesLabel.label += ` (searching ${generateSpinnerText(updateSpinnerText)})`; + spinnerText = ` (searching ${generateSpinnerText(updateSpinnerText)})`; refreshListDebounced(); } - - // allow user to manually type an IP address - itemsRefreshed.push( - { label: ' ', kind: vscode.QuickPickItemKind.Separator }, - { label: manualLabel, device: { id: Number.MAX_SAFE_INTEGER } } as any + quickPick.items = this.createHostQuickPickList( + this.activeDeviceManager.getActiveDevices(), + this.activeDeviceManager.lastUsedDevice, + spinnerText ); - - //find the active item from our list (if there is one) - const activeItem = itemsRefreshed.find(x => { - return x.device?.id === ((quickPick.activeItems?.[0] as any)?.device as RokuDeviceDetails)?.id; - }); - quickPick.items = itemsRefreshed; - if (activeItem) { - quickPick.activeItems = [activeItem]; - } + quickPick.activeItems = activeItems; quickPick.show(); }; @@ -543,6 +512,66 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio return result; } + private createHostLabel(device: RokuDeviceDetails) { + return `${device.ip} | ${device.deviceInfo['user-device-name']} - ${device.deviceInfo['serial-number']} - ${device.deviceInfo['model-number']}`; + } + + /** + * Generate the item list for the `this.promptForHost()` call + */ + private createHostQuickPickList(devices: RokuDeviceDetails[], lastUsedDevice: RokuDeviceDetails, spinnerText: string) { + //the collection of items we will eventually return + let items: Array = []; + + //yank the last used device out of the list so we can think about the remaining list more easily + lastUsedDevice = devices.find(x => x.id === lastUsedDevice?.id); + //remove the lastUsedDevice from the devices list so we can more easily reason with the rest of the list + devices = devices.filter(x => x.id !== lastUsedDevice?.id); + + // Ensure the most recently used device is at the top of the list + if (lastUsedDevice) { + //add a separator for "last used" + items.push({ + label: 'last used', + kind: vscode.QuickPickItemKind.Separator + }); + + //add the device + items.push({ + label: this.createHostLabel(lastUsedDevice), + device: lastUsedDevice + }); + } + + //add all other devices + if (devices.length > 0) { + items.push({ + label: lastUsedDevice ? 'other devices' : 'devices', + kind: vscode.QuickPickItemKind.Separator + }); + + //add each device + for (const device of devices) { + //add the device + items.push({ + label: this.createHostLabel(device), + device: device + }); + } + } + + //include a divider between devices and "manual" option (only if we have devices) + if (spinnerText || lastUsedDevice || devices.length) { + items.push({ label: spinnerText.trim() || ' ', kind: vscode.QuickPickItemKind.Separator }); + } + + // allow user to manually type an IP address + items.push( + { label: 'Enter manually', device: { id: Number.MAX_SAFE_INTEGER } } as any + ); + return items; + } + /** * Validates the password parameter in the config and opens an input ui if set to ${promptForPassword} * @param config current config object diff --git a/src/mockVscode.spec.ts b/src/mockVscode.spec.ts index 17ba818f..d9c45773 100644 --- a/src/mockVscode.spec.ts +++ b/src/mockVscode.spec.ts @@ -1,4 +1,11 @@ -import type { Command, Range, TreeDataProvider, TreeItemCollapsibleState, Uri, WorkspaceFolder, ConfigurationScope, ExtensionContext, WorkspaceConfiguration, OutputChannel } from 'vscode'; +import { EventEmitter } from 'eventemitter3'; +import type { Command, Range, TreeDataProvider, TreeItemCollapsibleState, Uri, WorkspaceFolder, ConfigurationScope, ExtensionContext, WorkspaceConfiguration, OutputChannel, QuickPickItem } from 'vscode'; + +//copied from vscode to help with unit tests +enum QuickPickItemKind { + Separator = -1, + Default = 0 +} afterEach(() => { delete vscode.workspace.workspaceFile; @@ -20,6 +27,7 @@ export let vscode = { CodeAction: class { }, Diagnostic: class { }, CallHierarchyItem: class { }, + QuickPickItemKind: QuickPickItemKind, StatusBarAlignment: { Left: 1, Right: 2 @@ -144,6 +152,7 @@ export let vscode = { onDidCloseTextDocument: () => { } }, window: { + showInputBox: () => { }, createStatusBarItem: () => { return { clear: () => { }, @@ -151,6 +160,39 @@ export let vscode = { show: () => { } }; }, + createQuickPick: () => { + class QuickPick { + private emitter = new EventEmitter(); + + public placeholder = ''; + + public items: QuickPickItem[]; + public keepScrollPosition = false; + + public show() { } + + public onDidAccept(cb) { + this.emitter.on('didAccept', cb); + } + + public onDidHide(cb) { + this.emitter.on('didHide', cb); + } + + public hide() { + this.emitter.emit('didHide'); + } + + public onDidChangeSelection(cb) { + this.emitter.on('didChangeSelection', cb); + } + + public dispose() { + this.emitter.removeAllListeners(); + } + } + return new QuickPick(); + }, createOutputChannel: function(name?: string) { return { name: name, From ec533be88021fa0afae0ad4929a16ee8086ec8e1 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 30 Oct 2023 17:43:34 -0400 Subject: [PATCH 4/7] better cleanup after host picker closes --- src/ActiveDeviceManager.ts | 17 ++++--- src/DebugConfigurationProvider.ts | 82 ++++++++++++++++++++++++------- 2 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/ActiveDeviceManager.ts b/src/ActiveDeviceManager.ts index 3c951c99..24bf17c2 100644 --- a/src/ActiveDeviceManager.ts +++ b/src/ActiveDeviceManager.ts @@ -9,6 +9,7 @@ import { URL } from 'url'; import { util } from './util'; import * as vscode from 'vscode'; import { firstBy } from 'thenby'; +import type { Disposable } from 'vscode'; const DEFAULT_TIMEOUT = 10000; @@ -45,13 +46,15 @@ export class ActiveDeviceManager { private emitter = new EventEmitter(); - public on(eventName: 'device-expire', handler: (device: RokuDeviceDetails) => void); - public on(eventName: 'device-found', handler: (device: RokuDeviceDetails) => void); - public on(eventName: string, handler: (payload: any) => void) { + public on(eventName: 'device-expire', handler: (device: RokuDeviceDetails) => void): Disposable; + public on(eventName: 'device-found', handler: (device: RokuDeviceDetails) => void): Disposable; + public on(eventName: string, handler: (payload: any) => void): Disposable { this.emitter.on(eventName, handler); - return () => { - if (this.emitter !== undefined) { - this.emitter.removeListener(eventName, handler); + return { + dispose: () => { + if (this.emitter !== undefined) { + this.emitter.removeListener(eventName, handler); + } } }; } @@ -254,7 +257,7 @@ class RokuFinder extends EventEmitter { const device: RokuDeviceDetails = { location: url.origin, ip: url.hostname, - id: info['device-info']['device-id'], + id: info['device-info']['device-id']?.toString?.(), deviceInfo: info['device-info'] }; this.emit('found', device); diff --git a/src/DebugConfigurationProvider.ts b/src/DebugConfigurationProvider.ts index 14ef66b5..67dead10 100644 --- a/src/DebugConfigurationProvider.ts +++ b/src/DebugConfigurationProvider.ts @@ -7,6 +7,7 @@ import * as rta from 'roku-test-automation'; import type { CancellationToken, DebugConfigurationProvider, + Disposable, ExtensionContext, QuickPickItem, WorkspaceFolder @@ -19,6 +20,12 @@ import type { TelemetryManager } from './managers/TelemetryManager'; import type { ActiveDeviceManager, RokuDeviceDetails } from './ActiveDeviceManager'; import { debounce } from 'debounce'; +/** + * An id to represent the "Enter manually" option in the host picker + */ +const manualHostItemId = `${Number.MAX_SAFE_INTEGER}`; +const manualLabel = 'Enter manually'; + export class BrightScriptDebugConfigurationProvider implements DebugConfigurationProvider { public constructor( @@ -438,16 +445,22 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio */ private async promptForHost() { const deferred = new Deferred(); - const disposables: Array<() => any> = []; + const disposables: Array = []; const discoveryTime = 5_000; - const manualLabel = 'Enter manually'; //create the quickpick item const quickPick = vscode.window.createQuickPick(); + disposables.push(quickPick); quickPick.placeholder = `Please Select a Roku or manually type an IP address`; quickPick.keepScrollPosition = true; + function dispose() { + for (const disposable of disposables) { + disposable.dispose(); + } + } + //allow the user to manually type an IP address quickPick.onDidAccept(() => { deferred.resolve(quickPick.value); @@ -456,33 +469,57 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio //create a text-based spinner factory for use in the "loading ..." label const generateSpinnerText = util.createTextSpinner(3); + const itemCache = new Map(); + const refreshListDebounced = debounce(() => refreshList(true), 400); const refreshList = (updateSpinnerText = false) => { + console.log('refreshList', { updateSpinnerText: updateSpinnerText }); const { activeItems } = quickPick; let spinnerText = ''; if (this.activeDeviceManager.timeSinceLastDiscoveredDevice < discoveryTime) { spinnerText = ` (searching ${generateSpinnerText(updateSpinnerText)})`; refreshListDebounced(); } - quickPick.items = this.createHostQuickPickList( + const items = this.createHostQuickPickList( this.activeDeviceManager.getActiveDevices(), this.activeDeviceManager.lastUsedDevice, - spinnerText + spinnerText, + itemCache ); - quickPick.activeItems = activeItems; + quickPick.items = items; + + // highlight the first non-separator item + if (activeItems.length === 0) { + for (const item of items) { + if (item.kind !== vscode.QuickPickItemKind.Separator && item.device?.id !== manualHostItemId) { + quickPick.activeItems = [item]; + break; + } + } + } else { + //restore previously highlighted item + quickPick.activeItems = activeItems; + } quickPick.show(); }; //anytime the device picker adds/removes a device, update the list disposables.push( - this.activeDeviceManager.on('device-found', () => refreshList()), - this.activeDeviceManager.on('device-expire', () => refreshList()) + this.activeDeviceManager.on('device-found', () => { + console.log('device found'); + refreshList(); + }), + this.activeDeviceManager.on('device-expire', () => { + console.log('device expire'); + refreshList(); + }) ); quickPick.onDidHide(() => { + dispose(); deferred.reject(new Error('No host was selected')); - quickPick.dispose(); + }); quickPick.onDidChangeSelection(selection => { @@ -505,10 +542,7 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio //run the list refresh once to show the popup refreshList(); const result = await deferred.promise; - quickPick.dispose(); - for (const disposable of disposables) { - disposable(); - } + dispose(); return result; } @@ -519,12 +553,12 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio /** * Generate the item list for the `this.promptForHost()` call */ - private createHostQuickPickList(devices: RokuDeviceDetails[], lastUsedDevice: RokuDeviceDetails, spinnerText: string) { + private createHostQuickPickList(devices: RokuDeviceDetails[], lastUsedDevice: RokuDeviceDetails, spinnerText: string, cache = new Map()) { //the collection of items we will eventually return - let items: Array = []; + let items: QuickPickHostItem[] = []; - //yank the last used device out of the list so we can think about the remaining list more easily - lastUsedDevice = devices.find(x => x.id === lastUsedDevice?.id); + //find the lastUsedDevice from the devices list if possible, or use the data from the lastUsedDevice if not + lastUsedDevice = devices.find(x => x.id === lastUsedDevice?.id) ?? lastUsedDevice; //remove the lastUsedDevice from the devices list so we can more easily reason with the rest of the list devices = devices.filter(x => x.id !== lastUsedDevice?.id); @@ -567,8 +601,20 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio // allow user to manually type an IP address items.push( - { label: 'Enter manually', device: { id: Number.MAX_SAFE_INTEGER } } as any + { label: 'Enter manually', device: { id: manualHostItemId } } as any ); + + // replace items with their cached versions if found (to maintain references) + for (let i = 0; i < items.length; i++) { + const item = items[i]; + if (cache.has(item.label)) { + items[i] = cache.get(item.label); + items[i].device = item.device; + } else { + cache.set(item.label, item); + } + } + return items; } @@ -690,3 +736,5 @@ export interface BrightScriptLaunchConfiguration extends LaunchConfiguration { */ remoteControlMode?: { activateOnSessionStart?: boolean; deactivateOnSessionEnd?: boolean }; } + +type QuickPickHostItem = QuickPickItem & { device?: RokuDeviceDetails }; From ec7258dd52785a3936b92851ae5b00d367226ca3 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 30 Oct 2023 20:37:14 -0400 Subject: [PATCH 5/7] Fix some tests --- src/DebugConfigurationProvider.spec.ts | 5 +++-- src/DebugConfigurationProvider.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/DebugConfigurationProvider.spec.ts b/src/DebugConfigurationProvider.spec.ts index 8b16f1d9..0ca4d574 100644 --- a/src/DebugConfigurationProvider.spec.ts +++ b/src/DebugConfigurationProvider.spec.ts @@ -7,6 +7,7 @@ import type { WorkspaceFolder } from 'vscode'; import { QuickPickItemKind } from 'vscode'; import Uri from 'vscode-uri'; import type { BrightScriptLaunchConfiguration } from './DebugConfigurationProvider'; +import { manualHostItemId } from './DebugConfigurationProvider'; import { BrightScriptDebugConfigurationProvider } from './DebugConfigurationProvider'; import { vscode } from './mockVscode.spec'; import { standardizePath as s } from 'brighterscript'; @@ -370,7 +371,7 @@ describe('BrightScriptConfigurationProvider', () => { ).to.eql([{ label: 'Enter manually', device: { - id: Number.MAX_SAFE_INTEGER + id: manualHostItemId } }]); }); @@ -393,7 +394,7 @@ describe('BrightScriptConfigurationProvider', () => { }, { label: 'Enter manually', device: { - id: Number.MAX_SAFE_INTEGER + id: manualHostItemId } }] ); diff --git a/src/DebugConfigurationProvider.ts b/src/DebugConfigurationProvider.ts index 67dead10..851d0609 100644 --- a/src/DebugConfigurationProvider.ts +++ b/src/DebugConfigurationProvider.ts @@ -23,7 +23,7 @@ import { debounce } from 'debounce'; /** * An id to represent the "Enter manually" option in the host picker */ -const manualHostItemId = `${Number.MAX_SAFE_INTEGER}`; +export const manualHostItemId = `${Number.MAX_SAFE_INTEGER}`; const manualLabel = 'Enter manually'; export class BrightScriptDebugConfigurationProvider implements DebugConfigurationProvider { From 3a96ae6af3480b23010af1419d859302206d3d3b Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 31 Oct 2023 10:07:49 -0400 Subject: [PATCH 6/7] Use built-in busy spinner instead of changing text --- src/ActiveDeviceManager.ts | 24 ++-- src/DebugConfigurationProvider.spec.ts | 21 ++- src/DebugConfigurationProvider.ts | 125 +++++++++--------- .../OnlineDevicesViewProvider.ts | 2 +- 4 files changed, 85 insertions(+), 87 deletions(-) diff --git a/src/ActiveDeviceManager.ts b/src/ActiveDeviceManager.ts index 24bf17c2..7f03ca4f 100644 --- a/src/ActiveDeviceManager.ts +++ b/src/ActiveDeviceManager.ts @@ -39,27 +39,31 @@ export class ActiveDeviceManager { this.deviceCache = new NodeCache({ stdTTL: 3600, checkperiod: 120 }); //anytime a device leaves the cache (either expired or manually deleted) this.deviceCache.on('del', (deviceId, device) => { - void this.emit('device-expire', device); + void this.emit('device-expired', device); }); this.processEnabledState(); } private emitter = new EventEmitter(); - public on(eventName: 'device-expire', handler: (device: RokuDeviceDetails) => void): Disposable; - public on(eventName: 'device-found', handler: (device: RokuDeviceDetails) => void): Disposable; - public on(eventName: string, handler: (payload: any) => void): Disposable { + public on(eventName: 'device-expired', handler: (device: RokuDeviceDetails) => void, disposables?: Disposable[]): () => void; + public on(eventName: 'device-found', handler: (device: RokuDeviceDetails) => void, disposables?: Disposable[]): () => void; + public on(eventName: string, handler: (payload: any) => void, disposables?: Disposable[]): () => void { this.emitter.on(eventName, handler); - return { - dispose: () => { - if (this.emitter !== undefined) { - this.emitter.removeListener(eventName, handler); - } + const unsubscribe = () => { + if (this.emitter !== undefined) { + this.emitter.removeListener(eventName, handler); } }; + + disposables?.push({ + dispose: unsubscribe + }); + + return unsubscribe; } - private async emit(eventName: 'device-expire', device: RokuDeviceDetails); + private async emit(eventName: 'device-expired', device: RokuDeviceDetails); private async emit(eventName: 'device-found', device: RokuDeviceDetails); private async emit(eventName: string, data?: any) { //emit these events on next tick, otherwise they will be processed immediately which could cause issues diff --git a/src/DebugConfigurationProvider.spec.ts b/src/DebugConfigurationProvider.spec.ts index 0ca4d574..efd53e4c 100644 --- a/src/DebugConfigurationProvider.spec.ts +++ b/src/DebugConfigurationProvider.spec.ts @@ -367,7 +367,7 @@ describe('BrightScriptConfigurationProvider', () => { it('includes "manual', () => { expect( - configProvider['createHostQuickPickList']([], undefined, '') + configProvider['createHostQuickPickList']([], undefined) ).to.eql([{ label: 'Enter manually', device: { @@ -378,7 +378,7 @@ describe('BrightScriptConfigurationProvider', () => { it('includes separators for devices and manual options', () => { expect( - configProvider['createHostQuickPickList']([devices[0]], undefined, '') + configProvider['createHostQuickPickList']([devices[0]], undefined) ).to.eql([ { kind: QuickPickItemKind.Separator, @@ -402,7 +402,7 @@ describe('BrightScriptConfigurationProvider', () => { it('moves active device to the top', () => { expect( - configProvider['createHostQuickPickList']([devices[0], devices[1], devices[2]], devices[1], '').map(x => x.label) + configProvider['createHostQuickPickList']([devices[0], devices[1], devices[2]], devices[1]).map(x => x.label) ).to.eql([ 'last used', label(devices[1]), @@ -416,47 +416,46 @@ describe('BrightScriptConfigurationProvider', () => { it('includes the spinner text when "last used" and "other devices" separators are both present', () => { expect( - configProvider['createHostQuickPickList'](devices, devices[1], ' (searching ...)').map(x => x.label) + configProvider['createHostQuickPickList'](devices, devices[1]).map(x => x.label) ).to.eql([ 'last used', label(devices[1]), 'other devices', label(devices[0]), label(devices[2]), - '(searching ...)', + ' ', 'Enter manually' ]); }); it('includes the spinner text if "devices" separator is present', () => { expect( - configProvider['createHostQuickPickList'](devices, null, ' (searching ...)').map(x => x.label) + configProvider['createHostQuickPickList'](devices, null).map(x => x.label) ).to.eql([ 'devices', label(devices[0]), label(devices[1]), label(devices[2]), - '(searching ...)', + ' ', 'Enter manually' ]); }); it('includes the spinner text if only "last used" separator is present', () => { expect( - configProvider['createHostQuickPickList']([devices[0]], devices[0], ' (searching ...)').map(x => x.label) + configProvider['createHostQuickPickList']([devices[0]], devices[0]).map(x => x.label) ).to.eql([ 'last used', label(devices[0]), - '(searching ...)', + ' ', 'Enter manually' ]); }); it('includes the spinner text when no other device entries are present', () => { expect( - configProvider['createHostQuickPickList']([], null, ' (searching ...)').map(x => x.label) + configProvider['createHostQuickPickList']([], null).map(x => x.label) ).to.eql([ - '(searching ...)', 'Enter manually' ]); }); diff --git a/src/DebugConfigurationProvider.ts b/src/DebugConfigurationProvider.ts index 851d0609..279ea0ef 100644 --- a/src/DebugConfigurationProvider.ts +++ b/src/DebugConfigurationProvider.ts @@ -400,35 +400,22 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio return config; } + private async promptForHostManual() { + return this.openInputBox('The IP address of your Roku device'); + } + /** * Validates the host parameter in the config and opens an input ui if set to ${promptForHost} * @param config current config object */ private async processHostParameter(config: BrightScriptLaunchConfiguration): Promise { - let showInputBox = false; - if (config.host.trim() === '${promptForHost}' || (config?.deepLinkUrl?.includes('${promptForHost}'))) { if (this.activeDeviceManager.enabled) { - const host = await this.promptForHost(); - if (host === 'Enter manually') { - showInputBox = true; - } else if (host) { - config.host = host; - } else { - // User canceled. Give them one more change to enter an ip - showInputBox = true; - } + config.host = await this.promptForHost(); } else { - showInputBox = true; + config.host = await this.promptForHostManual(); } - } else { - showInputBox = true; - } - - if (showInputBox) { - config.host = await this.openInputBox('The IP address of your Roku device'); } - // #endregion //check the host and throw error if not provided or update the workspace to set last host if (!config.host) { @@ -444,7 +431,7 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio * Prompt the user to pick a host from a list of devices */ private async promptForHost() { - const deferred = new Deferred(); + const deferred = new Deferred<{ ip: string; manual?: boolean } | { ip?: string; manual: true }>(); const disposables: Array = []; const discoveryTime = 5_000; @@ -461,65 +448,64 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio } } - //allow the user to manually type an IP address + //detect if the user types an IP address into the picker and presses enter. quickPick.onDidAccept(() => { - deferred.resolve(quickPick.value); + deferred.resolve({ + ip: quickPick.value + }); }); - //create a text-based spinner factory for use in the "loading ..." label - const generateSpinnerText = util.createTextSpinner(3); - - const itemCache = new Map(); - - const refreshListDebounced = debounce(() => refreshList(true), 400); + let activeChangesSinceRefresh = 0; + let activeItem: QuickPickItem; - const refreshList = (updateSpinnerText = false) => { - console.log('refreshList', { updateSpinnerText: updateSpinnerText }); - const { activeItems } = quickPick; - let spinnerText = ''; - if (this.activeDeviceManager.timeSinceLastDiscoveredDevice < discoveryTime) { - spinnerText = ` (searching ${generateSpinnerText(updateSpinnerText)})`; - refreshListDebounced(); + // remember the currently active item so we can maintain active selection when refreshing the list + quickPick.onDidChangeActive((items) => { + // reset our activeChanges tracker since users cannot cause items.length to be 0 (meaning a refresh has just happened) + if (items.length === 0) { + activeChangesSinceRefresh = 0; + return; } + if (activeChangesSinceRefresh > 0) { + activeItem = items[0]; + } + activeChangesSinceRefresh++; + }); + + const itemCache = new Map(); + quickPick.show(); + const refreshList = () => { const items = this.createHostQuickPickList( this.activeDeviceManager.getActiveDevices(), this.activeDeviceManager.lastUsedDevice, - spinnerText, itemCache ); quickPick.items = items; - // highlight the first non-separator item - if (activeItems.length === 0) { - for (const item of items) { - if (item.kind !== vscode.QuickPickItemKind.Separator && item.device?.id !== manualHostItemId) { - quickPick.activeItems = [item]; - break; - } - } - } else { - //restore previously highlighted item - quickPick.activeItems = activeItems; + // update the busy spinner based on how long it's been since the last discovered device + quickPick.busy = this.activeDeviceManager.timeSinceLastDiscoveredDevice < discoveryTime; + setTimeout(() => { + quickPick.busy = this.activeDeviceManager.timeSinceLastDiscoveredDevice < discoveryTime; + }, discoveryTime - this.activeDeviceManager.timeSinceLastDiscoveredDevice + 20); + + // clear the activeItem if we can't find it in the list + if (!quickPick.items.includes(activeItem)) { + activeItem = undefined; + } + + // if the user manually selected an item, re-focus that item now that we refreshed the list + if (activeItem) { + quickPick.activeItems = [activeItem]; } - quickPick.show(); + // quickPick.show(); }; //anytime the device picker adds/removes a device, update the list - disposables.push( - this.activeDeviceManager.on('device-found', () => { - console.log('device found'); - refreshList(); - }), - this.activeDeviceManager.on('device-expire', () => { - console.log('device expire'); - refreshList(); - }) - ); + this.activeDeviceManager.on('device-found', refreshList, disposables); + this.activeDeviceManager.on('device-expired', refreshList, disposables); quickPick.onDidHide(() => { dispose(); deferred.reject(new Error('No host was selected')); - }); quickPick.onDidChangeSelection(selection => { @@ -529,11 +515,11 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio // Handle separator selection } else { if (selectedItem.label === manualLabel) { - deferred.resolve(manualLabel); + deferred.resolve({ manual: true }); } else { const device = (selectedItem as any).device as RokuDeviceDetails; this.activeDeviceManager.lastUsedDevice = device; - deferred.resolve(device.ip); + deferred.resolve(device); } quickPick.dispose(); } @@ -543,9 +529,18 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio refreshList(); const result = await deferred.promise; dispose(); - return result; + if (result?.manual === true) { + return this.promptForHostManual(); + } else { + return result?.ip; + } } + /** + * Generate the label used when showing "host" entries in a quick picker + * @param device the device containing all the info + * @returns a properly formatted host string + */ private createHostLabel(device: RokuDeviceDetails) { return `${device.ip} | ${device.deviceInfo['user-device-name']} - ${device.deviceInfo['serial-number']} - ${device.deviceInfo['model-number']}`; } @@ -553,7 +548,7 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio /** * Generate the item list for the `this.promptForHost()` call */ - private createHostQuickPickList(devices: RokuDeviceDetails[], lastUsedDevice: RokuDeviceDetails, spinnerText: string, cache = new Map()) { + private createHostQuickPickList(devices: RokuDeviceDetails[], lastUsedDevice: RokuDeviceDetails, cache = new Map()) { //the collection of items we will eventually return let items: QuickPickHostItem[] = []; @@ -595,8 +590,8 @@ export class BrightScriptDebugConfigurationProvider implements DebugConfiguratio } //include a divider between devices and "manual" option (only if we have devices) - if (spinnerText || lastUsedDevice || devices.length) { - items.push({ label: spinnerText.trim() || ' ', kind: vscode.QuickPickItemKind.Separator }); + if (lastUsedDevice || devices.length) { + items.push({ label: ' ', kind: vscode.QuickPickItemKind.Separator }); } // allow user to manually type an IP address diff --git a/src/viewProviders/OnlineDevicesViewProvider.ts b/src/viewProviders/OnlineDevicesViewProvider.ts index 30228186..91b1e105 100644 --- a/src/viewProviders/OnlineDevicesViewProvider.ts +++ b/src/viewProviders/OnlineDevicesViewProvider.ts @@ -32,7 +32,7 @@ export class OnlineDevicesViewProvider implements vscode.TreeDataProvider { + this.activeDeviceManager.on('device-expired', device => { // Remove the device from the list const foundIndex = this.devices.findIndex(x => x.id === device.id); this.devices.splice(foundIndex, 1); From 29362f3efd4cb274a2a710a0e8b4d9666fecc6c5 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 31 Oct 2023 10:12:43 -0400 Subject: [PATCH 7/7] remove unused util functions --- src/util.spec.ts | 34 ---------------------------------- src/util.ts | 35 ----------------------------------- 2 files changed, 69 deletions(-) diff --git a/src/util.spec.ts b/src/util.spec.ts index 331d9ca9..67b07cf7 100644 --- a/src/util.spec.ts +++ b/src/util.spec.ts @@ -328,38 +328,4 @@ describe('Util', () => { ]); }); }); - - describe('generateLoadingText', () => { - it('generates properly', () => { - const spin = util.createTextSpinner(3, '-', '*'); - expect(spin()).to.eql('--*'); - expect(spin()).to.eql('-*-'); - expect(spin()).to.eql('*--'); - expect(spin()).to.eql('--*'); - expect(spin()).to.eql('-*-'); - expect(spin()).to.eql('*--'); - }); - - it('returns same value when passed false', () => { - const spin = util.createTextSpinner(3, '-', '*'); - expect(spin()).to.eql('--*'); - expect(spin(false)).to.eql('--*'); - - expect(spin()).to.eql('-*-'); - expect(spin(false)).to.eql('-*-'); - - expect(spin()).to.eql('*--'); - expect(spin(false)).to.eql('*--'); - - expect(spin()).to.eql('--*'); - expect(spin(false)).to.eql('--*'); - - expect(spin()).to.eql('-*-'); - expect(spin(false)).to.eql('-*-'); - - expect(spin()).to.eql('*--'); - expect(spin(false)).to.eql('*--'); - - }); - }); }); diff --git a/src/util.ts b/src/util.ts index 44d032b3..447b8333 100644 --- a/src/util.ts +++ b/src/util.ts @@ -381,41 +381,6 @@ class Util { public escapeRegex(text: string) { return text?.toString().replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'); } - - /** - * Returns a function that generates a string of off+on chars for showing "animated" loading in text-based locations - * @param max the total number of chars to show - * @param offChar the char to show when that index is "off" - * @param onChar the char to show when that index is "on" - * @returns function that generates loading strings - */ - public createTextSpinner(max: number, offChar = '◦', onChar = '•') { - let current = 2; - const fullText = offChar.repeat(max).split(''); - /** - * Generate the next text - * @param increment if false, will return the same value as last time - */ - return function spinner(increment = true) { - const text = [...fullText]; - if (increment) { - current++; - } - text[current % max] = onChar; - return text.reverse().join(''); - }; - } - - /** - * Set an interval - * @returns a function that can be called to clear the interval - */ - public setInterval(callback: () => any, duration: number) { - const handle = setInterval(callback, duration); - return () => { - clearInterval(handle); - }; - } } const util = new Util();