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();