From 9280231e75e0789182e03481106db781ffe5af3d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 16:10:56 +0000 Subject: [PATCH] Extract key and flag handling into pure, testable functions Two behaviours were only reachable by writing bytes to stdin or by running the CLI, so neither had tests. Both are now ordinary functions. keyToAction (src/key-actions.ts) maps a keypress to a GameAction, or undefined for keys the game ignores. Escape stays in ui.tsx, since quitting the app isn't a game action. This required moving GameAction from ui.tsx to types.ts first, otherwise the new module and ui.tsx would import each other; that also removes the existing reducer -> ui.tsx cycle (finding 2.2), since the reducer now takes GameAction from types. The extraction is not purely mechanical. useInput ran its checks as four independent ifs, so ctrl+Q dispatched input-letter 'Q' *and* give-up, and any ctrl chord typed its letter. Returning a single action forces a resolution: ctrl chords are checked first and are never letters. The stray letter was invisible before because the loss screen hides the current row, so no user-visible behaviour changes here. parseFlags (src/cli-options.ts) turns meow's flags into App's props, or into a message explaining why the game can't start. cli.tsx keeps the process.exit and console.log; only the decision moved. This also fixes a regression from the earlier newGame change. That replaced `opts?.numBoards || 1` with `numBoards ?? 1`, and `??` does not catch 0, so `--num-boards 0` crashed on the first render where it used to fall back to one board. meow also yields NaN for a non-numeric value, which broke the same way. Normalising with `||` in parseFlags restores the old behaviour and puts it somewhere testable. A negative count still crashes, unchanged, and is recorded as it.fails for finding 1.3. Verified by mutation: reordering the ctrl check after the letter check, and reverting the normalisation to `??`, are each caught by the tests covering that behaviour. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013EoMUcnWHajEBC54SvGaox --- src/cli-options.test.ts | 106 ++++++++++++++++++++++++++++++++++++++ src/cli-options.ts | 49 ++++++++++++++++++ src/cli.tsx | 32 +++--------- src/key-actions.test.ts | 92 +++++++++++++++++++++++++++++++++ src/key-actions.ts | 40 ++++++++++++++ src/state/reducer.test.ts | 3 +- src/state/reducer.ts | 3 +- src/types.ts | 6 +++ src/ui.tsx | 25 ++------- 9 files changed, 306 insertions(+), 50 deletions(-) create mode 100644 src/cli-options.test.ts create mode 100644 src/cli-options.ts create mode 100644 src/key-actions.test.ts create mode 100644 src/key-actions.ts diff --git a/src/cli-options.test.ts b/src/cli-options.test.ts new file mode 100644 index 00000000..3f7e1881 --- /dev/null +++ b/src/cli-options.test.ts @@ -0,0 +1,106 @@ +import { describe, it } from "vitest"; +import { knownStateNames, parseFlags } from "./cli-options.js"; +import { testStates } from "./state/game-states.js"; +import { expectEqual } from "./test-util.js"; + +/** meow always supplies these two, so every case starts from them. */ +const baseFlags = { numBoards: 1, quit: false }; + +const optionsOf = (result: ReturnType) => { + if (!result.ok) { + throw new Error(`expected flags to parse, got: ${result.error}`); + } + return result.options; +}; + +describe("parseFlags", () => { + describe("test states", () => { + it("starts an ordinary game when --test is absent", () => { + expectEqual(optionsOf(parseFlags(baseFlags)).initialState, undefined); + }); + + it("loads a known test state", () => { + const { initialState } = optionsOf( + parseFlags({ ...baseFlags, test: "midgame" }), + ); + expectEqual(initialState?.status, testStates.midgame.status); + expectEqual( + initialState?.gameBoards[0].solution, + testStates.midgame.gameBoards[0].solution, + ); + }); + + it("marks the state for exit with --quit", () => { + const { initialState } = optionsOf( + parseFlags({ ...baseFlags, test: "win", quit: true }), + ); + expectEqual(initialState?.exitPlease, true); + }); + + it("leaves the state running without --quit", () => { + const { initialState } = optionsOf( + parseFlags({ ...baseFlags, test: "win" }), + ); + expectEqual(initialState?.exitPlease, false); + }); + + it("rejects an unknown test state, naming the valid ones", () => { + const result = parseFlags({ ...baseFlags, test: "bogus" }); + expectEqual(result, { + ok: false, + error: `Unknown test state 'bogus'. Valid states are ${knownStateNames}`, + }); + }); + + it("names every known state in that message", () => { + for (const name of Object.keys(testStates)) { + expectEqual(knownStateNames.split("|").includes(name), true); + } + }); + }); + + describe("board count", () => { + it("passes an ordinary count through", () => { + expectEqual( + optionsOf(parseFlags({ ...baseFlags, numBoards: 3 })).numBoards, + 3, + ); + }); + + // meow hands back 0 for `--num-boards 0` and NaN for `--num-boards abc`. + // Neither should leave the game with no boards to render. + it("treats zero boards as one", () => { + expectEqual( + optionsOf(parseFlags({ ...baseFlags, numBoards: 0 })).numBoards, + 1, + ); + }); + + it("treats a non-numeric count as one board", () => { + expectEqual( + optionsOf(parseFlags({ ...baseFlags, numBoards: NaN })).numBoards, + 1, + ); + }); + + // Known bug — see docs/architecture-review.md finding 1.3. A negative + // count still reaches newGame and leaves it with no boards at all. + // Flip to `it` when fixed. + it.fails("rejects a negative board count", () => { + expectEqual(parseFlags({ ...baseFlags, numBoards: -3 }).ok, false); + }); + }); + + describe("guess count", () => { + it("passes --num-guesses through", () => { + expectEqual( + optionsOf(parseFlags({ ...baseFlags, numGuesses: 10 })).numGuesses, + 10, + ); + }); + + it("leaves the guess count unset when absent", () => { + expectEqual(optionsOf(parseFlags(baseFlags)).numGuesses, undefined); + }); + }); +}); diff --git a/src/cli-options.ts b/src/cli-options.ts new file mode 100644 index 00000000..55fd6885 --- /dev/null +++ b/src/cli-options.ts @@ -0,0 +1,49 @@ +import { isKnownState, testStates } from "./state/game-states.js"; +import { GameState } from "./types.js"; + +export const knownStateNames = Object.keys(testStates).join("|"); + +/** The flags meow hands back, narrowed to what the game reads. */ +export type CliFlags = { + test?: string; + quit?: boolean; + numBoards: number; + numGuesses?: number; +}; + +/** The props cli.tsx passes to App. */ +export type CliOptions = { + initialState?: GameState; + numBoards: number; + numGuesses?: number; +}; + +/** Either the options to start with, or the reason we can't start. */ +export type ParseResult = + | { ok: true; options: CliOptions } + | { ok: false; error: string }; + +export function parseFlags(flags: CliFlags): ParseResult { + const { test, quit, numBoards, numGuesses } = flags; + + if (test != undefined && !isKnownState(test)) { + return { + ok: false, + error: `Unknown test state '${test}'. Valid states are ${knownStateNames}`, + }; + } + + return { + ok: true, + options: { + initialState: + test == undefined + ? undefined + : { ...testStates[test], exitPlease: quit }, + // `||` rather than `??`: meow yields 0 for `--num-boards 0` and NaN for + // a non-numeric value, and neither should mean "no boards at all". + numBoards: numBoards || 1, + numGuesses, + }, + }; +} diff --git a/src/cli.tsx b/src/cli.tsx index 3b9e0b69..13d648ff 100644 --- a/src/cli.tsx +++ b/src/cli.tsx @@ -2,11 +2,9 @@ import { render } from "ink"; import meow from "meow"; import React from "react"; -import { isKnownState, testStates } from "./state/game-states.js"; +import { knownStateNames, parseFlags } from "./cli-options.js"; import App from "./ui.js"; -const knownStateNames = Object.keys(testStates).join("|"); - const cli = meow( ` Usage @@ -42,32 +40,14 @@ const cli = meow( }, ); -function chooseState( - stateName: string | undefined, - exitPlease: boolean | undefined, -) { - if (stateName == undefined) { - return undefined; - } - if (!isKnownState(stateName)) { - console.log( - `Unknown test state '${stateName}'. Valid states are ${knownStateNames}`, - ); - process.exit(1); - } - - const state = testStates[stateName]; +const parsed = parseFlags(cli.flags); - return { ...state, exitPlease }; +if (!parsed.ok) { + console.log(parsed.error); + process.exit(1); } -const _app = render( - , -); +const _app = render(); // this was cauing a 'Warning: Detected unsettled top-level await' with exit code 13: // await app.waitUntilExit(); diff --git a/src/key-actions.test.ts b/src/key-actions.test.ts new file mode 100644 index 00000000..72a99fa1 --- /dev/null +++ b/src/key-actions.test.ts @@ -0,0 +1,92 @@ +import { describe, it } from "vitest"; +import { keyToAction } from "./key-actions.js"; +import { expectEqual } from "./test-util.js"; + +/** No modifier or special key pressed. */ +const plain = {}; + +describe("keyToAction", () => { + describe("letters", () => { + it("types a lowercase letter as uppercase", () => { + expectEqual(keyToAction("a", plain), { + action: "input-letter", + letter: "A", + }); + }); + + it("types an uppercase letter as-is", () => { + expectEqual(keyToAction("Z", plain), { + action: "input-letter", + letter: "Z", + }); + }); + + it("accepts every letter of the alphabet, in either case", () => { + const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + for (const letter of alphabet) { + expectEqual(keyToAction(letter, plain), { + action: "input-letter", + letter, + }); + expectEqual(keyToAction(letter.toLowerCase(), plain), { + action: "input-letter", + letter, + }); + } + }); + + it("ignores anything that is not a letter", () => { + for (const input of ["1", " ", "-", "?", "é", "🙂"]) { + expectEqual(keyToAction(input, plain), undefined); + } + }); + + it("ignores multi-character input, such as a paste", () => { + expectEqual(keyToAction("abc", plain), undefined); + expectEqual(keyToAction("", plain), undefined); + }); + }); + + describe("editing keys", () => { + it("submits on return", () => { + expectEqual(keyToAction("", { return: true }), { + action: "submit-guess", + }); + }); + + it("deletes on backspace", () => { + expectEqual(keyToAction("", { backspace: true }), { + action: "backspace", + }); + }); + + it("deletes on delete", () => { + expectEqual(keyToAction("", { delete: true }), { action: "backspace" }); + }); + }); + + describe("ctrl chords", () => { + it("gives up on ctrl+Q", () => { + expectEqual(keyToAction("q", { ctrl: true }), { action: "give-up" }); + }); + + it("does not also type a Q on ctrl+Q", () => { + // The handler used to run each check independently, so ctrl+Q both + // typed a letter and gave up. Only the give-up should survive. + const action = keyToAction("q", { ctrl: true }); + expectEqual(action?.action, "give-up"); + }); + + it("ignores every other ctrl chord", () => { + for (const input of ["a", "c", "n", "z"]) { + expectEqual(keyToAction(input, { ctrl: true }), undefined); + } + }); + }); + + describe("keys the game leaves alone", () => { + it("ignores escape, which quits the app rather than moving the game", () => { + expectEqual(keyToAction("", { escape: true }), undefined); + }); + }); +}); diff --git a/src/key-actions.ts b/src/key-actions.ts new file mode 100644 index 00000000..3bf52362 --- /dev/null +++ b/src/key-actions.ts @@ -0,0 +1,40 @@ +import { GameAction } from "./types.js"; + +/** + * The parts of ink's Key that the game looks at. Declared structurally rather + * than importing ink's Key so tests can describe a keypress in a few fields. + */ +export type KeyPress = { + escape?: boolean; + return?: boolean; + backspace?: boolean; + delete?: boolean; + ctrl?: boolean; +}; + +/** + * What a keypress means to the game, or undefined for keys it ignores. + * Quitting the app is deliberately not here: it ends the process rather than + * moving the game along, so ui.tsx handles escape itself. + */ +export function keyToAction( + input: string, + key: KeyPress, +): GameAction | undefined { + if (key.ctrl) { + // A ctrl chord is never a letter to type. Checking this first is what + // stops ctrl+Q from both typing a 'Q' and giving up. + return input == "q" ? { action: "give-up" } : undefined; + } + if (key.return) { + return { action: "submit-guess" }; + } + if (key.backspace || key.delete) { + return { action: "backspace" }; + } + const letter = input.toUpperCase(); + if (input.length == 1 && letter >= "A" && letter <= "Z") { + return { action: "input-letter", letter }; + } + return undefined; +} diff --git a/src/state/reducer.test.ts b/src/state/reducer.test.ts index efd47643..b98e1644 100644 --- a/src/state/reducer.test.ts +++ b/src/state/reducer.test.ts @@ -1,8 +1,7 @@ import { describe, it } from "vitest"; import { KEY_NEW_GAME, KEY_QUIT, WORD_LEN } from "../constants.js"; import { expectEqual, seededRandom } from "../test-util.js"; -import { GameAction } from "../ui.js"; -import { GameState } from "../types.js"; +import { GameAction, GameState } from "../types.js"; import { possibleSolutions } from "../words/possible-solutions.js"; import { newGame } from "./game-states.js"; import { reducer } from "./reducer.js"; diff --git a/src/state/reducer.ts b/src/state/reducer.ts index 2bba574b..b3d89194 100644 --- a/src/state/reducer.ts +++ b/src/state/reducer.ts @@ -1,7 +1,6 @@ import { KEY_NEW_GAME, KEY_QUIT, WORD_LEN } from "../constants.js"; import { colorGuess, isValidWord, pickSolutions } from "../game-logic.js"; -import { GameBoardState, GameState } from "../types.js"; -import { GameAction } from "../ui.js"; +import { GameAction, GameBoardState, GameState } from "../types.js"; import { newGame } from "./game-states.js"; const rowIsFull = (state: GameState & { status: "guessing" }) => diff --git a/src/types.ts b/src/types.ts index d9b4e868..5fe1c68f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -30,3 +30,9 @@ export type GameState = { status: "loss"; } ); + +export type GameAction = + | { action: "input-letter"; letter: string } + | { action: "submit-guess" } + | { action: "backspace" } + | { action: "give-up" }; diff --git a/src/ui.tsx b/src/ui.tsx index c61016ba..c81dbf8b 100644 --- a/src/ui.tsx +++ b/src/ui.tsx @@ -5,18 +5,13 @@ import { Keyboard } from "./components/keyboard.js"; import { StatusText } from "./components/status-text.js"; import { TitleText } from "./components/title-text.js"; import { deriveGameColors } from "./game-colors.js"; +import { keyToAction } from "./key-actions.js"; import { pickSolutions } from "./game-logic.js"; import { newGame } from "./state/game-states.js"; import { reducer } from "./state/reducer.js"; import { GameState } from "./types.js"; import { useStdoutDimensions } from "./use-stdout-dimensions.js"; -export type GameAction = - | { action: "input-letter"; letter: string } - | { action: "submit-guess" } - | { action: "backspace" } - | { action: "give-up" }; - const App: FC<{ initialState?: GameState; numBoards?: number; @@ -47,21 +42,11 @@ const App: FC<{ (input, key) => { if (key.escape) { exit(); + return; } - if (input.length == 1) { - const c = input.toUpperCase(); - if (c >= "A" && c <= "Z") { - dispatch({ action: "input-letter", letter: c }); - } - } - if (key.return) { - dispatch({ action: "submit-guess" }); - } - if (key.backspace || key.delete) { - dispatch({ action: "backspace" }); - } - if (key.ctrl && input == "q") { - dispatch({ action: "give-up" }); + const action = keyToAction(input, key); + if (action) { + dispatch(action); } }, { isActive: gameState.exitPlease != true },