Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ Requirements:
## Quick start

```bash
hunk # show help
hunk # launch the default review UI (same as `hunk diff`)
hunk --help # show top-level help
hunk --version # print the installed version
```

Expand Down
20 changes: 16 additions & 4 deletions src/core/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,11 @@ function renderCliVersion() {
}

/** Build the top-level help text shown by bare `hunk` and `hunk --help`. */
function renderCliHelp() {
export function renderCliHelp() {
return [
"Usage: hunk <command> [options]",
"Usage:",
" hunk",
" hunk <command> [options]",
"",
"Desktop-inspired terminal diff viewer for agent-authored changesets.",
"",
Expand Down Expand Up @@ -139,6 +141,7 @@ function renderCliHelp() {
" --exclude-untracked hide untracked files in working tree reviews",
"",
"Notes:",
" Bare `hunk` starts the default review UI (`hunk diff`) in an interactive terminal.",
" Run `hunk <command> --help` for command-specific syntax and options.",
"",
].join("\n");
Expand Down Expand Up @@ -445,7 +448,12 @@ async function parseDifftoolCommand(tokens: string[], argv: string[]): Promise<P
}

function requireReloadableCliInput(input: ParsedCliInput): CliInput {
if (input.kind === "help" || input.kind === "pager" || input.kind === "mcp-serve") {
if (
input.kind === "bare" ||
input.kind === "help" ||
input.kind === "pager" ||
input.kind === "mcp-serve"
) {
throw new Error(
"Session reload requires a Hunk review command after --, such as `diff` or `show`.",
);
Expand Down Expand Up @@ -1042,7 +1050,11 @@ export async function parseCli(argv: string[]): Promise<ParsedCliInput> {
const args = argv.slice(2);
const [commandName, ...rest] = args;

if (!commandName || commandName === "help" || commandName === "--help" || commandName === "-h") {
if (!commandName) {
return { kind: "bare" };
}

if (commandName === "help" || commandName === "--help" || commandName === "-h") {
return { kind: "help", text: renderCliHelp() };
}

Expand Down
45 changes: 38 additions & 7 deletions src/core/startup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ import {
usesPipedPatchInput,
type ControllingTerminal,
} from "./terminal";
import type { AppBootstrap, CliInput, ParsedCliInput, SessionCommandInput } from "./types";
import type {
AppBootstrap,
CliInput,
PagerCommandInput,
ParsedCliInput,
SessionCommandInput,
} from "./types";
import { canReloadInput } from "./watch";
import { parseCli } from "./cli";
import { parseCli, renderCliHelp } from "./cli";

export type StartupPlan =
| {
Expand Down Expand Up @@ -44,6 +50,7 @@ export interface StartupDeps {
loadAppBootstrapImpl?: typeof loadAppBootstrap;
usesPipedPatchInputImpl?: typeof usesPipedPatchInput;
openControllingTerminalImpl?: typeof openControllingTerminal;
stdinIsTTY?: boolean;
}

/** Normalize startup work so help, pager, and app-bootstrap paths can be tested directly. */
Expand All @@ -60,8 +67,9 @@ export async function prepareStartupPlan(
const loadAppBootstrapImpl = deps.loadAppBootstrapImpl ?? loadAppBootstrap;
const usesPipedPatchInputImpl = deps.usesPipedPatchInputImpl ?? usesPipedPatchInput;
const openControllingTerminalImpl = deps.openControllingTerminalImpl ?? openControllingTerminal;
const stdinIsTTY = deps.stdinIsTTY ?? Boolean(process.stdin.isTTY);

let parsedCliInput = await parseCliImpl(argv);
const parsedCliInput = await parseCliImpl(argv);

if (parsedCliInput.kind === "help") {
return {
Expand All @@ -83,28 +91,51 @@ export async function prepareStartupPlan(
};
}

if (parsedCliInput.kind === "pager") {
const bareInvocation = parsedCliInput.kind === "bare";
let appCliInput: CliInput | PagerCommandInput;

if (bareInvocation) {
// Keep bare `hunk` ergonomic in interactive shells while preserving pager-style stdin flows.
if (stdinIsTTY) {
appCliInput = { kind: "git", staged: false, options: {} };
} else {
appCliInput = { kind: "pager", options: {} };
}
} else if (parsedCliInput.kind === "pager") {
appCliInput = parsedCliInput;
} else {
appCliInput = parsedCliInput;
}
Comment on lines +104 to +108
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant else if and else branches

Both the else if (parsedCliInput.kind === "pager") and the final else branch do exactly the same thing (appCliInput = parsedCliInput). Since help, mcp-serve, session, and bare have all been handled above, the only remaining shapes are CliInput | PagerCommandInput, making the distinction between the two branches meaningless.

Suggested change
} else if (parsedCliInput.kind === "pager") {
appCliInput = parsedCliInput;
} else {
appCliInput = parsedCliInput;
}
} else {
appCliInput = parsedCliInput;
}


if (appCliInput.kind === "pager") {
const stdinText = await readStdinText();

if (stdinText.length === 0 && bareInvocation) {
return {
kind: "help",
text: renderCliHelp(),
};
}
Comment on lines +113 to +118
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 renderCliHelp bypasses dependency injection

Every other side-effectful operation in prepareStartupPlan is routed through an injectable dep (e.g. parseCliImpl, readStdinText). Here renderCliHelp() is called directly, so tests that inject a custom parseCliImpl returning { kind: "bare" } cannot control the help text produced by the bare + empty-stdin path. This is fine for the current test suite (the real renderCliHelp works fine), but it breaks the otherwise-consistent injection pattern. Consider adding an optional renderCliHelpImpl dep, or accepting a pre-rendered helpText string alongside the bare input.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


if (!looksLikePatchInputImpl(stdinText)) {
return {
kind: "plain-text-pager",
text: stdinText,
};
}

parsedCliInput = {
appCliInput = {
kind: "patch",
file: "-",
text: stdinText,
options: {
...parsedCliInput.options,
...appCliInput.options,
pager: true,
},
};
}

const runtimeCliInput = resolveRuntimeCliInputImpl(parsedCliInput);
const runtimeCliInput = resolveRuntimeCliInputImpl(appCliInput);
const configured = resolveConfiguredCliInputImpl(runtimeCliInput);
const cliInput = configured.input;

Expand Down
5 changes: 5 additions & 0 deletions src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ export interface HelpCommandInput {
text: string;
}

export interface BareCommandInput {
kind: "bare";
}

export interface PagerCommandInput {
kind: "pager";
options: CommonOptions;
Expand Down Expand Up @@ -232,6 +236,7 @@ export type CliInput =
export type ParsedCliInput =
| CliInput
| HelpCommandInput
| BareCommandInput
| PagerCommandInput
| McpServeCommandInput
| SessionCommandInput;
Expand Down
43 changes: 21 additions & 22 deletions test/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,34 +22,33 @@ afterEach(() => {
});

describe("parseCli", () => {
test("prints help when no subcommand is passed", async () => {
test("returns a bare invocation marker when no subcommand is passed", async () => {
const parsed = await parseCli(["bun", "hunk"]);

expect(parsed.kind).toBe("help");
if (parsed.kind !== "help") {
throw new Error("Expected top-level help output.");
}

expect(parsed.text).toContain("Usage:");
expect(parsed.text).toContain("hunk diff");
expect(parsed.text).toContain("hunk show");
expect(parsed.text).toContain("Global options:");
expect(parsed.text).toContain("Common review options:");
expect(parsed.text).toContain("auto-reload when the current diff input changes");
expect(parsed.text).toContain("Git diff options:");
expect(parsed.text).toContain("Notes:");
expect(parsed.text).toContain(
"Run `hunk <command> --help` for command-specific syntax and options.",
);
expect(parsed.text).not.toContain("Config:");
expect(parsed.text).not.toContain("Examples:");
expect(parsed).toEqual({ kind: "bare" });
});

test("prints the same top-level help for --help", async () => {
const bare = await parseCli(["bun", "hunk"]);
test("prints top-level help for explicit --help", async () => {
const explicit = await parseCli(["bun", "hunk", "--help"]);

expect(explicit).toEqual(bare);
expect(explicit.kind).toBe("help");
if (explicit.kind !== "help") {
throw new Error("Expected explicit help output.");
}

expect(explicit.text).toContain("Usage:");
expect(explicit.text).toContain("hunk diff");
expect(explicit.text).toContain("hunk show");
expect(explicit.text).toContain("Global options:");
expect(explicit.text).toContain("Common review options:");
expect(explicit.text).toContain("auto-reload when the current diff input changes");
expect(explicit.text).toContain("Git diff options:");
expect(explicit.text).toContain("Notes:");
expect(explicit.text).toContain(
"Run `hunk <command> --help` for command-specific syntax and options.",
);
expect(explicit.text).not.toContain("Config:");
expect(explicit.text).not.toContain("Examples:");
});

test("prints the package version for --version and version", async () => {
Expand Down
99 changes: 97 additions & 2 deletions test/startup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,44 @@ function createBootstrap(input: CliInput): AppBootstrap {
}

describe("startup planning", () => {
test("returns help output without entering app startup", async () => {
let loaded = false;
test("defaults bare interactive invocations to working-tree diff startup", async () => {
const seenInputs: CliInput[] = [];

const plan = await prepareStartupPlan(["bun", "hunk"], {
parseCliImpl: async () => ({ kind: "bare" }),
stdinIsTTY: true,
resolveRuntimeCliInputImpl(input) {
seenInputs.push(input);
return input;
},
resolveConfiguredCliInputImpl(input) {
seenInputs.push(input);
return { input } as never;
},
loadAppBootstrapImpl: async (input) => {
seenInputs.push(input);
return createBootstrap(input);
},
usesPipedPatchInputImpl: () => false,
});

expect(plan.kind).toBe("app");
if (plan.kind !== "app") {
throw new Error("Expected app startup plan.");
}

expect(plan.cliInput).toEqual({
kind: "git",
staged: false,
options: {},
});
expect(seenInputs).toHaveLength(3);
});

test("returns help output for explicit --help without entering app startup", async () => {
let loaded = false;

const plan = await prepareStartupPlan(["bun", "hunk", "--help"], {
parseCliImpl: async () => ({ kind: "help", text: "Usage: hunk\n" }),
loadAppBootstrapImpl: async () => {
loaded = true;
Expand All @@ -32,6 +66,28 @@ describe("startup planning", () => {
expect(loaded).toBe(false);
});

test("keeps bare non-interactive invocation on help when stdin is empty", async () => {
let loaded = false;

const plan = await prepareStartupPlan(["bun", "hunk"], {
parseCliImpl: async () => ({ kind: "bare" }),
stdinIsTTY: false,
readStdinText: async () => "",
looksLikePatchInputImpl: () => false,
loadAppBootstrapImpl: async () => {
loaded = true;
throw new Error("unreachable");
},
});

expect(plan.kind).toBe("help");
if (plan.kind !== "help") {
throw new Error("Expected help output.");
}
expect(plan.text).toContain("Usage:");
expect(loaded).toBe(false);
});

test("passes the MCP serve command through without app bootstrap work", async () => {
let loaded = false;

Expand Down Expand Up @@ -121,6 +177,45 @@ describe("startup planning", () => {
expect(seenInputs).toHaveLength(3);
});

test("normalizes bare non-interactive diff stdin into patch app startup", async () => {
const seenInputs: CliInput[] = [];

const plan = await prepareStartupPlan(["bun", "hunk"], {
parseCliImpl: async () => ({ kind: "bare" }),
stdinIsTTY: false,
readStdinText: async () => "diff --git a/a.ts b/a.ts\n@@ -1 +1 @@\n-old\n+new\n",
looksLikePatchInputImpl: () => true,
resolveRuntimeCliInputImpl(input) {
seenInputs.push(input);
return input;
},
resolveConfiguredCliInputImpl(input) {
seenInputs.push(input);
return { input } as never;
},
loadAppBootstrapImpl: async (input) => {
seenInputs.push(input);
return createBootstrap(input);
},
usesPipedPatchInputImpl: () => false,
});

expect(plan.kind).toBe("app");
if (plan.kind !== "app") {
throw new Error("Expected app startup plan.");
}

expect(plan.cliInput).toMatchObject({
kind: "patch",
file: "-",
text: "diff --git a/a.ts b/a.ts\n@@ -1 +1 @@\n-old\n+new\n",
options: {
pager: true,
},
});
expect(seenInputs).toHaveLength(3);
});

test("rejects watch mode for stdin-backed patch inputs", async () => {
const cliInput: CliInput = {
kind: "patch",
Expand Down
Loading