refactor(cli): extract withClient helper to remove command boilerplate#181
Merged
Conversation
Each of the 26 client-using commands repeated the same Analytics +
getConfig + assertWritable + ConfluenceClient + try/handleCommandError
scaffolding. Extracted into a single withClient(name, handler, { writable })
helper so handlers focus on command logic. Tracking semantics are
preserved (handlers still call analytics.track on success so cancel/
dry-run/conditional keys keep working).
bin/confluence.js: 940 insertions, 1128 deletions.
4 tasks
pchuri
added a commit
that referenced
this pull request
May 11, 2026
* refactor(cli): migrate comment command to withClient with onError hook The comment command was the only client-using command left with manual analytics + getConfig + assertWritable + ConfluenceClient + try/catch scaffolding after #181, because its catch block needed to print API response details and inline-comment-specific hints. Add an `onError(error, ...actionArgs)` option to withClient that runs between the standard `Error:` log line and process.exit, and extend handleCommandError with a matching `onExtra` callback. The hook is wrapped in try/catch so a throwing hint path cannot break the error flow. Move the comment command's API-response dump and inline-meta hint into the hook. Existing behavior (output ordering, exit code, failure tracking) is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(cli): add unit tests for withClient wrapper The withClient helper sits on the shared error/analytics/client path for every CLI command that talks to the server but had no direct test coverage. Add six cases covering: context injection ({ client, config, analytics } + forwarded action args), the writable-on-readOnly exit path, failure tracking on handler throw, the new onError hook (invocation order before process.exit, action-arg forwarding, throw isolation), and getConfig failure being treated as a tracked failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 2.6.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
1 participant
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.
Summary
withClient(name, handler, { writable })helper that wraps the standardAnalytics+getConfig+assertWritable+ConfluenceClient+try/handleCommandErrorscaffolding.bin/confluence.js: 940 insertions, 1128 deletions (net −188 lines, 2249 → 2061).Why
Every client-using command had the same ~6-line pre/post-amble. Extracting it shortens the file, makes each command's intent obvious, and centralizes the read-only profile guard so future commands can opt in via
{ writable: true }instead of re-implementing the gate.Behavior preservation
analytics.track(name, true)themselves, so conditional / cancel / dry-run tracking keys (*_cancel,copy_tree_dry_run,versions_purgewithresult.failed === 0) are unchanged.console.log, thrown error messages) are identical — diff only differs by indentation.comment <pageId>is intentionally not migrated: it has a custom error path that prints inline-comment-specific hints based on the API error keys.init,stats,install-skill,profile add/remove,convertare not migrated either: they don't construct aConfluenceClient.Test plan
npm test— 670/670 tests pass (incl. existing CLI integration tests incli-entry.test.js,metadata-cli.test.js,export.test.jsthat exercise the migratedread,info,childrenpaths and the error →handleCommandError→ exit-1 path)npm run lint— cleannode bin/index.js --help— all commands registered and visiblenode -c