Store/restore view states to/from settings.json#997
Open
jthuangarm wants to merge 20 commits into
Open
Conversation
…d to reset all settings
…or periodicUpdate
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds persistence for “dynamic” UI state (periodic-update toggles, view filters, and CPU timer enablement) by storing/restoring per-debug-configuration state in settings.json, plus a new command to reset all persisted dynamic view state.
Changes:
- Introduces a
vscode-cmsis-debugger.viewStateconfiguration object and helper module to read/merge/write per-config view state. - Restores/saves Component Viewer/Core Peripherals periodic update + filter state, and persists CPU timer enablement per debug configuration.
- Adds a
vscode-cmsis-debugger.resetDynamicViewStatecommand and small UX updates (toolbar/command icons, status bar quick pick enable/disable).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/views/dynamic-view-states.ts | New helper for reading/merging/writing per-config dynamic view state to VS Code settings. |
| src/views/dynamic-view-states.test.ts | Unit tests for dynamic view-state read/write/merge behavior. |
| src/views/component-viewer/component-viewer-base.ts | Saves/restores periodic-update + filter state; debounced persistence while typing. |
| src/views/component-viewer/test/unit/component-viewer-base.test.ts | Updates unit tests to cover view-state restore/reset behavior. |
| src/features/cpu-states/cpu-states.ts | Persists CPU timer enable/disable per config and wires restored state into runtime. |
| src/features/cpu-states/cpu-states.test.ts | Adds tests for persistence/restore and context switching behavior. |
| src/features/cpu-states/cpu-states-statusbar-item.ts | Adds enable/disable CPU timer actions to the status bar quick pick. |
| src/desktop/extension.ts | Registers “Reset All Dynamic View States” command and resets runtime state. |
| src/debug-session/gdbtarget-debug-session.ts | Adds getConfigStateKey() to compute a stable persistence key including target-type. |
| src/debug-session/test/debug-session.factory.ts | Extends test session factory types to support config state keys and target-type. |
| src/cbuild-run/cbuild-run-reader.ts | Exposes getTargetType() used for deriving persistence keys. |
| package.json | Adds new command, view-state configuration schema, and periodic-update command icons/menus. |
| mocks/vscode.js | Extends VS Code mock to support configuration inspect/update and ConfigurationTarget. |
Comments suppressed due to low confidence (4)
src/views/component-viewer/test/unit/component-viewer-base.test.ts:137
readComponentViewerStateis configured withmockReturnValue(undefined)twice in a row. This is redundant and likely a copy/paste mistake; keep a single mock setup to avoid masking future changes to the intended behavior.
controller = createController(context, provider);
asMockedFunction(readComponentViewerState).mockReturnValue(undefined);
asMockedFunction(readComponentViewerState).mockReturnValue(undefined);
src/views/component-viewer/test/unit/component-viewer-base.test.ts:1068
- This test sets up
readComponentViewerState.mockReturnValue(...)twice and asserts the call/side-effects twice, butrestorePeriodicUpdateAndFilteronly reads state once and applies the filter once. As written, the duplicated expectations are likely to fail; remove the duplicates and assert the expected single call / singlesetFilterinvocation.
asMockedFunction(readComponentViewerState).mockReturnValue({
periodicUpdateEnabled: false,
filterPattern: 'word',
});
asMockedFunction(readComponentViewerState).mockReturnValue({
periodicUpdateEnabled: false,
filterPattern: 'word',
});
const session = debugSessionFactory('s1');
const restoreState = (controller as unknown as {
restorePeriodicUpdateAndFilter: (s: Session) => Promise<void>;
}).restorePeriodicUpdateAndFilter.bind(controller);
await restoreState(session);
expect(readComponentViewerState).toHaveBeenCalledWith('testClass', 's1');
expect(readComponentViewerState).toHaveBeenCalledWith('testClass', 's1');
expect((controller as unknown as { _refreshTimerEnabled: boolean })._refreshTimerEnabled).toBe(false);
expect(vscode.commands.executeCommand).toHaveBeenCalledWith('setContext', 'testClass.periodicUpdateEnabled', false);
expect(provider.setFilter).toHaveBeenCalledWith('word');
expect(provider.setFilter).toHaveBeenCalledWith('word');
expect(vscode.commands.executeCommand).toHaveBeenCalledWith('setContext', 'testClass.filterActive', true);
src/views/component-viewer/component-viewer-base.ts:241
- Inside the debounce timer callback,
saveCurrentState()(async) is invoked without awaiting or handling the returned Promise. If it rejects, this becomes an unhandled rejection. Considervoid this.saveCurrentState().catch(...)(or make the callback async and handle errors) to avoid silent failures.
this._filterDebounceTimer = setTimeout(() => {
this._filterDebounceTimer = undefined;
this.saveCurrentState();
}, ComponentViewerBase.filterDebounceMs);
src/views/component-viewer/component-viewer-base.ts:278
handleClearFilter()callssaveCurrentState()(async) without awaiting/handling the returned Promise. If persisting settings fails, the rejection will be unobserved. Considervoid this.saveCurrentState().catch(...)(or convertinghandleClearFilterto async and awaiting) for reliability.
// Cancel any pending debounced save and persist the cleared state immediately.
if (this._filterDebounceTimer) {
clearTimeout(this._filterDebounceTimer);
this._filterDebounceTimer = undefined;
}
this.saveCurrentState();
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (6)
src/views/component-viewer/component-viewer-base.ts:140
vscode.commands.executeCommand('setContext', ...)returns a Thenable; here it’s invoked withoutawaitorvoid, which can violateno-floating-promisesand may surface unhandled rejections. Pleaseawaitit (if you need ordering) or prefix withvoidto explicitly ignore the promise.
vscode.commands.executeCommand('setContext', `${this._viewId}.periodicUpdateEnabled`, true);
return true;
src/views/component-viewer/component-viewer-base.ts:546
- The
setContextcalls in this reset-to-defaults block are not awaited or explicitly ignored. To avoid floating promises (and keep consistent with other calls in this file, e.g.void vscode.commands.executeCommand(...)), usevoidorawaitfor these commands.
// Always reset to defaults before applying saved state to prevent state leaking while switching sessions
this._refreshTimerEnabled = true;
vscode.commands.executeCommand('setContext', `${this._viewId}.periodicUpdateEnabled`, true);
this._componentViewerTreeDataProvider.setFilter(undefined);
vscode.commands.executeCommand('setContext', `${this._viewId}.filterActive`, false);
src/views/component-viewer/component-viewer-base.ts:560
- These additional
setContextcalls are also made withoutawait/void. Please eitherawaitthem (if subsequent logic depends on the context update) or explicitly ignore withvoidto satisfy promise-handling rules.
if (state.periodicUpdateEnabled !== undefined) {
this._refreshTimerEnabled = state.periodicUpdateEnabled;
vscode.commands.executeCommand('setContext', `${this._viewId}.periodicUpdateEnabled`, state.periodicUpdateEnabled);
componentViewerLogger.info(`${this._viewName}: Restored periodicUpdateEnabled=${state.periodicUpdateEnabled}`);
}
if (state.filterPattern !== undefined) {
this._componentViewerTreeDataProvider.setFilter(state.filterPattern);
vscode.commands.executeCommand('setContext', `${this._viewId}.filterActive`, true);
componentViewerLogger.info(`${this._viewName}: Restored filterPattern='${state.filterPattern}'`);
src/views/component-viewer/component-viewer-base.ts:569
resetViewState()callsvscode.commands.executeCommand('setContext', ...)withoutawait/void, which can triggerno-floating-promisesand makes ordering unclear. Please handle the returned Thenable explicitly.
public async resetViewState(): Promise<void> {
// Reset in-memory state to defaults.
this._refreshTimerEnabled = true;
vscode.commands.executeCommand('setContext', `${this._viewId}.periodicUpdateEnabled`, true);
this._componentViewerTreeDataProvider.setFilter(undefined);
vscode.commands.executeCommand('setContext', `${this._viewId}.filterActive`, false);
src/features/cpu-states/cpu-states.ts:373
vscode.commands.executeCommand('setContext', ...)is invoked withoutawait/void. Please handle/ignore the Thenable explicitly (consistent with the rest of the codebase) to avoid floating promises.
for (const states of this.sessionCpuStates.values()) {
states.enableCpuStatesFlag = true;
}
vscode.commands.executeCommand('setContext', 'vscode-cmsis-debugger.cpuTimerEnabled', true);
logger.info('CPU States: CPU Timer reset');
src/views/component-viewer/test/unit/component-viewer-base.test.ts:1050
- This test sets the same
mockReturnValuetwice and then repeatstoHaveBeenCalledWithassertions. More importantly, it doesn’t assert the expected call sequence (setFilter(undefined)for reset, thensetFilter('word')), so it won’t catch regressions where defaults aren’t applied first. Consider removing duplicate lines and asserting call order / call counts to verify the reset-then-apply behavior.
it('restorePeriodicUpdateAndFilter restores periodicUpdateEnabled and filter from workspace settings', async () => {
asMockedFunction(readComponentViewerState).mockReturnValue({
periodicUpdateEnabled: false,
filterPattern: 'word',
});
asMockedFunction(readComponentViewerState).mockReturnValue({
periodicUpdateEnabled: false,
filterPattern: 'word',
});
Comment on lines
83
to
88
| isRunning: true, | ||
| hasStates: undefined, | ||
| skipFrequencyUpdate: false | ||
| skipFrequencyUpdate: false, | ||
| enableCpuStatesFlag: true, | ||
| configStateKey: session.session.configuration.name | ||
| }; |
Collaborator
Author
There was a problem hiding this comment.
This is intentional.
At session start, the cbuild-run file may not be parsed yet, so I use session.session.configuration.name as a temporary key. handleConnected() then refines it with session.getConfigStateKey() once the parsed cbuild-run data is available.
The risky window is only between session start and handleConnected() completion, which should be very short. Avoiding it completely would require pending writes or migration logic, which seems too complex for this edge case.
|
Coverage Impact ⬇️ Merging this pull request will decrease total coverage on Modified Files with Diff Coverage (7)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
Changes
vscode-cmsis-debugger.viewStateconfiguration property inpackage.jsonto persist dynamic view state per debug configuration, including fields forcomponentViewer,corePeripherals, andcpuStates.vscode-cmsis-debugger.resetDynamicViewStateto reset all dynamic view states.Component ViewerandCore Peripherals.Example output in settings.json:
Screenshots
To enable/disable CPU Timer:
To enable/disable periodic update:
Checklist