From 6a437cc1ad5a594f1af036d286f6d5a8d7d707ad Mon Sep 17 00:00:00 2001 From: Seenu K Date: Sat, 8 Aug 2026 12:46:04 +0530 Subject: [PATCH] Refuse an occupied destination before asking, not after MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scaffoldGame` raised `target already exists` at the very end, so the CLI greeted you, probed for Firebase, and took an organization, a git answer and a workflows answer before mentioning that the directory was taken. That is the same insult as learning `flutterfire` is missing after two minutes of Flutter and pub, which this release just finished fixing. `destinationProblem` is now asked before the greeting. It names the first three entries standing in the way rather than saying "not empty", because that usually settles whether the path was mistyped or already scaffolded once, where a bare "not empty" only invites an `ls`. It deliberately does not offer to remove the directory, which is where several scaffolders in this family go. This one writes ninety-odd files and calls `flutter create`; the cost of that answer being wrong against a mistyped path has no upper bound, and every other question here is reversible. A directory that exists and is empty is no longer refused at all — people make one out of habit, and cloning a repository with no first commit leaves exactly that. It is removed and rewritten, since the staging rename publishes itself *as* that path and cannot do it while something is standing there. The changeset is amended rather than added to: #41 is merged but the release PR is still open, so this is the same unreleased change. Co-Authored-By: Claude Opus 5 --- .changeset/lucky-moons-shake.md | 2 + packages/create-eigen-game/src/cli.ts | 16 +++-- packages/create-eigen-game/src/index.ts | 46 +++++++++++++- .../create-eigen-game/test/scaffold.spec.ts | 63 ++++++++++++++++++- 4 files changed, 119 insertions(+), 8 deletions(-) diff --git a/.changeset/lucky-moons-shake.md b/.changeset/lucky-moons-shake.md index 6bf0e3b..e482260 100644 --- a/.changeset/lucky-moons-shake.md +++ b/.changeset/lucky-moons-shake.md @@ -14,4 +14,6 @@ Git and the GitHub Actions workflows are questions too, defaulted yes and no res `--ci` is gone. It read as "I am running in CI" — which is what it means in `npm ci`, in Jest, and in this CLI's own `isCI` check — while actually meaning "emit GitHub Actions workflows". It is now `--workflows` / `--no-workflows`, and the subcommand is `add workflows`. `add ci` still works, undocumented, because it is written into the README of every project already scaffolded. +An occupied destination is refused **before** the first question rather than by `scaffoldGame` after the last one. Being told the directory was taken after carefully giving an organization is the same insult as being told about `flutterfire` after two minutes of Flutter and pub. The message names the first three things standing in the way, which usually settles whether the path was mistyped or already scaffolded once — where "not empty" only invites an `ls`. There is deliberately no offer to delete it: every other question here is reversible and that one is not. A directory that exists and is *empty* is no longer refused at all, since people make one out of habit and cloning a repository with no first commit leaves exactly that. + Also adds `--version`, and stops asking about git at all when the destination is already inside a repository, since the scaffold declines to nest one there whatever it is told. diff --git a/packages/create-eigen-game/src/cli.ts b/packages/create-eigen-game/src/cli.ts index f658d6f..199f77b 100644 --- a/packages/create-eigen-game/src/cli.ts +++ b/packages/create-eigen-game/src/cli.ts @@ -5,7 +5,7 @@ import { fileURLToPath } from "node:url"; import { parseArgs } from "node:util"; import { cancel, intro, isCI, isTTY, log, note, outro, taskLog } from "@clack/prompts"; import color from "picocolors"; -import { addContinuousIntegration, applicationId, detectPackageManager, type FirebaseProblem, firebaseReadiness, insideWorkTree, normaliseTerminalWidth, type PackageManager, plainReporter, type Reporter, scaffoldGame } from "./index.js"; +import { addContinuousIntegration, applicationId, destinationProblem, detectPackageManager, type FirebaseProblem, firebaseReadiness, insideWorkTree, normaliseTerminalWidth, type PackageManager, plainReporter, type Reporter, scaffoldGame } from "./index.js"; import { askForGit, askForOrg, askForPackageManager, askForWorkflows, askToScaffoldWithoutFirebase, DEFAULT_ORG, ORG } from "./prompt.js"; import { summarise } from "./summary.js"; @@ -292,12 +292,18 @@ async function main(args: string[]): Promise { const directory = positionals[0]; - greet(); - - // `applicationId` validates the slug, so a destination that cannot be a game - // name fails before the first question rather than after the last one. + // Everything that makes the command impossible, before the greeting rather + // than after the last question. `applicationId` validates the slug; + // `destinationProblem` is the one `scaffoldGame` would raise at the far end, + // asked here instead — being told the directory was occupied *after* giving + // an organization is the same insult as being told about `flutterfire` after + // two minutes of Flutter and pub. const game = basename(resolve(directory)); applicationId(directory); + const occupied = destinationProblem(directory); + if (occupied !== undefined) throw new Error(occupied); + + greet(); // Every question this run will ask, settled before any of it starts. A flag // is an answer already given; anything left over is asked, or — with nothing diff --git a/packages/create-eigen-game/src/index.ts b/packages/create-eigen-game/src/index.ts index f99db62..6f76ebc 100644 --- a/packages/create-eigen-game/src/index.ts +++ b/packages/create-eigen-game/src/index.ts @@ -1,5 +1,5 @@ import { execFileSync } from "node:child_process"; -import { appendFileSync, cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { appendFileSync, cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, renameSync, rmdirSync, rmSync, statSync, writeFileSync } from "node:fs"; import { basename, dirname, relative, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { ENGINE_PACKAGE, engineRange } from "./engine-range.js"; @@ -626,6 +626,43 @@ function signedOut(probe: Probe): boolean { * declines to nest a repository — but to stop the CLI asking a question whose * answer cannot matter. */ +/** + * Why this destination cannot be scaffolded into, said as a whole message, or + * `undefined` when there is nothing in the way. + * + * A directory that exists and is *empty* is not in the way. People make one out + * of habit, and cloning a repository that has not had its first commit yet + * leaves exactly that — so it is removed and rewritten rather than refused over + * nothing. Anything with contents in it is refused. + * + * Refused, and deliberately not offered as a "remove it and continue?" + * question, which is where several scaffolders in this family go. This one + * writes ninety-odd files and runs `flutter create`; the cost of getting that + * answer wrong against a mistyped path has no upper bound, and `rm -rf` is one + * command that belongs to the person who can see what is in there. Every other + * question this CLI asks is reversible. This one would not be. + * + * Exported so the CLI can ask it before the first question rather than after + * the last one — {@link scaffoldGame} asks it again, for callers that are not + * the CLI. + */ +export function destinationProblem(directory: string): string | undefined { + const root = resolve(directory); + if (!existsSync(root)) return undefined; + + if (!statSync(root).isDirectory()) return `${root} already exists, and is a file.`; + + const entries = readdirSync(root); + if (entries.length === 0) return undefined; + + // Named, not counted. "not empty" invites an `ls`; the three that are + // actually there usually settle whether this was the wrong path or a + // directory that has already been scaffolded once. + const shown = entries.slice(0, 3).sort().join(", "); + const rest = entries.length > 3 ? `, and ${entries.length - 3} more` : ""; + return `${root} already exists, and is not empty — it holds ${shown}${rest}.\n\nScaffold somewhere else, or clear it out yourself. This will not delete a directory you already had.`; +} + export function insideWorkTree(directory: string, probe: Probe = probeCommand): boolean { let candidate = resolve(directory); while (!existsSync(candidate)) { @@ -740,7 +777,12 @@ export function scaffoldGame(options: ScaffoldOptions): ScaffoldResult { const reporter = options.reporter ?? plainReporter; const run = options.run ?? capturingRunner(reporter); - if (existsSync(root)) throw new Error(`target already exists: ${root}`); + const occupied = destinationProblem(root); + if (occupied !== undefined) throw new Error(occupied); + // Empty, or `destinationProblem` would have said so. Removed rather than + // scaffolded into, because the rename below publishes the staging directory + // *as* this path and cannot do that while something is standing there. + if (existsSync(root)) rmdirSync(root); mkdirSync(dirname(root), { recursive: true }); diff --git a/packages/create-eigen-game/test/scaffold.spec.ts b/packages/create-eigen-game/test/scaffold.spec.ts index e67b039..38e2b6c 100644 --- a/packages/create-eigen-game/test/scaffold.spec.ts +++ b/packages/create-eigen-game/test/scaffold.spec.ts @@ -4,7 +4,7 @@ import { tmpdir } from "node:os"; import { relative, resolve } from "node:path"; import { buildGameContract } from "@eigeninteractive/testkit"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { addContinuousIntegration, capturingRunner, decodeUtf8, detectPackageManager, firebaseReadiness, insideWorkTree, normaliseTerminalWidth, type Probe, type Reporter, scaffoldGame } from "../src/index.js"; +import { addContinuousIntegration, capturingRunner, decodeUtf8, destinationProblem, detectPackageManager, firebaseReadiness, insideWorkTree, normaliseTerminalWidth, type Probe, type Reporter, scaffoldGame } from "../src/index.js"; import gameModule from "../templates/worker/src/module/index.js"; const temporaryParent = (): string => mkdtempSync(resolve(tmpdir(), "create-eigen-game-")); @@ -21,6 +21,27 @@ const temporaryParent = (): string => mkdtempSync(resolve(tmpdir(), "create-eige const expectedEngineRange = `^${(JSON.parse(readFileSync(resolve(import.meta.dirname, "../../server/package.json"), "utf8")) as { version: string }).version}`; describe("scaffoldGame", () => { + it("scaffolds into a directory that exists but is empty", () => { + const root = resolve(temporaryParent(), "my-game"); + mkdirSync(root); + + scaffoldGame({ directory: root, bootstrap: false, packageManager: "npm" }); + + expect(existsSync(resolve(root, "server/package.json"))).toBe(true); + }); + + it("refuses a directory with anything in it, and leaves it exactly as it was", () => { + const root = resolve(temporaryParent(), "my-game"); + mkdirSync(root); + writeFileSync(resolve(root, "notes.txt"), "mine"); + + expect(() => scaffoldGame({ directory: root, bootstrap: false, packageManager: "npm" })).toThrow(/already exists, and is not empty/); + // The refusal is the whole feature: nothing here is worth less than a + // scaffold, and only the person who can see it knows that. + expect(readdirSync(root)).toEqual(["notes.txt"]); + expect(readFileSync(resolve(root, "notes.txt"), "utf8")).toBe("mine"); + }); + it("renders the canonical templates as a combined repository", () => { const root = resolve(temporaryParent(), "my-game"); @@ -429,6 +450,46 @@ describe("firebaseReadiness", () => { }); }); +describe("destinationProblem", () => { + it("has nothing to say about a destination that does not exist", () => { + expect(destinationProblem(resolve(temporaryParent(), "my-game"))).toBeUndefined(); + }); + + it("treats an existing but empty directory as free", () => { + // Made out of habit, or left by cloning a repository with no first commit + // in it. Refusing over nothing would be its own papercut. + const root = resolve(temporaryParent(), "my-game"); + mkdirSync(root); + + expect(destinationProblem(root)).toBeUndefined(); + }); + + it("names what is in the way rather than saying it is not empty", () => { + const root = resolve(temporaryParent(), "my-game"); + mkdirSync(root); + for (const entry of ["README.md", "app", "server", "notes.txt"]) writeFileSync(resolve(root, entry), ""); + + const problem = destinationProblem(root); + + // Three of them, because that usually settles whether this was a mistyped + // path or a directory that has already been scaffolded once — where "not + // empty" only invites an `ls`. + expect(problem).toContain("README.md, app, notes.txt"); + expect(problem).toContain("and 1 more"); + // No offer to delete it: this run would write ninety files and call + // `flutter create`, and the answer being wrong against a mistyped path + // costs more than any other question here. + expect(problem).toContain("will not delete a directory you already had"); + }); + + it("catches a destination that is a file", () => { + const root = resolve(temporaryParent(), "my-game"); + writeFileSync(root, ""); + + expect(destinationProblem(root)).toContain("is a file"); + }); +}); + describe("insideWorkTree", () => { it("asks the nearest ancestor that exists, since the destination does not yet", () => { const parent = temporaryParent();