diff --git a/.changeset/quieter-flag-tip.md b/.changeset/quieter-flag-tip.md new file mode 100644 index 0000000..c98dd76 --- /dev/null +++ b/.changeset/quieter-flag-tip.md @@ -0,0 +1,5 @@ +--- +"@somewhatabstract/x": patch +--- + +The "use `--` to separate flags" tip is now only shown when an x option flag (e.g. `--dry-run`, `-d`) appears after the script name without a `--` separator — previously it fired for any flag-looking argument, which was noisy because most of those flags pass through to the script just fine. Set `NO_WARN=1` to silence the tip entirely. diff --git a/package.json b/package.json index 868d0bd..b09ccca 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "author": "Jeff Yates ", "keywords": [], "license": "MIT", - "packageManager": "pnpm@11.0.0+sha512.5bd187500e49cc6c3d891d973b432c02b844a5eb7209172c90a517a3ef4f579ed5c23d409b699e6a9dc418ff7b2b1890e63f6d74f1d3fc49848f37779c89c84c", + "packageManager": "pnpm@11.4.0+sha512.f0febc7e37552ab485494a914241b338e0b3580b93d54ce31f00933015880863129038a1b4ae4e414a0ee63ac35bf21197e990172c4a68256450b5636310968f", "engines": { "node": ">= 24", "npm": "please-use-pnpm", diff --git a/src/__tests__/x.test.ts b/src/__tests__/x.test.ts index cbb9603..bbb0ef9 100644 --- a/src/__tests__/x.test.ts +++ b/src/__tests__/x.test.ts @@ -34,6 +34,7 @@ describe("bin/x", () => { afterEach(() => { vi.restoreAllMocks(); + vi.unstubAllEnvs(); }); it("should call xImpl with the script name from argv", async () => { @@ -162,12 +163,12 @@ describe("bin/x", () => { ); }); - it("should show a tip when args contain flags and -- was not used", async () => { + it("should show a tip when an x flag appears after the script name without --", async () => { // Arrange xImplMock.mockResolvedValue({exitCode: 0}); // Act - await main(["node", "x.mjs", "my-script", "--unknown-flag", "value"]); + await main(["node", "x.mjs", "my-script", "--dry-run"]); // Assert expect(console.warn).toHaveBeenCalledWith( @@ -175,16 +176,29 @@ describe("bin/x", () => { ); }); - it("should show the corrected command in the tip when args contain flags without --", async () => { + it("should show the corrected command in the tip when an x flag follows the script name", async () => { // Arrange xImplMock.mockResolvedValue({exitCode: 0}); // Act - await main(["node", "x.mjs", "my-script", "--unknown-flag", "value"]); + await main(["node", "x.mjs", "my-script", "--dry-run", "value"]); + + // Assert + expect(console.warn).toHaveBeenCalledWith( + " x my-script -- --dry-run value", + ); + }); + + it("should advertise NO_WARN in the tip so users know how to silence it", async () => { + // Arrange + xImplMock.mockResolvedValue({exitCode: 0}); + + // Act + await main(["node", "x.mjs", "my-script", "--dry-run"]); // Assert expect(console.warn).toHaveBeenCalledWith( - " x my-script -- --unknown-flag value", + "(Set NO_WARN=1 to silence this tip.)", ); }); @@ -193,14 +207,7 @@ describe("bin/x", () => { xImplMock.mockResolvedValue({exitCode: 0}); // Act - await main([ - "node", - "x.mjs", - "my-script", - "--", - "--unknown-flag", - "value", - ]); + await main(["node", "x.mjs", "my-script", "--", "--dry-run", "value"]); // Assert expect(console.warn).not.toHaveBeenCalled(); @@ -223,6 +230,81 @@ describe("bin/x", () => { expect(console.warn).not.toHaveBeenCalled(); }); + it("should not show a tip when args contain non-x flags (script's own flags pass through cleanly)", async () => { + // Arrange + xImplMock.mockResolvedValue({exitCode: 0}); + + // Act + await main(["node", "x.mjs", "my-script", "--unknown-flag", "value"]); + + // Assert + expect(console.warn).not.toHaveBeenCalled(); + }); + + it("should not show a tip when an x flag appears before the script name (unambiguously for x)", async () => { + // Arrange + xImplMock.mockResolvedValue({exitCode: 0}); + + // Act + await main(["node", "x.mjs", "--dry-run", "my-script"]); + + // Assert + expect(console.warn).not.toHaveBeenCalled(); + }); + + it("should not show a tip when NO_WARN is set, even if an x flag follows the script name", async () => { + // Arrange + xImplMock.mockResolvedValue({exitCode: 0}); + vi.stubEnv("NO_WARN", "1"); + + // Act + await main(["node", "x.mjs", "my-script", "--dry-run"]); + + // Assert + expect(console.warn).not.toHaveBeenCalled(); + }); + + it("should show a tip when --list follows the script name, even though x exits into list mode", async () => { + // Arrange + listImplMock.mockResolvedValue({exitCode: 0}); + + // Act + await main(["node", "x.mjs", "my-script", "--list"]); + + // Assert + expect(console.warn).toHaveBeenCalledWith( + `Tip: To pass flags to "my-script", use '--' to separate them:`, + ); + }); + + it("should show a tip when --help follows the script name, even though x exits into help mode", async () => { + // Arrange + + // Act + await main(["node", "x.mjs", "my-script", "--help"]); + + // Assert + expect(console.warn).toHaveBeenCalledWith( + `Tip: To pass flags to "my-script", use '--' to separate them:`, + ); + }); + + it("should print the tip after the command's output, so it lands at the bottom of the terminal", async () => { + // Arrange + listImplMock.mockResolvedValue({exitCode: 0}); + + // Act + await main(["node", "x.mjs", "my-script", "--list"]); + + // Assert + const listCallOrder = listImplMock.mock.invocationCallOrder[0]; + const warnCallOrders = vi.mocked(console.warn).mock.invocationCallOrder; + const firstWarnAfterList = warnCallOrders.find( + (order) => order > listCallOrder, + ); + expect(firstWarnAfterList).toBeDefined(); + }); + it("should pass positional args to xImpl when user omits --", async () => { // Arrange xImplMock.mockResolvedValue({exitCode: 0}); @@ -310,11 +392,11 @@ describe("bin/x", () => { xImplMock.mockResolvedValue({exitCode: 0}); // Act - await main(["node", "x.mjs", "e2e", "setup", "--flag", "value"]); + await main(["node", "x.mjs", "e2e", "setup", "--dry-run", "value"]); // Assert expect(console.warn).toHaveBeenCalledWith( - " x e2e -- setup --flag value", + " x e2e -- setup --dry-run value", ); }); diff --git a/src/bin/x.ts b/src/bin/x.ts index d5d6b00..8e1b3ce 100755 --- a/src/bin/x.ts +++ b/src/bin/x.ts @@ -2,7 +2,7 @@ import {realpathSync} from "node:fs"; import {basename} from "node:path"; import {fileURLToPath} from "node:url"; -import yargs from "yargs"; +import yargs, {type Argv} from "yargs"; import {hideBin} from "yargs/helpers"; import {HandledError} from "../errors"; import {getCompletions} from "../get-completions"; @@ -11,6 +11,28 @@ import {outputHelpWithSplash} from "../output-help-with-splash"; import {validateArgv} from "../validate-argv"; import {type XResult, xImpl} from "../x-impl"; +const POSITIONAL_NAMES = new Set(["script-name", "scriptName"]); + +/** + * Collect the flag forms (e.g. "--list", "-l") of all of x's own options from + * a configured yargs instance after parsing. Keeps the warning logic in sync + * with the yargs config without a hand-maintained list. + */ +function collectXFlags(yi: Argv): Set { + const flags = new Set(); + const parsed = yi.parsed; + if (parsed && typeof parsed === "object") { + for (const name of Object.keys(parsed.aliases)) { + if (POSITIONAL_NAMES.has(name)) continue; + flags.add(name.length === 1 ? `-${name}` : `--${name}`); + } + } + // --completion is registered as a yargs completion command, not an + // option, so it doesn't appear in parsed.aliases. + flags.add("--completion"); + return flags; +} + export async function main(rawArgv: string[]): Promise { const rawArgs = hideBin(rawArgv); @@ -104,41 +126,76 @@ export async function main(rawArgv: string[]): Promise { ? await yi.parse() : await yi.scriptName("x").parse(); - if (argv.help || argv.h) { - outputHelpWithSplash(yi); - return {exitCode: 0}; + // Decide up-front whether to print the misplaced-flag tip, but defer the + // actual output until the command finishes so it lands at the bottom of + // the terminal where the user is already looking. The detection has to + // happen now (before --help/--list early-exits) so we capture the user's + // intent against the raw args; the printing happens in a finally below. + const scriptName = (argv["script-name"] as string | undefined)?.trim(); + const printFlagTip = makeFlagTipPrinter(scriptName, rawArgs, yi); + + try { + if (argv.help || argv.h) { + outputHelpWithSplash(yi); + return {exitCode: 0}; + } + + // Check if we are in list mode + const listArg = argv.list; + if (listArg !== undefined) { + const mode = listArg === "full" ? "full" : "names-only"; + const json = !!argv.json; + return await listImpl({mode, json}); + } + + const args = (argv._ as string[]) || []; + const options = { + dryRun: argv["dry-run"] as boolean, + }; + + // scriptName is guaranteed non-empty here by validateArgv (since + // --list/--help would have early-exited above if it was missing). + return await xImpl(scriptName as string, args, options); + } finally { + printFlagTip(); } +} - // Check if we are in list mode - const listArg = argv.list; - if (listArg !== undefined) { - const mode = listArg === "full" ? "full" : "names-only"; - const json = !!argv.json; - return await listImpl({mode, json}); +/** + * Detect if the user passed an x flag after the script name without `--`, + * and return a function that prints the tip when called. Returns a no-op + * when there's nothing to warn about (or when NO_WARN is set). Doing the + * detection eagerly and the printing lazily lets us land the tip at the + * bottom of the output where it's most visible. + */ +function makeFlagTipPrinter( + scriptName: string | undefined, + rawArgs: string[], + yi: Argv, +): () => void { + const noop = () => {}; + if (!scriptName || process.env.NO_WARN || rawArgs.includes("--")) { + return noop; } + const scriptIdx = rawArgs.indexOf(scriptName); + if (scriptIdx < 0) return noop; - const args = (argv._ as string[]) || []; - const options = { - dryRun: argv["dry-run"] as boolean, - }; + const beforeScript = rawArgs.slice(0, scriptIdx); + const afterScript = rawArgs.slice(scriptIdx + 1); + const xFlags = collectXFlags(yi); + const hasXFlagAfterScript = afterScript.some( + (arg) => xFlags.has(arg) || xFlags.has(arg.split("=")[0]), + ); + if (!hasXFlagAfterScript) return noop; - // If any args look like flags and -- was not used, warn the user and suggest - // using -- to explicitly separate flag arguments from x's own options. - const scriptName = (argv["script-name"] as string).trim(); - if (!rawArgs.includes("--")) { - const flagLikeArgs = args.filter( - (arg) => typeof arg === "string" && arg.startsWith("-"), + const prefix = [...beforeScript, scriptName].join(" "); + return () => { + console.warn( + `Tip: To pass flags to "${scriptName}", use '--' to separate them:`, ); - if (flagLikeArgs.length > 0) { - console.warn( - `Tip: To pass flags to "${scriptName}", use '--' to separate them:`, - ); - console.warn(` x ${scriptName} -- ${args.join(" ")}`); - } - } - - // Run the implementation and exit with the appropriate code - return xImpl(scriptName, args, options); + console.warn(` x ${prefix} -- ${afterScript.join(" ")}`); + console.warn(`(Set NO_WARN=1 to silence this tip.)`); + }; } // Only run main if we aren't being imported as a module.