feat(client): add limit configurable in getSpaces and spaces command#163
Conversation
Previously the spaces API call had a hardcoded limit of 500. This change allows callers to request more than 500 spaces by passing an explicit limit to getSpaces(), and exposes it as a --limit option on the `confluence spaces` CLI command. - getSpaces(limit = 500): limit is now a parameter (default 500) - `confluence spaces --limit <n>`: new CLI option, defaults to 500 - Add tests verifying the limit param is forwarded to the API request
0c0bdf9 to
b071307
Compare
| } | ||
|
|
||
| /** | ||
| * Get all spaces |
There was a problem hiding this comment.
Removed "all" since the command does not retrieve all spaces by default.
| // List spaces command | ||
| program | ||
| .command('spaces') | ||
| .description('List all Confluence spaces') |
There was a problem hiding this comment.
Removed "all" since the command does not retrieve all spaces by default.
The spaces command fetches up to --limit results (default 500), so describing it as "all spaces" is inaccurate. Replace with "spaces" or "available spaces" throughout docs and examples.
pchuri
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Clean, focused change that addresses #166 well, and the second commit polishing the "all" wording across docs/examples/SKILL.md after self-review is much appreciated.
LGTM — approving. A couple of optional nits that could be addressed here or in a follow-up:
-
Input validation —
parseInt(options.limit)inbin/confluence.jswill produceNaNfor--limit abcand accepts non-positive values. Theattachments,properties, andcommentscommands already use this pattern for consistency:const limit = parseInt(options.limit, 10); if (Number.isNaN(limit) || limit <= 0) { throw new Error('Limit must be a positive number.'); }
-
parseIntradix — minor, but newer commands in this file pass10explicitly (parseInt(x, 10)). -
(Out of scope, just flagging) The Confluence
/spaceendpoint may cap results server-side, so--limit 1000may not always return 1000 entries in a single call. True "fetch more than the server cap" support would need pagination viastart(similar togetAllProperties). Worth a follow-up issue if users hit this.
None of these are blockers — the PR is a clear improvement as-is. Thanks again!
|
🎉 This PR is included in version 2.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Thanks for the review and feedback! I really appreciate it :) |
Description
Previously the spaces API call had a hardcoded limit of 500.
This change allows callers to request more than 500 spaces by passing an explicit limit to getSpaces(),
and exposes it as a --limit option on the
confluence spacesCLI command.confluence spaces --limit <n>: new CLI option, defaults to 500Type of Change
Testing
Checklist
closes #166