Skip to content
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ All notable user-visible changes to Hunk are documented in this file.

### Fixed

- Included untracked files when `hunk diff <ref>` still compares against the live working tree, while keeping explicit revset diffs commit-to-commit only.
- Balanced Pierre word-level highlights so split-view inline changes stay visible without overpowering the surrounding diff row.
- Smoothed mouse-wheel review scrolling so small diffs stay precise while sustained wheel gestures still speed up.
- Fixed Shift+mouse-wheel horizontal scrolling so it no longer leaks a one-line vertical scroll in some terminals.
Expand Down
47 changes: 44 additions & 3 deletions src/core/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,50 @@ export function runGitText(options: RunGitTextOptions) {
return runGitCommand(options).stdout;
}

/**
* Return whether one `hunk diff` input still compares against the live working tree.
*
* Plain `hunk diff <ref>` keeps the working tree on one side, so untracked files should still
* appear. Explicit revision-set expressions like `a..b`, `a...b`, or `rev^!` expand into positive
* and negative revisions and should stay commit-to-commit only.
*/
function isWorkingTreeGitDiffInput(
input: GitCommandInput,
{
cwd = process.cwd(),
gitExecutable = "git",
}: Pick<RunGitTextOptions, "cwd" | "gitExecutable"> = {},
) {
if (input.staged) {
return false;
}

if (!input.range) {
return true;
}

const revs = runGitText({
input,
args: ["rev-parse", "--revs-only", input.range],
cwd,
gitExecutable,
})
.split("\n")
.map((line) => line.trim())
.filter(Boolean);

const positiveRevs = revs.filter((line) => !line.startsWith("^"));
const negativeRevs = revs.filter((line) => line.startsWith("^"));

return positiveRevs.length === 1 && negativeRevs.length === 0;
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 Extra git rev-parse subprocess on every untracked-file check

isWorkingTreeGitDiffInput spawns a synchronous git rev-parse --revs-only <range> subprocess on every call to listGitUntrackedFiles. In watch mode gitWorkingTreeWatchSignature calls listGitUntrackedFiles on each poll tick, so this adds a full extra git process per watch cycle whenever a range is set. The result never changes within a session, so it could be computed once and memoized/passed in rather than re-run every time.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/core/git.ts
Line: 320-333

Comment:
**Extra `git rev-parse` subprocess on every untracked-file check**

`isWorkingTreeGitDiffInput` spawns a synchronous `git rev-parse --revs-only <range>` subprocess on every call to `listGitUntrackedFiles`. In watch mode `gitWorkingTreeWatchSignature` calls `listGitUntrackedFiles` on each poll tick, so this adds a full extra git process per watch cycle whenever a range is set. The result never changes within a session, so it could be computed once and memoized/passed in rather than re-run every time.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. I cached the range classification per repo/range so watch-mode polls no longer spawn an extra git rev-parse once the one-ref vs revset decision has been computed.

This comment was generated by Pi using OpenAI o3

}

/** Return whether working-tree review should synthesize untracked files into the patch stream. */
function shouldIncludeUntrackedFiles(input: GitCommandInput) {
return !input.staged && !input.range && input.options.excludeUntracked !== true;
function shouldIncludeUntrackedFiles(
input: GitCommandInput,
options: Pick<RunGitTextOptions, "cwd" | "gitExecutable"> = {},
) {
return input.options.excludeUntracked !== true && isWorkingTreeGitDiffInput(input, options);
}

/** Parse porcelain status output down to repo-root-relative untracked file paths. */
Expand Down Expand Up @@ -348,7 +389,7 @@ export function listGitUntrackedFiles(
gitExecutable = "git",
}: Omit<RunGitTextOptions, "input" | "args"> & { repoRoot?: string } = {},
) {
if (!shouldIncludeUntrackedFiles(input)) {
if (!shouldIncludeUntrackedFiles(input, { cwd, gitExecutable })) {
return [];
}

Expand Down
77 changes: 77 additions & 0 deletions src/core/loaders.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,83 @@ describe("loadAppBootstrap", () => {
expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["example.ts"]);
});

test("includes untracked files when diff compares the working tree against one ref", async () => {
const dir = createTempRepo("hunk-git-ref-untracked-");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n");
git(dir, "add", "tracked.ts");
git(dir, "commit", "-m", "initial");
git(dir, "branch", "main");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n");
git(dir, "add", "tracked.ts");
git(dir, "commit", "-m", "second");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 3;\n");
writeFileSync(join(dir, "new-file.ts"), "export const added = true;\n");

const bootstrap = await loadFromRepo(dir, {
kind: "git",
range: "main",
staged: false,
options: { mode: "auto" },
});

expect(bootstrap.changeset.files.map((file) => file.path)).toEqual([
"tracked.ts",
"new-file.ts",
]);
});

test("excludes untracked files for explicit git ranges that do not include the working tree", async () => {
const dir = createTempRepo("hunk-git-range-no-untracked-");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n");
git(dir, "add", "tracked.ts");
git(dir, "commit", "-m", "initial");
git(dir, "branch", "main");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n");
git(dir, "add", "tracked.ts");
git(dir, "commit", "-m", "second");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 3;\n");
writeFileSync(join(dir, "new-file.ts"), "export const added = true;\n");

const bootstrap = await loadFromRepo(dir, {
kind: "git",
range: "main..HEAD",
staged: false,
options: { mode: "auto" },
});

expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["tracked.ts"]);
});

test("excludes untracked files for revset diffs like HEAD^! that do not include the working tree", async () => {
const dir = createTempRepo("hunk-git-revset-no-untracked-");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n");
git(dir, "add", "tracked.ts");
git(dir, "commit", "-m", "initial");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n");
git(dir, "add", "tracked.ts");
git(dir, "commit", "-m", "second");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 3;\n");
writeFileSync(join(dir, "new-file.ts"), "export const added = true;\n");

const bootstrap = await loadFromRepo(dir, {
kind: "git",
range: "HEAD^!",
staged: false,
options: { mode: "auto" },
});

expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["tracked.ts"]);
});

test("loads untracked files whose names need parser-safe diff headers", async () => {
const dir = createTempRepo("hunk-git-quoted-untracked-");

Expand Down
40 changes: 36 additions & 4 deletions src/core/watch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,19 @@ function withCwd<T>(cwd: string, callback: () => T) {
}
}

function createGitInput(overrides: Partial<Extract<CliInput, { kind: "git" }>["options"]> = {}) {
function createGitInput({
options,
...overrides
}: {
options?: Partial<Extract<CliInput, { kind: "git" }>["options"]>;
} & Partial<Omit<Extract<CliInput, { kind: "git" }>, "kind" | "options">> = {}) {
return {
kind: "git",
staged: false,
...overrides,
options: {
mode: "auto",
...overrides,
...options,
},
} satisfies Extract<CliInput, { kind: "git" }>;
}
Expand Down Expand Up @@ -101,13 +107,39 @@ describe("computeWatchSignature", () => {
writeFileSync(untrackedPath, "first\n");

const initialSignature = withCwd(dir, () =>
computeWatchSignature(createGitInput({ excludeUntracked: true })),
computeWatchSignature(createGitInput({ options: { excludeUntracked: true } })),
);
writeFileSync(untrackedPath, "second\n");
const changedSignature = withCwd(dir, () =>
computeWatchSignature(createGitInput({ excludeUntracked: true })),
computeWatchSignature(createGitInput({ options: { excludeUntracked: true } })),
);

expect(changedSignature).toEqual(initialSignature);
});

test("tracks untracked file changes when diff compares the working tree against one ref", () => {
const dir = createTempRepo("hunk-watch-ref-untracked-");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n");
git(dir, "add", "tracked.ts");
git(dir, "commit", "-m", "initial");
git(dir, "branch", "main");

writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n");
git(dir, "add", "tracked.ts");
git(dir, "commit", "-m", "second");

const untrackedPath = join(dir, "note.txt");
writeFileSync(untrackedPath, "first\n");

const initialSignature = withCwd(dir, () =>
computeWatchSignature(createGitInput({ range: "main" })),
);
writeFileSync(untrackedPath, "second\n");
const changedSignature = withCwd(dir, () =>
computeWatchSignature(createGitInput({ range: "main" })),
);

expect(changedSignature).not.toEqual(initialSignature);
});
});
Loading