Skip to content

Commit 8a387cc

Browse files
umair-ablyumair-ably
committed
Address PR #170 review feedback
1 parent ad3105f commit 8a387cc

24 files changed

Lines changed: 210 additions & 104 deletions

File tree

‎.claude/skills/ably-new-command/references/patterns.md‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -419,9 +419,9 @@ async run(): Promise<void> {
419419
}
420420
```
421421
422-
**Product API list with pagination** (e.g., `push devices list`, `channels list`) — use `collectPaginatedResults` or `collectHttpPaginatedResults`:
422+
**Product API list with pagination** (e.g., `push devices list`, `channels list`) — use `collectPaginatedResults`:
423423
```typescript
424-
import { collectPaginatedResults, collectHttpPaginatedResults, formatPaginationWarning } from "../../utils/pagination.js";
424+
import { buildPaginationNext, collectPaginatedResults, formatPaginationWarning } from "../../utils/pagination.js";
425425

426426
async run(): Promise<void> {
427427
const { flags } = await this.parse(MyListCommand);
@@ -430,21 +430,17 @@ async run(): Promise<void> {
430430
const rest = await this.createAblyRestClient(flags);
431431
if (!rest) return;
432432

433-
// For SDK methods that return PaginatedResult:
434433
const firstPage = await rest.someResource.list({ limit: flags.limit });
435434
const { items, hasMore, pagesConsumed } = await collectPaginatedResults(firstPage, flags.limit);
436435

437-
// For rest.request() that returns HttpPaginatedResponse:
438-
// const firstPage = await rest.request("get", "/some/endpoint", 2, { limit: String(flags.limit) });
439-
// const { items, hasMore, pagesConsumed } = await collectHttpPaginatedResults(firstPage, flags.limit);
440-
441436
const paginationWarning = formatPaginationWarning(pagesConsumed, items.length);
442437
if (paginationWarning && !this.shouldOutputJson(flags)) {
443438
this.log(paginationWarning);
444439
}
445440

446441
if (this.shouldOutputJson(flags)) {
447-
this.logJsonResult({ items, hasMore }, flags);
442+
const next = buildPaginationNext(hasMore);
443+
this.logJsonResult({ items, hasMore, ...(next && { next }) }, flags);
448444
} else {
449445
this.log(`Found ${items.length} items:\n`);
450446
for (const item of items) {
@@ -470,8 +466,8 @@ Key conventions for list output:
470466
- `formatLabel(text)` for field labels in detail lines (automatically appends `:`)
471467
- `formatSuccess()` is not used in list commands — it's for confirming an action completed
472468
- `formatLimitWarning()` should only be shown when `hasMore` is true — it means there are more results beyond the limit
473-
- Always include `hasMore` in JSON output for paginated commands so consumers know if results are truncated
474-
- Use `collectPaginatedResults()` for SDK paginated results, `collectHttpPaginatedResults()` for `rest.request()` results, and `collectFilteredPaginatedResults()` when a client-side filter is applied across pages
469+
- Always include `hasMore` and `next` in JSON output for paginated commands. `next` provides continuation hints (and `start` timestamp for history commands)
470+
- Use `collectPaginatedResults()` for SDK paginated results and `collectFilteredPaginatedResults()` when a client-side filter is applied across pages
475471
476472
---
477473

‎docs/Project-Structure.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ This document outlines the directory structure of the Ably CLI project.
8888
│ ├── history.ts # History query parameter builder
8989
│ ├── message.ts # Message interpolation ({{.Count}}, {{.Timestamp}})
9090
│ ├── output.ts # Output helpers (progress, success, resource, etc.)
91+
│ ├── pagination.ts # Generic pagination utilities (collectPaginatedResults, collectFilteredPaginatedResults)
9192
│ ├── prompt-confirmation.ts # Y/N confirmation prompts
9293
│ ├── readline-helper.ts # Readline utilities for interactive mode
9394
│ ├── sigint-exit.ts # SIGINT/Ctrl+C handling (exit code 130)

‎src/commands/apps/rules/list.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export default class RulesListCommand extends ControlBaseCommand {
120120
flags.limit,
121121
"channel rules",
122122
);
123-
if (warning) this.logToStderr(warning);
123+
if (warning) this.log(warning);
124124
}
125125
}
126126
} catch (error) {

‎src/commands/auth/keys/list.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export default class KeysListCommand extends ControlBaseCommand {
111111

112112
if (hasMore) {
113113
const warning = formatLimitWarning(keys.length, flags.limit, "keys");
114-
if (warning) this.logToStderr(warning);
114+
if (warning) this.log(warning);
115115
}
116116
}
117117
} catch (error) {

‎src/commands/channels/history.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from "../../utils/output.js";
1616
import type { MessageDisplayFields } from "../../utils/output.js";
1717
import {
18+
buildPaginationNext,
1819
collectPaginatedResults,
1920
formatPaginationWarning,
2021
} from "../../utils/pagination.js";
@@ -98,12 +99,15 @@ export default class ChannelsHistory extends AblyBaseCommand {
9899
messages.length,
99100
);
100101
if (paginationWarning && !this.shouldOutputJson(flags)) {
101-
this.logToStderr(paginationWarning);
102+
this.log(paginationWarning);
102103
}
103104

104105
// Display results based on format
105106
if (this.shouldOutputJson(flags)) {
106-
this.logJsonResult({ messages, hasMore }, flags);
107+
const lastTimestamp =
108+
messages.length > 0 ? messages.at(-1)!.timestamp : undefined;
109+
const next = buildPaginationNext(hasMore, lastTimestamp);
110+
this.logJsonResult({ messages, hasMore, ...(next && { next }) }, flags);
107111
} else {
108112
if (messages.length === 0) {
109113
this.log("No messages found in the channel history.");
@@ -145,7 +149,7 @@ export default class ChannelsHistory extends AblyBaseCommand {
145149
flags.limit,
146150
"messages",
147151
);
148-
if (warning) this.logToStderr(warning);
152+
if (warning) this.log(warning);
149153
}
150154
}
151155
} catch (error) {

‎src/commands/channels/list.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import {
88
formatResource,
99
} from "../../utils/output.js";
1010
import {
11-
collectHttpPaginatedResults,
11+
buildPaginationNext,
12+
collectPaginatedResults,
1213
formatPaginationWarning,
1314
} from "../../utils/pagination.js";
1415

@@ -101,7 +102,7 @@ export default class ChannelsList extends AblyBaseCommand {
101102
items: channels,
102103
hasMore,
103104
pagesConsumed,
104-
} = await collectHttpPaginatedResults<ChannelItem>(
105+
} = await collectPaginatedResults<ChannelItem>(
105106
channelsResponse,
106107
flags.limit,
107108
);
@@ -116,13 +117,15 @@ export default class ChannelsList extends AblyBaseCommand {
116117

117118
// Output channels based on format
118119
if (this.shouldOutputJson(flags)) {
120+
const next = buildPaginationNext(hasMore);
119121
this.logJsonResult(
120122
{
121123
channels: channels.map((channel: ChannelItem) => ({
122124
channelId: channel.channelId,
123125
metrics: channel.status?.occupancy?.metrics || {},
124126
})),
125127
hasMore,
128+
...(next && { next }),
126129
timestamp: new Date().toISOString(),
127130
total: channels.length,
128131
},

‎src/commands/logs/connection-lifecycle/history.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
formatLimitWarning,
1414
} from "../../../utils/output.js";
1515
import {
16+
buildPaginationNext,
1617
collectPaginatedResults,
1718
formatPaginationWarning,
1819
} from "../../../utils/pagination.js";
@@ -78,6 +79,9 @@ export default class LogsConnectionLifecycleHistory extends AblyBaseCommand {
7879

7980
// Output results based on format
8081
if (this.shouldOutputJson(flags)) {
82+
const lastTimestamp =
83+
messages.length > 0 ? messages.at(-1)!.timestamp : undefined;
84+
const next = buildPaginationNext(hasMore, lastTimestamp);
8185
this.logJsonResult(
8286
{
8387
hasMore,
@@ -90,6 +94,7 @@ export default class LogsConnectionLifecycleHistory extends AblyBaseCommand {
9094
name: msg.name,
9195
timestamp: formatMessageTimestamp(msg.timestamp),
9296
})),
97+
...(next && { next }),
9398
},
9499
flags,
95100
);

‎src/commands/logs/history.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
formatLimitWarning,
1414
} from "../../utils/output.js";
1515
import {
16+
buildPaginationNext,
1617
collectPaginatedResults,
1718
formatPaginationWarning,
1819
} from "../../utils/pagination.js";
@@ -78,6 +79,9 @@ export default class LogsHistory extends AblyBaseCommand {
7879

7980
// Output results based on format
8081
if (this.shouldOutputJson(flags)) {
82+
const lastTimestamp =
83+
messages.length > 0 ? messages.at(-1)!.timestamp : undefined;
84+
const next = buildPaginationNext(hasMore, lastTimestamp);
8185
this.logJsonResult(
8286
{
8387
hasMore,
@@ -90,6 +94,7 @@ export default class LogsHistory extends AblyBaseCommand {
9094
name: msg.name,
9195
timestamp: formatMessageTimestamp(msg.timestamp),
9296
})),
97+
...(next && { next }),
9398
},
9499
flags,
95100
);

‎src/commands/logs/push/history.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
formatLabel,
1616
} from "../../../utils/output.js";
1717
import {
18+
buildPaginationNext,
1819
collectPaginatedResults,
1920
formatPaginationWarning,
2021
} from "../../../utils/pagination.js";
@@ -79,6 +80,9 @@ export default class LogsPushHistory extends AblyBaseCommand {
7980

8081
// Output results based on format
8182
if (this.shouldOutputJson(flags)) {
83+
const lastTimestamp =
84+
messages.length > 0 ? messages.at(-1)!.timestamp : undefined;
85+
const next = buildPaginationNext(hasMore, lastTimestamp);
8286
this.logJsonResult(
8387
{
8488
hasMore,
@@ -92,6 +96,7 @@ export default class LogsPushHistory extends AblyBaseCommand {
9296
name: msg.name,
9397
timestamp: formatMessageTimestamp(msg.timestamp),
9498
})),
99+
...(next && { next }),
95100
},
96101
flags,
97102
);

‎src/commands/push/channels/list-channels.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
formatSuccess,
1212
} from "../../../utils/output.js";
1313
import {
14+
buildPaginationNext,
1415
collectPaginatedResults,
1516
formatPaginationWarning,
1617
} from "../../../utils/pagination.js";
@@ -57,11 +58,12 @@ export default class PushChannelsListChannels extends AblyBaseCommand {
5758
channels.length,
5859
);
5960
if (paginationWarning && !this.shouldOutputJson(flags)) {
60-
this.logToStderr(paginationWarning);
61+
this.log(paginationWarning);
6162
}
6263

6364
if (this.shouldOutputJson(flags)) {
64-
this.logJsonResult({ channels, hasMore }, flags);
65+
const next = buildPaginationNext(hasMore);
66+
this.logJsonResult({ channels, hasMore, ...(next && { next }) }, flags);
6567
return;
6668
}
6769

@@ -86,7 +88,7 @@ export default class PushChannelsListChannels extends AblyBaseCommand {
8688
flags.limit,
8789
"channels",
8890
);
89-
if (limitWarning) this.logToStderr(limitWarning);
91+
if (limitWarning) this.log(limitWarning);
9092
}
9193
} catch (error) {
9294
this.fail(error, flags as BaseFlags, "pushChannelListChannels");

0 commit comments

Comments
 (0)