Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 17 additions & 0 deletions src/main/gitService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,23 @@ export class GitService {
], undefined, validation.patch);
}

async discardHunk(request: GitHunkRequest): Promise<GitOperationResult> {
if (request.side !== "unstaged") {
return this.createOperationFailure(request.repoPath, "Only unstaged hunks can be reverted.");
}
const validation = await this.validateHunkRequest(request, "unstaged");
if ("error" in validation) {
return this.createOperationFailure(request.repoPath, validation.error);
}

return this.runGitOperation(request.repoPath, [
"apply",
"--reverse",
"--whitespace=nowarn",
"-"
Comment on lines +1025 to +1029

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve file-mode edits when discarding a content hunk

When a file has both an unstaged mode change (such as chmod +x) and content edits, the selected hunk patch still contains the file-level old mode/new mode headers collected by groupDiffRowsByHunk. Passing that patch directly to git apply --reverse therefore reverts the executable-bit change along with the selected content hunk, silently discarding an unrelated working-tree edit. Remove file-mode metadata from the patch used for this operation so that only the chosen hunk is reversed.

Useful? React with 👍 / 👎.

], undefined, validation.patch);
}

async unstageHunk(request: GitHunkRequest): Promise<GitOperationResult> {
const validation = await this.validateHunkRequest(request, "staged");
if ("error" in validation) {
Expand Down
12 changes: 12 additions & 0 deletions src/main/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,18 @@ ipcMain.handle(IPC_CHANNELS.unstageFiles, async (event, request: CoordinatedRequ
);
});

ipcMain.handle(IPC_CHANNELS.discardHunk, async (event, request: CoordinatedRequest<GitHunkRequest>) => {
return runExclusiveGitOperation(
async () => {
if ((await vcsRouter.resolveKind(request.repoPath)) !== "git") {
return createOperationFailure(request.repoPath, "Revert Hunk is available only for Git repositories.");
}
return gitService.discardHunk(request);
},
repositoryOperationOptions(event, request.operationId, request.repoPath)
);
});

ipcMain.handle(IPC_CHANNELS.stageHunk, async (event, request: CoordinatedRequest<GitHunkRequest>) => {
return runExclusiveGitOperation(
async () => (await vcsRouter.serviceForRepo(request.repoPath)).stageHunk(request),
Expand Down
2 changes: 2 additions & 0 deletions src/main/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,8 @@ const api: GitheadApi = {
ipcRenderer.invoke(IPC_CHANNELS.stageFiles, request) as ReturnType<GitheadApi["stageFiles"]>,
unstageFiles: (request: CoordinatedRequest<GitPathRequest>) =>
ipcRenderer.invoke(IPC_CHANNELS.unstageFiles, request) as ReturnType<GitheadApi["unstageFiles"]>,
discardHunk: (request: CoordinatedRequest<GitHunkRequest>) =>
ipcRenderer.invoke(IPC_CHANNELS.discardHunk, request) as ReturnType<GitheadApi["discardHunk"]>,
stageHunk: (request: CoordinatedRequest<GitHunkRequest>) =>
ipcRenderer.invoke(IPC_CHANNELS.stageHunk, request) as ReturnType<GitheadApi["stageHunk"]>,
unstageHunk: (request: CoordinatedRequest<GitHunkRequest>) =>
Expand Down
21 changes: 21 additions & 0 deletions src/renderer/App.history.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1935,6 +1935,27 @@ describe("App", { timeout: 10_000 }, () => {
expect(screen.queryByRole("button", { name: "Wrap diff lines" })).toBeNull();
});

it("confirms hunk discard and reloads the remaining changes", async () => {
const user = userEvent.setup();
const file = createStatusFile("src/App.tsx", { isUnstaged: true, worktreeStatus: "M" });
const diff = createTextDiff(file.path, "discard-me");
vi.mocked(githead.getRepoSummary).mockResolvedValue(createSummary({ files: [file] }));
vi.mocked(githead.getFileDiff).mockResolvedValueOnce(diff).mockResolvedValue(createTextDiff(file.path, "remaining-hunk"));
render(<App />);
await user.click(await screen.findByRole("option", { name: /src\/App\.tsx/ }));
await user.click(await screen.findByRole("button", { name: "Revert Hunk" }));
expect(githead.discardHunk).not.toHaveBeenCalled();
await user.click(screen.getByRole("button", { name: "Cancel" }));
expect(githead.discardHunk).not.toHaveBeenCalled();
await user.click(screen.getByRole("button", { name: "Revert Hunk" }));
await user.click(screen.getByRole("button", { name: "Revert changes" }));
await waitFor(() => expect(githead.discardHunk).toHaveBeenCalledWith({
repoPath, path: file.path, side: "unstaged", patch: `${diff.text}\n`, operationId: expect.any(String)
}));
expect(await screen.findByText("remaining-hunk")).toBeTruthy();
expect(githead.stageHunk).not.toHaveBeenCalled();
});

it("keeps an unstaged file selected and reloads its remaining diff after staging a hunk", async () => {
const user = userEvent.setup();
const initialFile = createStatusFile("src/App.tsx", {
Expand Down
45 changes: 40 additions & 5 deletions src/renderer/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5098,15 +5098,19 @@ export function App({ initialAppSettings = null }: { initialAppSettings?: AppSet
);
}, [runRepoOperation]);

const applySelectedHunk = useCallback(async (patch: string): Promise<void> => {
const applySelectedHunk = useCallback(async (patch: string, discard = false): Promise<void> => {
const current = stateRef.current;
const selection = current.selection;
if (!selection || current.diffChanged) {
if (!selection || current.diffChanged || (discard && selection.side !== "unstaged")) {
return;
}
const repoPath = current.repoPath;

const result = selection.side === "unstaged"
const result = discard
? await runRepoOperation("Reverting hunk", selection, (operationId) =>
window.githead.discardHunk({ repoPath, path: selection.path, side: selection.side, patch, operationId })
)
: selection.side === "unstaged"
? await runRepoOperation("Staging hunk", selection, (operationId) =>
window.githead.stageHunk({
repoPath,
Expand Down Expand Up @@ -10489,7 +10493,7 @@ function StatusView({
onUnstageFiles: (paths: string[], selection?: FileSelection) => void;
onRefreshDiff: () => void;
onDownloadImage: () => void;
onApplyHunk: (patch: string) => void;
onApplyHunk: (patch: string, discard?: boolean) => void;
onContextAction: (file: GitStatusFile, side: GitDiffSide, kind: ContextActionKind, paths?: string[]) => void;
onUpdateSubmodules: (path?: string) => void;
onSyncSubmodules: () => void;
Expand Down Expand Up @@ -10523,7 +10527,8 @@ function StatusView({
? {
side: selectedSide,
disabled: disabled || diffChanged,
onApply: onApplyHunk
onApply: onApplyHunk,
onDiscard: selectedSide === "unstaged" ? (patch) => onApplyHunk(patch, true) : undefined

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid offering hunk reversion for submodule gitlinks

When an initialized submodule is checked out at a different commit, its unstaged diff is a text hunk and this unconditionally exposes Revert Hunk. I checked git apply -h (--reverse: “apply the patch in reverse”) and reproduced the resulting 160000 gitlink patch behavior: git apply --reverse exits 0 with an unable to rmdir warning while leaving the submodule HEAD unchanged, so discardHunk reports success and the same change reappears after refresh. Gate this action when selectedFile?.submodule is present, or implement submodule checkout semantics instead of applying the gitlink patch.

Useful? React with 👍 / 👎.

}
: undefined
), [canApplyHunks, diffChanged, disabled, onApplyHunk, selectedSide]);
Expand Down Expand Up @@ -11168,6 +11173,7 @@ function DiffPanel({
}

interface DiffHunkAction {
onDiscard?: ((patch: string) => void) | undefined;
side: GitDiffSide;
disabled: boolean;
onApply: (patch: string) => void;
Expand All @@ -11184,6 +11190,8 @@ const DiffRows = memo(function DiffRows({
truncated: boolean;
hunkAction?: DiffHunkAction | undefined;
}): ReactNode {
const [discardTarget, setDiscardTarget] = useState<{ patch: string; filePath: string; text: string } | null>(null);
const discardTargetCurrent = discardTarget?.filePath === filePath && discardTarget.text === text;
const sessionRef = useRef<ReturnType<typeof createDiffProcessingSession> | null>(null);
const {
value: processedValue,
Expand Down Expand Up @@ -11233,6 +11241,21 @@ const DiffRows = memo(function DiffRows({
return (
<div className="diff-rows" ref={rootRef} aria-busy={!processed} onPointerDownCapture={onPointerDownCapture}>
{!processed ? <LoadingState label="Processing diff" className="min-h-32" /> : null}
<Dialog open={Boolean(discardTarget && discardTargetCurrent && hunkAction?.onDiscard)} onOpenChange={(open) => { if (!open) setDiscardTarget(null); }}>
<DialogContent>
<DialogHeader>
<DialogTitle>Revert this hunk?</DialogTitle>
<DialogDescription>The changes in this hunk will be reverted. Other hunks and staged changes will be kept.</DialogDescription>
</DialogHeader>
<DialogFooter>
<Button type="button" variant="outline" onClick={() => setDiscardTarget(null)}>Cancel</Button>
<Button type="button" variant="destructive" disabled={hunkAction?.disabled || !discardTargetCurrent} onClick={() => {
if (discardTarget && discardTargetCurrent && !hunkAction?.disabled) hunkAction?.onDiscard?.(discardTarget.patch);
setDiscardTarget(null);
}}>Revert changes</Button>
</DialogFooter>
</DialogContent>
</Dialog>
{groups.map((group, groupIndex) => {
const groupKey = `${groupIndex}:${group.kind}:${group.rows[0]?.text ?? ""}`;
const rowViews = group.rows.flatMap((row, rowIndex) => {
Expand All @@ -11259,6 +11282,18 @@ const DiffRows = memo(function DiffRows({
<span aria-hidden="true" />
<span className="diff-hunk-title">{formatHunkTitle(group.rows, hunkNumber)}</span>
<span className="diff-hunk-actions">
{hunkAction?.onDiscard && group.patch ? (
<Button
type="button"
variant="outline"
size="xs"
className="diff-hunk-action"
disabled={hunkAction.disabled}
onClick={() => setDiscardTarget({ patch: group.patch!, filePath, text })}
>
Revert Hunk
</Button>
) : null}
{hunkAction && group.patch ? (
<TooltipTarget content={hunkActionLabel}>
<Button
Expand Down
1 change: 1 addition & 0 deletions src/renderer/AppTestHarness.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ export function createGitheadMock(): GitheadApi {
stageFiles: vi.fn().mockResolvedValue(okOperation),
unstageFiles: vi.fn().mockResolvedValue(okOperation),
stageHunk: vi.fn().mockResolvedValue(okOperation),
discardHunk: vi.fn().mockResolvedValue(okOperation),
unstageHunk: vi.fn().mockResolvedValue(okOperation),
commitChanges: vi.fn().mockResolvedValue(okOperation),
commitWithRemoteCheck: vi.fn().mockResolvedValue({
Expand Down
48 changes: 48 additions & 0 deletions src/renderer/lineStaging.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,54 @@
import { createLinePatch, groupDiffRowsByHunk, parseUnifiedDiff, type DiffRowGroup } from "./diffParser";

describe("line staging patches", { timeout: 20_000 }, () => {
it("discards one unstaged hunk while preserving other hunks and the index", async () => {
await withRepository(async ({ repoPath, run, service }) => {
const original = Array.from({ length: 32 }, (_, index) => `value ${index + 1}`);
const filePath = path.join(repoPath, "hunks.txt");
await fs.writeFile(filePath, `${original.join("\n")}\n`);
await run(["add", "hunks.txt"]);
await run(["commit", "-m", "Add hunks"]);
const staged = [...original];
staged[15] = "staged change";
await fs.writeFile(filePath, `${staged.join("\n")}\n`);
await run(["add", "hunks.txt"]);
const changed = [...staged];
changed[1] = "first change";
changed[29] = "last change";
await fs.writeFile(filePath, `${changed.join("\n")}\n`);
const diff = await service.getFileDiff({ repoPath, path: "hunks.txt", side: "unstaged" });
const hunks = groupDiffRowsByHunk(parseUnifiedDiff(diff.text)).filter((group) => group.kind === "hunk");
expect(hunks).toHaveLength(2);
const result = await service.discardHunk({ repoPath, path: "hunks.txt", side: "unstaged", patch: hunks[0]!.patch! });
expect(result.exitCode, result.stderr).toBe(0);
changed[1] = staged[1]!;
expect(await fs.readFile(filePath, "utf8")).toBe(`${changed.join("\n")}\n`);

Check failure on line 34 in src/renderer/lineStaging.integration.test.ts

View workflow job for this annotation

GitHub Actions / Windows validation

src/renderer/lineStaging.integration.test.ts > line staging patches > discards one unstaged hunk while preserving other hunks and the index

AssertionError: expected 'value 1\r\nvalue 2\r\nvalue 3\r\nvalu…' to be 'value 1\nvalue 2\nvalue 3\nvalue 4\nv…' // Object.is equality - Expected + Received - value 1 + value 1 - value 2 + value 2 - value 3 + value 3 - value 4 + value 4 - value 5 + value 5 - value 6 + value 6 - value 7 + value 7 - value 8 + value 8 - value 9 + value 9 - value 10 + value 10 - value 11 + value 11 - value 12 + value 12 - value 13 + value 13 - value 14 + value 14 - value 15 + value 15 - staged change + staged change - value 17 + value 17 - value 18 + value 18 - value 19 + value 19 - value 20 + value 20 - value 21 + value 21 - value 22 + value 22 - value 23 + value 23 - value 24 + value 24 - value 25 + value 25 - value 26 + value 26 - value 27 + value 27 - value 28 + value 28 - value 29 + value 29 - last change + last change - value 31 + value 31 - value 32 + value 32 ❯ src/renderer/lineStaging.integration.test.ts:34:51 ❯ withRepository src/renderer/lineStaging.integration.test.ts:222:5 ❯ src/renderer/lineStaging.integration.test.ts:14:5
expect((await run(["show", ":hunks.txt"])).stdout).toBe(`${staged.join("\n")}\n`);

const stale = await service.discardHunk({ repoPath, path: "hunks.txt", side: "unstaged", patch: hunks[0]!.patch! });
expect(stale.exitCode).not.toBe(0);
expect(await fs.readFile(filePath, "utf8")).toBe(`${changed.join("\n")}\n`);
const wrongSide = await service.discardHunk({ repoPath, path: "hunks.txt", side: "staged", patch: hunks[1]!.patch! });
expect(wrongSide.exitCode).not.toBe(0);
});
});

it("restores a deleted file and removes an untracked file when discarding their hunks", async () => {
await withRepository(async ({ repoPath, run, service }) => {
await fs.rm(path.join(repoPath, "base.txt"));
await fs.writeFile(path.join(repoPath, "new.txt"), "new content\n");
for (const name of ["base.txt", "new.txt"]) {
const diff = await service.getFileDiff({ repoPath, path: name, side: "unstaged" });
const hunk = groupDiffRowsByHunk(parseUnifiedDiff(diff.text)).find((group) => group.kind === "hunk");
const result = await service.discardHunk({ repoPath, path: name, side: "unstaged", patch: hunk!.patch! });
expect(result.exitCode, result.stderr).toBe(0);
}
expect(await fs.readFile(path.join(repoPath, "base.txt"), "utf8")).toBe("base\n");

Check failure on line 55 in src/renderer/lineStaging.integration.test.ts

View workflow job for this annotation

GitHub Actions / Windows validation

src/renderer/lineStaging.integration.test.ts > line staging patches > restores a deleted file and removes an untracked file when discarding their hunks

AssertionError: expected 'base\r\n' to be 'base\n' // Object.is equality - Expected + Received - base + base ❯ src/renderer/lineStaging.integration.test.ts:55:74 ❯ withRepository src/renderer/lineStaging.integration.test.ts:222:5 ❯ src/renderer/lineStaging.integration.test.ts:46:5
await expect(fs.stat(path.join(repoPath, "new.txt"))).rejects.toThrow();
expect((await run(["status", "--porcelain"])).stdout).toBe("");
});
});

it("creates one commit from two selected hunks in one file", async () => {
await withRepository(async ({ repoPath, run, service }) => {
const original = Array.from({ length: 24 }, (_, index) => `value ${index + 1}`);
Expand Down
1 change: 1 addition & 0 deletions src/shared/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export const IPC_CHANNELS = {
stageFiles: "git:stage-files",
unstageFiles: "git:unstage-files",
stageHunk: "git:stage-hunk",
discardHunk: "git:discard-hunk",
unstageHunk: "git:unstage-hunk",
commitChanges: "git:commit-changes",
commitWithRemoteCheck: "git:commit-with-remote-check",
Expand Down
1 change: 1 addition & 0 deletions src/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2385,6 +2385,7 @@ export interface GitheadApi {
stageFiles(request: CoordinatedRequest<GitPathRequest>): Promise<GitOperationResult>;
unstageFiles(request: CoordinatedRequest<GitPathRequest>): Promise<GitOperationResult>;
stageHunk(request: CoordinatedRequest<GitHunkRequest>): Promise<GitOperationResult>;
discardHunk(request: CoordinatedRequest<GitHunkRequest>): Promise<GitOperationResult>;
unstageHunk(request: CoordinatedRequest<GitHunkRequest>): Promise<GitOperationResult>;
commitChanges(request: CoordinatedRequest<GitCommitRequest>): Promise<GitOperationResult>;
commitWithRemoteCheck(request: CoordinatedRequest<GitCommitRequest>): Promise<GitCommitWithRemoteCheckResult>;
Expand Down
Loading