Skip to content

Commit 661aad7

Browse files
committed
Add formatWarning output helper and adopt across codebase
Replace ad-hoc chalk.yellow(...) warning patterns with a consistent formatWarning() helper from src/utils/output.ts. Adopted in base commands, connections test, interactive mode, and command-not-found hook.
1 parent 0d30bc9 commit 661aad7

File tree

12 files changed

+41
-27
lines changed

12 files changed

+41
-27
lines changed

.claude/skills/ably-new-command/SKILL.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ if (this.shouldOutputJson(flags)) {
189189
|--------|-------|---------|
190190
| `formatProgress(msg)` | Action in progress — appends `...` automatically | `formatProgress("Attaching to channel")` |
191191
| `formatSuccess(msg)` | Green checkmarkalways end with `.` (period, not `!`) | `formatSuccess("Subscribed to channel " + formatResource(name) + ".")` |
192+
| `formatWarning(msg)` | Yellow ``for non-fatal warnings. Don't prefix with "Warning:" | `formatWarning("Persistence is automatically enabled.")` |
192193
| `formatListening(msg)` | Dim textauto-appends "Press Ctrl+C to exit." | `formatListening("Listening for messages.")` |
193194
| `formatResource(name)` | Cyanfor resource names, never use quotes | `formatResource(channelName)` |
194195
| `formatTimestamp(ts)` | Dim `[timestamp]`for event streams | `formatTimestamp(isoString)` |
@@ -370,7 +371,7 @@ pnpm test:unit # Run tests
370371
- [ ] Correct flag set (`productApiFlags` vs `ControlBaseCommand.globalFlags`)
371372
- [ ] `clientIdFlag` only if command needs client identity
372373
- [ ] All human output wrapped in `if (!this.shouldOutputJson(flags))`
373-
- [ ] Output helpers used correctly (`formatProgress`, `formatSuccess`, `formatListening`, `formatResource`, `formatTimestamp`, `formatClientId`, `formatEventType`, `formatLabel`, `formatHeading`, `formatIndex`)
374+
- [ ] Output helpers used correctly (`formatProgress`, `formatSuccess`, `formatWarning`, `formatListening`, `formatResource`, `formatTimestamp`, `formatClientId`, `formatEventType`, `formatLabel`, `formatHeading`, `formatIndex`)
374375
- [ ] `success()` messages end with `.` (period)
375376
- [ ] Resource names use `resource(name)`, never quoted
376377
- [ ] JSON output uses `logJsonResult()` (one-shot) or `logJsonEvent()` (streaming), not direct `formatJsonRecord()`

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ All output helpers use the `format` prefix and are exported from `src/utils/outp
186186

187187
- **Progress**: `formatProgress("Attaching to channel: " + formatResource(name))`no color on action text, appends `...` automatically. Never manually write `"Doing something..."`always use `formatProgress("Doing something")`.
188188
- **Success**: `formatSuccess("Message published to channel " + formatResource(name) + ".")`green checkmark, **must** end with `.` (not `!`). Never use `chalk.green(...)` directlyalways use `formatSuccess()`.
189+
- **Warnings**: `formatWarning("Message text here.")`yellow `` symbol. Never use `chalk.yellow("Warning: ...")` directlyalways use `formatWarning()`. Don't include "Warning:" prefix in the message — the symbol conveys it.
189190
- **Listening**: `formatListening("Listening for messages.")`dim, includes "Press Ctrl+C to exit." Don't combine listening text inside a `formatSuccess()` call — use a separate `formatListening()` call.
190191
- **Resource names**: Always `formatResource(name)` (cyan), never quotedincluding in `logCliEvent` messages.
191192
- **Timestamps**: `formatTimestamp(ts)`dim `[timestamp]` for event streams. `formatMessageTimestamp(message.timestamp)`converts Ably message timestamp (number|undefined) to ISO string.
@@ -229,7 +230,6 @@ this.error() ← oclif exit (ONLY inside fail, nowhere else)
229230
- **`runControlCommand<T>`** returns `Promise<T>` (not nullable) — calls `this.fail()` internally on error.
230231
231232
### Additional output patterns (direct chalk, not helpers)
232-
- **Warnings**: `chalk.yellow("Warning: ...")` — for non-fatal warnings
233233
- **No app error**: `'No app specified. Use --app flag or select an app with "ably apps switch"'`
234234
235235
### Help output theme

src/base-command.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { CommandError } from "./errors/command-error.js";
1313
import { coreGlobalFlags } from "./flags.js";
1414
import { InteractiveHelper } from "./services/interactive-helper.js";
1515
import { BaseFlags, CommandConfig } from "./types/cli.js";
16-
import { buildJsonRecord } from "./utils/output.js";
16+
import { buildJsonRecord, formatWarning } from "./utils/output.js";
1717
import { getCliVersion } from "./utils/version.js";
1818
import Spaces from "@ably/spaces";
1919
import { ChatClient } from "@ably/chat";
@@ -1265,7 +1265,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand {
12651265
// Log timeout only if not in JSON mode
12661266
if (!this.shouldOutputJson({})) {
12671267
// TODO: Pass actual flags here
1268-
this.log(chalk.yellow("Cleanup operation timed out."));
1268+
this.log(formatWarning("Cleanup operation timed out."));
12691269
}
12701270
reject(new Error("Cleanup timed out")); // Reject promise on timeout
12711271
}, effectiveTimeout);
@@ -1352,7 +1352,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand {
13521352
break;
13531353
}
13541354
case "disconnected": {
1355-
this.log(chalk.yellow("! Disconnected from Ably"));
1355+
this.log(formatWarning("Disconnected from Ably"));
13561356
break;
13571357
}
13581358
case "failed": {
@@ -1364,7 +1364,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand {
13641364
break;
13651365
}
13661366
case "suspended": {
1367-
this.log(chalk.yellow("! Connection suspended"));
1367+
this.log(formatWarning("Connection suspended"));
13681368
break;
13691369
}
13701370
case "connecting": {

src/base-topic-command.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import inquirer from "inquirer";
33
import pkg from "fast-levenshtein";
44
import { InteractiveBaseCommand } from "./interactive-base-command.js";
55
import { runInquirerWithReadlineRestore } from "./utils/readline-helper.js";
6+
import { formatWarning } from "./utils/output.js";
67
import * as readline from "node:readline";
78
import {
89
WEB_CLI_RESTRICTED_COMMANDS,
@@ -104,7 +105,7 @@ export abstract class BaseTopicCommand extends InteractiveBaseCommand {
104105
// In interactive mode, we need to ensure the message is visible
105106
// Write directly to stderr to avoid readline interference
106107
if (isInteractiveMode) {
107-
process.stderr.write(chalk.yellow(`Warning: ${warningMessage}\n`));
108+
process.stderr.write(`${formatWarning(warningMessage)}\n`);
108109
} else {
109110
this.warn(warningMessage);
110111
}

src/chat-base-command.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { productApiFlags } from "./flags.js";
55
import { BaseFlags } from "./types/cli.js";
66
import chalk from "chalk";
77

8-
import { formatSuccess, formatListening } from "./utils/output.js";
8+
import {
9+
formatSuccess,
10+
formatListening,
11+
formatWarning,
12+
} from "./utils/output.js";
913
import isTestMode from "./utils/test-mode.js";
1014

1115
export abstract class ChatBaseCommand extends AblyBaseCommand {
@@ -128,7 +132,7 @@ export abstract class ChatBaseCommand extends AblyBaseCommand {
128132
}
129133
case RoomStatus.Detached: {
130134
if (!this.shouldOutputJson(flags)) {
131-
this.log(chalk.yellow("Disconnected from Ably"));
135+
this.log(formatWarning("Disconnected from Ably"));
132136
}
133137
break;
134138
}

src/commands/connections/test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
formatProgress,
99
formatResource,
1010
formatSuccess,
11+
formatWarning,
1112
} from "../../utils/output.js";
1213

1314
export default class ConnectionsTest extends AblyBaseCommand {
@@ -172,7 +173,9 @@ export default class ConnectionsTest extends AblyBaseCommand {
172173
);
173174
} else if (partialSuccess) {
174175
this.log(
175-
`${chalk.yellow("!")} Some connection tests succeeded, but others failed`,
176+
formatWarning(
177+
"Some connection tests succeeded, but others failed",
178+
),
176179
);
177180
} else {
178181
this.log(`${chalk.red("✗")} All connection tests failed`);

src/commands/interactive.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
INTERACTIVE_UNSUITABLE_COMMANDS,
1414
} from "../base-command.js";
1515
import { TerminalDiagnostics } from "../utils/terminal-diagnostics.js";
16+
import { formatWarning } from "../utils/output.js";
1617
import "../utils/sigint-exit.js";
1718
import isWebCliMode from "../utils/web-mode.js";
1819

@@ -697,9 +698,7 @@ export default class Interactive extends Command {
697698
// Warn about unclosed quotes
698699
if (inDoubleQuote || inSingleQuote) {
699700
const quoteType = inDoubleQuote ? "double" : "single";
700-
console.error(
701-
chalk.yellow(`Warning: Unclosed ${quoteType} quote in command`),
702-
);
701+
console.error(formatWarning(`Unclosed ${quoteType} quote in command`));
703702
}
704703

705704
return args;

src/hooks/command_not_found/did-you-mean.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import chalk from "chalk";
33
import inquirer from "inquirer";
44
import pkg from "fast-levenshtein";
55
import { runInquirerWithReadlineRestore } from "../../utils/readline-helper.js";
6+
import { formatWarning } from "../../utils/output.js";
67
import * as readline from "node:readline";
78
const { get: levenshteinDistance } = pkg;
89

@@ -94,7 +95,7 @@ const hook: Hook<"command_not_found"> = async function (opts) {
9495
// Warn about command not found and suggest alternative with colored command names
9596
const warningMessage = `${chalk.cyan(displayOriginal.replaceAll(":", " "))} is not an ably command.`;
9697
if (isInteractiveMode) {
97-
console.log(chalk.yellow(`Warning: ${warningMessage}`));
98+
console.log(formatWarning(warningMessage));
9899
} else {
99100
this.warn(warningMessage);
100101
}

src/hooks/command_not_found/prompt-utils.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,6 @@ export function formatPromptMessage(
5555
// This file now only contains utility functions if needed,
5656
// or can be removed if formatPromptMessage is moved/inlined.
5757

58-
// Example of how you might use chalk for other styling if needed:
59-
// export function formatError(text: string): string {
60-
// return chalk.red(text);
61-
// }
62-
63-
// export function formatWarning(text: string): string {
64-
// return chalk.yellow(text);
65-
// }
66-
67-
// Example utility function using chalk (can be adapted or removed)
6858
export function formatSuggestion(suggestion: string): string {
6959
return chalk.blueBright(suggestion);
7060
}
71-
72-
// You can add other prompt-related utility functions here if needed.

src/utils/output.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ export function formatSuccess(message: string): string {
88
return `${chalk.green("✓")} ${message}`;
99
}
1010

11+
export function formatWarning(message: string): string {
12+
return `${chalk.yellow("⚠")} ${message}`;
13+
}
14+
1115
export function formatListening(description: string): string {
1216
return chalk.dim(`${description} Press Ctrl+C to exit.`);
1317
}

0 commit comments

Comments
 (0)