From 7f7a4af7c48b736fb678d83047ab76af7ca7983a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Fri, 30 Oct 2026 05:35:50 +0000 Subject: [PATCH] fix(cli): add takes several names in one call, and adopted audio is labelled voice or sfx (#6791) * fix(cli): adopted audio is labelled voice or sfx by its folder or file name, not always bgm * fix(cli): let the audio kind helper's name carry the rule * feat(cli): add installs several named items in one call * fix(cli): split add's per-item install, and keep soundfx folders as sfx --- packages/cli/src/cli.commands.test.ts | 1 + packages/cli/src/commands/add.test.ts | 46 ++++--- packages/cli/src/commands/add.ts | 117 +++++++++++++----- packages/cli/src/media-use/lib/adopt.mjs | 21 ++-- packages/cli/src/media-use/lib/adopt.test.mjs | 22 ++++ .../cli/src/utils/reject-extra-positionals.ts | 8 +- 6 files changed, 152 insertions(+), 63 deletions(-) diff --git a/packages/cli/src/cli.commands.test.ts b/packages/cli/src/cli.commands.test.ts index f14bc1c5b0..2676bc1955 100644 --- a/packages/cli/src/cli.commands.test.ts +++ b/packages/cli/src/cli.commands.test.ts @@ -19,6 +19,7 @@ function commandLoaderBlock(): string { // Vite cannot resolve a template-string import, so each listed top-level command is named here. const LIST_COMMANDS: Record Promise<{ default: unknown }>> = { + add: () => import("./commands/add.js"), audio: () => import("./commands/audio.js"), catalog: () => import("./commands/catalog.js"), compare: () => import("./commands/compare.js"), diff --git a/packages/cli/src/commands/add.test.ts b/packages/cli/src/commands/add.test.ts index 4df3966279..2cc3a3e873 100644 --- a/packages/cli/src/commands/add.test.ts +++ b/packages/cli/src/commands/add.test.ts @@ -17,7 +17,6 @@ import addCommand, { tagAddJson, } from "./add.js"; import { trackRegistryItemAdded } from "../telemetry/events.js"; -import { CliUsageError } from "../utils/commandResult.js"; import { trackCommandFailures } from "../utils/command-failure-tracking.js"; // Assert the emitted payload rather than the transport: `shouldTrack()` is @@ -525,17 +524,17 @@ describe("variable values in the snippet", () => { }); }); -describe("add command run() — extra positional arguments", () => { +describe("add command run() — several names", () => { let dir: string; - let errorSpy: ReturnType; beforeEach(() => { dir = tmp(); - errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeRegistryConfig(dir); + mockFetch(); }); afterEach(() => { - errorSpy.mockRestore(); + vi.restoreAllMocks(); vi.unstubAllGlobals(); rmSync(dir, { recursive: true, force: true }); }); @@ -545,22 +544,35 @@ describe("add command run() — extra positional arguments", () => { await runCittyCommand(cmd, { rawArgs }); } - it("rejects `add a --dir b c` through the CLI wrapper, before any registry call", async () => { - const fetchSpy = vi.fn(); - vi.stubGlobal("fetch", fetchSpy); + it("installs every name in one call and reports them together", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => {}); - await expect(runWrapped(["a", "--dir", dir, "b", "c"])).rejects.toThrow(CliUsageError); - expect(fetchSpy).not.toHaveBeenCalled(); - expect(errorSpy.mock.calls.flat().join(" ")).toContain( - "Unexpected extra arguments for hyperframes add: b, c", - ); - expect(errorSpy.mock.calls.flat().join(" ")).toContain("Run add once per item"); + await runWrapped(["my-block", "--dir", dir, "deprecated-block", "--no-clipboard", "--json"]); + + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ + ok: true, + installed: ["my-block", "deprecated-block"], + warnings: expect.any(Array), + errors: [], + }); + expect(existsSync(join(dir, "compositions", "my-block.html"))).toBe(true); + expect(existsSync(join(dir, "compositions", "deprecated-block.html"))).toBe(true); }); - it("does not fire on a normal single-item invocation", async () => { - mockFetch(); - writeRegistryConfig(dir); + it("keeps the installs that worked when one name is unknown, and fails the call", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + await expect( + runWrapped(["my-block", "no-such-block", "--dir", dir, "--no-clipboard", "--json"]), + ).rejects.toMatchObject({ result: { exitCode: 1 } }); + + const out = JSON.parse(String(log.mock.calls[0]?.[0])); + expect(out).toMatchObject({ ok: false, installed: ["my-block"] }); + expect(out.errors).toEqual([{ name: "no-such-block", error: expect.any(String) }]); + expect(existsSync(join(dir, "compositions", "my-block.html"))).toBe(true); + }); + + it("still installs a single name the way it did", async () => { await expect( runWrapped(["my-block", "--dir", dir, "--no-clipboard", "--json"]), ).resolves.toBeUndefined(); diff --git a/packages/cli/src/commands/add.ts b/packages/cli/src/commands/add.ts index 84a698620e..8ca51d9277 100644 --- a/packages/cli/src/commands/add.ts +++ b/packages/cli/src/commands/add.ts @@ -148,19 +148,70 @@ export function buildSnippet( return ""; } +function installJson(results: RunAddResult[], errors: Array<{ name: string; error: string }>) { + return { + ok: errors.length === 0, + installed: results.map((r) => r.name), + warnings: results.flatMap((r) => r.warnings.map((w) => `${r.name}: ${w}`)), + errors, + }; +} + /** `add --json` output: successful installs and each item's warnings or failure. */ export function tagAddJson( tag: string, results: RunAddResult[], errors: Array<{ name: string; error: string }> = [], ) { - return { - ok: errors.length === 0, - tag, - installed: results.map((r) => r.name), - warnings: results.flatMap((r) => r.warnings.map((w) => `${r.name}: ${w}`)), - errors, - }; + return { ...installJson(results, errors), tag }; +} + +export function severalAddJson( + results: RunAddResult[], + errors: Array<{ name: string; error: string }> = [], +) { + return installJson(results, errors); +} + +interface InstallContext { + projectDir: string; + force: boolean | undefined; + vars: string | undefined; + json: boolean; +} + +type InstallOutcome = { result: RunAddResult } | { name: string; error: string }; + +async function installOne(name: string, context: InstallContext): Promise { + try { + const result = await runAdd({ + name, + projectDir: context.projectDir, + skipClipboard: true, + force: context.force, + vars: context.vars, + }); + if (!context.json) { + for (const warning of result.warnings) console.log(` ${c.warn("Warning:")} ${warning}`); + console.log(` ${c.success("✓")} ${result.name}`); + } + return { result }; + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + if (!context.json) console.log(` ${c.error("✗")} ${name} (skipped: ${reason})`); + return { name, error: reason }; + } +} + +async function installEach(names: readonly string[], context: InstallContext) { + const results: RunAddResult[] = []; + const errors: Array<{ name: string; error: string }> = []; + for (const name of names) { + const outcome = await installOne(name, context); + if ("result" in outcome) results.push(outcome.result); + else errors.push(outcome); + } + return { results, errors }; } /** `--vars` is JSON, a file or `-`; a malformed one is an error, not a snippet that drops the values. */ @@ -458,7 +509,8 @@ export default defineCommand({ type: "positional", description: "Registry item name or tag. Single items install directly (e.g. shader-wipe). " + - "If the name matches a tag instead, all blocks with that tag are installed (e.g. html-in-canvas, captions).", + "If the name matches a tag instead, all blocks with that tag are installed (e.g. html-in-canvas, captions). " + + "Give several names to install each of them in one call.", required: true, }, dir: { @@ -502,6 +554,29 @@ export default defineCommand({ const hasConfigBefore = existsSync(projectConfigPath(projectDir)); + const names = Array.isArray(args._) && args._.length > 1 ? args._.map(String) : [args.name]; + if (names.length > 1) { + if (!json) { + console.log(""); + console.log(`${c.accent("◆")} Installing ${c.accent(String(names.length))} items`); + } + const { results, errors } = await installEach(names, { + projectDir, + force: args.force, + vars: args.vars, + json, + }); + if (json) { + console.log(JSON.stringify(severalAddJson(results, errors))); + } else { + console.log(""); + const summary = `Installed ${results.length}/${names.length} items`; + console.log(`${errors.length > 0 ? c.error("✗") : c.success("✓")} ${summary}`); + } + if (errors.length > 0) failCommand(); + return; + } + // Try single item first. If it fails, check if the name matches a tag. try { const result = await runAdd({ @@ -586,28 +661,10 @@ export default defineCommand({ ); } - const results: RunAddResult[] = []; - const errors: Array<{ name: string; error: string }> = []; - for (const item of items) { - try { - const result = await runAdd({ - name: item.name, - projectDir, - skipClipboard: true, - force: args.force, - vars: args.vars, - }); - results.push(result); - for (const warning of result.warnings) { - if (!json) console.log(` ${c.warn("Warning:")} ${warning}`); - } - if (!json) console.log(` ${c.success("✓")} ${result.name}`); - } catch (err) { - const reason = err instanceof Error ? err.message : String(err); - errors.push({ name: item.name, error: reason }); - if (!json) console.log(` ${c.error("✗")} ${item.name} (skipped: ${reason})`); - } - } + const { results, errors } = await installEach( + items.map((item) => item.name), + { projectDir, force: args.force, vars: args.vars, json }, + ); if (json) { console.log(JSON.stringify(tagAddJson(args.name, results, errors))); diff --git a/packages/cli/src/media-use/lib/adopt.mjs b/packages/cli/src/media-use/lib/adopt.mjs index b140ed02c7..e9950ffbf0 100644 --- a/packages/cli/src/media-use/lib/adopt.mjs +++ b/packages/cli/src/media-use/lib/adopt.mjs @@ -9,18 +9,19 @@ const AUDIO_EXT = new Set([".mp3", ".wav", ".ogg", ".m4a", ".aac"]); const IMAGE_EXT = new Set([".jpg", ".jpeg", ".png", ".gif", ".webp", ".svg", ".ico"]); const VIDEO_EXT = new Set([".mp4", ".webm", ".mov"]); +function audioKind(filePath) { + const words = filePath.toLowerCase().split(/[^a-z0-9]+/); + const has = (test) => words.some(test); + if (has((w) => w === "bgm" || w === "music" || w === "soundtrack")) return "bgm"; + if (has((w) => w === "sfx" || w === "fx" || (w.startsWith("sound") && w !== "soundtrack"))) + return "sfx"; + if (has((w) => w.startsWith("voice") || w.startsWith("narrat"))) return "voice"; + return "bgm"; +} + function inferType(filePath) { const ext = extname(filePath).toLowerCase(); - if (AUDIO_EXT.has(ext)) { - const lower = filePath.toLowerCase(); - if (lower.includes("/bgm/") || lower.includes("/music/") || lower.startsWith("bgm/")) - return "bgm"; - if (lower.includes("/sfx/") || lower.includes("/sound") || lower.startsWith("sfx/")) - return "sfx"; - if (lower.includes("/voice/") || lower.includes("/narrat") || lower.startsWith("voice/")) - return "voice"; - return "bgm"; - } + if (AUDIO_EXT.has(ext)) return audioKind(filePath); if (IMAGE_EXT.has(ext)) { if (ext === ".svg" || ext === ".ico") return "icon"; return "image"; diff --git a/packages/cli/src/media-use/lib/adopt.test.mjs b/packages/cli/src/media-use/lib/adopt.test.mjs index a961ae1776..9e9484380b 100644 --- a/packages/cli/src/media-use/lib/adopt.test.mjs +++ b/packages/cli/src/media-use/lib/adopt.test.mjs @@ -74,6 +74,28 @@ function runTests() { cleanup(); }); + test("labels an audio file by its folder or its file name, not always bgm", () => { + setup(); + drop("voice-intro.mp3"); + drop("sfx-click.mp3"); + drop("narration.mp3"); + drop("soundtrack/main.mp3"); + drop("track.mp3"); + drop("soundfx/pop.mp3"); + const types = Object.fromEntries( + adoptExistingAssets(tmp).map((record) => [record.path, record.type]), + ); + assert.deepEqual(types, { + "assets/voice-intro.mp3": "voice", + "assets/sfx-click.mp3": "sfx", + "assets/narration.mp3": "voice", + "assets/soundtrack/main.mp3": "bgm", + "assets/track.mp3": "bgm", + "assets/soundfx/pop.mp3": "sfx", + }); + cleanup(); + }); + let passed = 0; let failed = 0; for (const { name, fn } of tests) { diff --git a/packages/cli/src/utils/reject-extra-positionals.ts b/packages/cli/src/utils/reject-extra-positionals.ts index 0a45fecd0d..5999bc65cb 100644 --- a/packages/cli/src/utils/reject-extra-positionals.ts +++ b/packages/cli/src/utils/reject-extra-positionals.ts @@ -8,6 +8,7 @@ import { jsonFailure } from "./jsonError.js"; // Leaf commands that read extra positionals from `args._` on purpose, by command path. export const ACCEPTS_EXTRA_POSITIONALS = new Set([ + "add", "audio", "compare", "figma asset", @@ -18,10 +19,6 @@ export const ACCEPTS_EXTRA_POSITIONALS = new Set([ // Free-text commands: their last positional takes every remaining word (`tts hello world`). export const JOINS_EXTRA_POSITIONALS = new Set(["catalog", "tts"]); -const EXTRA_POSITIONAL_HINTS: Record = { - add: "Run add once per item, or pass a single tag to install every item tagged with it.", -}; - // citty parses `--json` on every command (`--json=1` as "1"); after `--` it is a positional. function wantsJson(value: unknown): boolean { return value === true || (typeof value === "string" && value !== "false" && value !== "0"); @@ -71,8 +68,7 @@ export function resolveExtraPositionals( const plural = extra.length === 1 ? "" : "s"; const message = `Unexpected extra argument${plural} for hyperframes ${path}: ${extra.join(", ")}\n` + - `Usage: ${usageLine(path, args)}` + - (EXTRA_POSITIONAL_HINTS[path] ? `\n${EXTRA_POSITIONAL_HINTS[path]}` : ""); + `Usage: ${usageLine(path, args)}`; if (wantsJson(parsed?.json)) console.log(JSON.stringify(jsonFailure("usage_error", message))); else console.error(message); // Telemetry reads the thrown message: the command path and a count, never the arguments.