Extract key and flag handling into pure, testable functions - #441
Open
jrr wants to merge 1 commit into
Open
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013EoMUcnWHajEBC54SvGaox
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Last of the three test-focused PRs (#439, #440). 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.tsMaps a keypress to a
GameAction, orundefinedfor keys the game ignores. Escape stays inui.tsx, since quitting the app isn't a game action.GameActionhad to move totypes.tsfirst. Otherwise the new module andui.tsxwould import each other. That also removes the existingreducer.ts→ui.tsxcycle (finding 2.2), since the reducer now takesGameActionfromtypes.ts—src/ui.jshas no importers left besidescli.tsx.The extraction is not purely mechanical.
useInputran its checks as four independentifs, so ctrl+Q dispatchedinput-letter 'Q'andgive-up, and any ctrl chord typed its letter into the row. 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.parseFlags—src/cli-options.tsTurns meow's flags into App's props, or into a message explaining why the game can't start.
cli.tsxkeeps theprocess.exitandconsole.log; only the decision moved, which takescli.tsxfrom 73 lines to 51.A regression from #439, fixed here
While testing the flag path I found that #439 broke
--num-boards 0. It replacedopts?.numBoards || 1withnumBoards ?? 1, and??does not catch0— so the game reachednewGamewith zero boards and crashed on the first render, where it used to fall back to one board. meow also hands backNaNfor a non-numeric value, which broke the same way.Confirmed against a clean worktree at
6be12e7, before any of these PRs:--num-boards 0--num-boards abc(NaN)--num-boards -3Normalising with
||insideparseFlagsrestores the old behaviour and puts it somewhere a test can reach. The negative case is untouched and recorded asit.failsfor finding 1.3.Verified by mutation
??Checks
yarn build,yarn lint,yarn prettier --check ., the--test midgame --quitsmoke run and the--test boguserror path all pass. 36 → 59 passing plus 4 expected failures.Where the review stands after these three
Done: reducer coverage, invariants,
GameActioncycle,expectEqualargument order, and the two extractions.Still open, and the natural next step: finding 1.1 — the multi-board guess limit. Three
it.failstests are already sitting on it and will flip to passing the momentguessesUsedmoves ontoGameState. After that, 1.3 (validate the flags, oneit.failswaiting) and 1.2 (floor the blank-row count).Generated by Claude Code