refactor: declarative command registry (-196 net lines)#65
Open
CompN3rd wants to merge 2 commits into
Open
Conversation
Adds src/commands/commandRegistry.ts with three helpers (add, addGuarded, addRefreshing) that fold the common command shapes: plain commands, sign-in-gated commands, and gated+refresh-on-truthy commands. Migrates all 64 command registrations in extension.ts to use the registry. Net savings: 256 lines (1186 -> 930). No behavior change. All command IDs, signatures, and semantics preserved. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Pull request overview
This PR refactors command registration in the extension activation path by introducing a small CommandRegistry helper and migrating command registrations in src/extension.ts to a more declarative style, reducing repeated boilerplate around registerCommand, ensureSignedIn, and “refresh if updated” patterns.
Changes:
- Add
CommandRegistry(add,addGuarded,addRefreshing) to centralize common command wrapper patterns. - Migrate command registrations in
src/extension.tsto useCommandRegistry, reducing repeatedcontext.subscriptions.push(registerCommand(...))and guard/refresh wrappers. - Register all command disposables at the end via
registry.registerAll(context).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/extension.ts | Migrates command registrations to CommandRegistry and centralizes registration/disposal. |
| src/commands/commandRegistry.ts | Introduces CommandRegistry with helpers for plain/guarded/refreshing command shapes. |
| workItemProvider.refresh(); | ||
| }) | ||
| ); | ||
| registry.addGuarded('adoext.refreshWorkItems', () => { |
| if (!choice || choice.value === current) { | ||
| return; | ||
| } | ||
| registry.addGuarded('adoext.refreshBacklog', () => { |
| boardProvider.refresh(); | ||
| }) | ||
| ); | ||
| registry.addGuarded('adoext.refreshSprints', () => { |
| await PlanningPanel.show(context, 'backlog', client, config, refreshAllViews); | ||
| }) | ||
| ); | ||
| registry.addGuarded('adoext.refreshBoards', () => { |
| pullRequestProvider.refresh(); | ||
| }) | ||
| ); | ||
| registry.addGuarded('adoext.refreshPullRequests', () => { |
| pipelinesProvider.refresh(); | ||
| }) | ||
| ); | ||
| registry.addGuarded('adoext.refreshPipelines', () => { |
Pre-refactor behavior was that 'adoext.refresh*' commands always invoked the provider's refresh() regardless of sign-in state, which let the tree transition into its setup / signed-out node. After moving them to addGuarded, refresh() was suppressed on sign-in failure and the tree kept whatever stale content it had. Reviewer flagged six commands: refreshWorkItems, refreshBacklog, refreshSprints, refreshBoards, refreshPullRequests, refreshPipelines. Add CommandRegistry.addRefresh() for the 'fire-and-forget view refresh' shape and switch all six over.
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.
Changes
New:
src/commands/commandRegistry.tsCommandRegistryclass with three helpers:add(id, handler)— plain command.addGuarded(id, handler)— runsensureSignedInfirst; aborts silently if not signed in.addRefreshing(id, handler, refresh)— guarded; callsrefresh()when handler returns truthy.Refactored
src/extension.tsAll 64 command registrations migrated. Removes the
context.subscriptions.push(vscode.commands.registerCommand(...))boilerplate and the repetitiveif (!(await ensureSignedIn())) { return; }/if (updated) refresh()blocks.Impact
extension.ts: 1186 → 930 lines (-256 lines in the file, -196 net including the new registry).Verification
npm run compile— both tsc passes + esbuild bundles green.npm run lint— 0 warnings, 0 errors.Notes
signInwith optional arg,setWorkItemFilterwith input box flow,addPullRequestCommentwith try/catch) stay on plainadd()since their bodies don't match the guarded/refreshing shape; the win there is just removing the wrapper boilerplate.mcpManageris still declared aftersignOutis registered — preserved by JS closure capture, same as before this refactor.