feat(git): float actions beside each diff hunk

Replace the shared hunk menu with compact per-hunk controls over following context. Preserve canonical patch checks, bind controls to rendered rows, and keep EOF actions inside the code column.

Validated 19 focused tests, UI type-check, UI/web lint and the web build. Browser checks covered unified and split layouts, themes, and one-line EOF hunks.
This commit is contained in:
Bohdan Triapitsyn
2026-09-09 22:36:15 +03:00
parent d9186afdc0
commit b267b995a5
9 changed files with 386 additions and 394 deletions
@@ -1,21 +1,59 @@
import React, { act } from 'react'; import React, { act } from 'react';
import { expect } from 'bun:test'; import { expect } from 'bun:test';
import { Window } from 'happy-dom'; import { Window } from 'happy-dom';
import { Worker as NodeWorker } from 'node:worker_threads';
import type { WorkerRequest, WorkerResponse } from '@pierre/diffs/worker';
import type { GitStatus } from '@/lib/api/types'; import type { GitStatus } from '@/lib/api/types';
export async function exerciseDiffHunkActions(snapshotCase?: 'cold' | 'cached') { const WorkerEventTarget = globalThis.EventTarget;
const dom = new Window({ url: 'http://localhost' }); const WorkerMessageEvent = globalThis.MessageEvent;
// Highlighting is unrelated to patch ownership. Keep that browser I/O const workerThreads = new Set<NodeWorker>();
// pending while exercising the real view, menus and Git adapter. const pendingHighlightRequests = new Set<string>();
class PendingHighlightWorker extends EventTarget { const workerFailures: Error[] = [];
postMessage() {} const workerEntry = import.meta.resolve('@pierre/diffs/worker/worker.js');
terminate() {}
// Run the installed Pierre worker unchanged. Only its browser message boundary
// is adapted to Node, so inline slots use real rendered diff rows in this test.
class FixtureHighlightWorker extends WorkerEventTarget {
private worker = new NodeWorker(new URL(`data:text/javascript,${encodeURIComponent(`
import { parentPort, workerData } from 'node:worker_threads';
globalThis.postMessage = (data) => parentPort.postMessage(data);
globalThis.self = { addEventListener(type, listener) {
if (type === 'message') parentPort.on('message', (data) => listener({ data }));
} };
await import(workerData);
`)}`), { workerData: workerEntry });
constructor() {
super();
workerThreads.add(this.worker);
this.worker.on('message', (data: WorkerResponse) => {
pendingHighlightRequests.delete(data.id);
this.dispatchEvent(new WorkerMessageEvent('message', { data }));
});
this.worker.on('error', (error) => workerFailures.push(error));
} }
postMessage(request: WorkerRequest) {
pendingHighlightRequests.add(request.id);
this.worker.postMessage(request);
}
terminate() { return this.worker.terminate(); }
}
export const closeDiffHunkWorkers = async () => {
await Promise.all([...workerThreads].map((worker) => worker.terminate()));
workerThreads.clear();
};
export async function exerciseDiffHunkActions(snapshotCase?: 'cold' | 'cached' | 'cold-single', layout: 'inline' | 'side-by-side' = 'inline') {
const dom = new Window({ url: 'http://localhost' });
const originals = new Map<string, PropertyDescriptor | undefined>(); const originals = new Map<string, PropertyDescriptor | undefined>();
for (const [name, value] of Object.entries({ for (const [name, value] of Object.entries({
window: dom, Window: dom.Window, document: dom.document, navigator: dom.navigator, location: dom.location, localStorage: dom.localStorage, window: dom, Window: dom.Window, document: dom.document, navigator: dom.navigator, location: dom.location, localStorage: dom.localStorage,
Element: dom.Element, HTMLElement: dom.HTMLElement, HTMLInputElement: dom.HTMLInputElement, Node: dom.Node, Element: dom.Element, HTMLElement: dom.HTMLElement, HTMLInputElement: dom.HTMLInputElement, HTMLButtonElement: dom.HTMLButtonElement, Node: dom.Node,
ShadowRoot: dom.ShadowRoot, Document: dom.Document, Worker: PendingHighlightWorker, ShadowRoot: dom.ShadowRoot, Document: dom.Document, Worker: FixtureHighlightWorker,
SVGElement: dom.SVGElement, DocumentFragment: dom.DocumentFragment, Text: dom.Text, Range: dom.Range, SVGElement: dom.SVGElement, DocumentFragment: dom.DocumentFragment, Text: dom.Text, Range: dom.Range,
customElements: dom.customElements, CSSStyleSheet: dom.CSSStyleSheet, customElements: dom.customElements, CSSStyleSheet: dom.CSSStyleSheet,
Event: dom.Event, CustomEvent: dom.CustomEvent, KeyboardEvent: dom.KeyboardEvent, MouseEvent: dom.MouseEvent, Event: dom.Event, CustomEvent: dom.CustomEvent, KeyboardEvent: dom.KeyboardEvent, MouseEvent: dom.MouseEvent,
@@ -27,6 +65,7 @@ export async function exerciseDiffHunkActions(snapshotCase?: 'cold' | 'cached')
Object.defineProperty(globalThis, name, { configurable: true, writable: true, value }); Object.defineProperty(globalThis, name, { configurable: true, writable: true, value });
} }
const { createRoot } = await import('react-dom/client'); const { createRoot } = await import('react-dom/client');
document.documentElement.style.fontSize = '16px';
const originalFetch = globalThis.fetch; const originalFetch = globalThis.fetch;
// Unrelated bootstrap I/O stays pending; the Git adapter below owns this // Unrelated bootstrap I/O stays pending; the Git adapter below owns this
// scenario's reads and mutations. No real account or filesystem is touched. // scenario's reads and mutations. No real account or filesystem is touched.
@@ -38,12 +77,13 @@ export async function exerciseDiffHunkActions(snapshotCase?: 'cold' | 'cached')
const { SyncProvider } = await import('@/sync/sync-context'); const { SyncProvider } = await import('@/sync/sync-context');
const { opencodeClient } = await import('@/lib/opencode/client'); const { opencodeClient } = await import('@/lib/opencode/client');
const { useGitStore } = await import('@/stores/useGitStore'); const { useGitStore } = await import('@/stores/useGitStore');
const changes = [1, 25, 50]; const changes = snapshotCase === 'cold-single' ? [1] : [1, 25, 50];
let remaining = [...changes]; let remaining = [...changes];
let fullContext = snapshotCase === 'cold'; const cold = snapshotCase === 'cold' || snapshotCase === 'cold-single';
let currentVersion = snapshotCase === 'cold' ? 2 : 1; let fullContext = cold;
let currentVersion = cold ? 2 : 1;
let fullVersion = 1; let fullVersion = 1;
let deferVersions = snapshotCase === 'cold'; let deferVersions = cold;
let releaseFull: (() => void) | undefined; let releaseFull: (() => void) | undefined;
let releaseCanonical: (() => void) | undefined; let releaseCanonical: (() => void) | undefined;
let openedPatch: string | null = null; let openedPatch: string | null = null;
@@ -86,10 +126,12 @@ export async function exerciseDiffHunkActions(snapshotCase?: 'cold' | 'cached')
useGitStore.getState().setActiveDirectory('/repo'); useGitStore.getState().setActiveDirectory('/repo');
const container = document.createElement('div'); const container = document.createElement('div');
container.dataset.diffVirtualRoot = ''; container.dataset.diffVirtualRoot = '';
container.getBoundingClientRect = () => new dom.DOMRect(0, 0, 1280, 2000);
Object.defineProperty(container, 'clientHeight', { value: 2000 });
document.body.append(container); document.body.append(container);
const root = createRoot(container); const root = createRoot(container);
const render = () => act(async () => root.render(<I18nProvider><SyncProvider sdk={opencodeClient.getSdkClient()} directory=""><RuntimeAPIContext.Provider value={apis}> const render = () => act(async () => root.render(<I18nProvider><SyncProvider sdk={opencodeClient.getSdkClient()} directory=""><RuntimeAPIContext.Provider value={apis}>
<MultiFileDiffEntry directory="/repo" file={file} layout="inline" wrapLines={false} isSelected={false} <MultiFileDiffEntry directory="/repo" file={file} layout={layout} wrapLines={false} isSelected={false}
isExpanded isMounted onSelect={() => {}} onExpandedChange={() => {}} registerSectionRef={() => {}} isExpanded isMounted onSelect={() => {}} onExpandedChange={() => {}} registerSectionRef={() => {}}
showOpenInEditorAction onOpenInEditor={(_path, diff) => { openedPatch = diff?.patch ?? null; }} showOpenInEditorAction onOpenInEditor={(_path, diff) => { openedPatch = diff?.patch ?? null; }}
hunkActionsEnabled={!historical} loadFullFiles={fullContext} hunkActionsEnabled={!historical} loadFullFiles={fullContext}
@@ -100,19 +142,36 @@ export async function exerciseDiffHunkActions(snapshotCase?: 'cold' | 'cached')
if (!element) throw new Error(`Missing ${selector}`); if (!element) throw new Error(`Missing ${selector}`);
await act(async () => element.click()); await act(async () => element.click());
}; };
const openHunks = async () => { const waitForActions = async (count: number) => {
const trigger = container.querySelector<HTMLButtonElement>('button[aria-label="Hunks"]'); const deadline = Date.now() + 5000;
if (!trigger) throw new Error('Missing hunk trigger'); while (Date.now() < deadline) {
if (trigger.disabled) throw new Error(`Hunk trigger disabled: ${container.textContent}`); if (workerFailures.length > 0) throw workerFailures[0];
await act(async () => { if (container.querySelectorAll('[data-hunk-actions]').length === count) {
trigger.focus(); for (const [index, line] of remaining.entries()) {
trigger.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true, cancelable: true })); const target = container.querySelector<HTMLElement>(`[data-hunk-action-target="${index}"]`);
}); const wrapper = target?.parentElement;
const slots = container.querySelector('diffs-container')?.shadowRoot?.querySelectorAll('slot') ?? [];
const slot = wrapper ? [...slots].find((candidate) => candidate.assignedElements().includes(wrapper)) : undefined;
expect(slot?.name).toBe(`annotation-additions-${line + 1}`);
expect(target?.style.height).toBe('0px');
expect(slot?.closest('[data-line-annotation]')).not.toBeNull();
if (layout === 'side-by-side') expect(slot?.closest('[data-additions]')).not.toBeNull();
}
return;
}
await act(async () => { await new Promise((resolve) => setTimeout(resolve, 10)); });
}
const host = container.querySelector('diffs-container');
throw new Error(`Missing inline actions: ${JSON.stringify({
expected: count, targets: host?.querySelectorAll('[data-hunk-action-target]').length,
slots: [...(host?.shadowRoot?.querySelectorAll('slot') ?? [])].map((slot) => ({ name: slot.name, assigned: slot.assignedElements().length })),
shadow: host?.shadowRoot?.innerHTML.slice(-1500),
})}`);
}; };
try { try {
await render(); await render();
if (snapshotCase) { if (snapshotCase) {
if (snapshotCase === 'cold') { if (cold) {
if (!releaseFull || !releaseCanonical) throw new Error('Both snapshot reads must start'); if (!releaseFull || !releaseCanonical) throw new Error('Both snapshot reads must start');
await act(async () => { releaseFull?.(); releaseCanonical?.(); }); await act(async () => { releaseFull?.(); releaseCanonical?.(); });
} else { } else {
@@ -122,52 +181,62 @@ export async function exerciseDiffHunkActions(snapshotCase?: 'cold' | 'cached')
await render(); await render();
} }
expect(container.textContent).toContain('Refresh the diff and try again'); expect(container.textContent).toContain('Refresh the diff and try again');
expect(container.querySelector('button[aria-label="Hunks"]')).toBeNull(); expect(container.querySelector('[data-hunk-actions]')).toBeNull();
expect(mutations).toBe(0); expect(mutations).toBe(0);
deferVersions = false; deferVersions = false;
fullVersion = currentVersion; fullVersion = currentVersion;
const retry = Array.from(container.querySelectorAll('button')).find((button) => button.textContent === 'Retry'); const retry = Array.from(container.querySelectorAll('button')).find((button) => button.textContent === 'Retry');
if (!retry) throw new Error('Missing snapshot retry'); if (!retry) throw new Error('Missing snapshot retry');
await act(async () => retry.click()); await act(async () => retry.click());
expect(container.textContent).toContain('Hunks · 3'); await waitForActions(changes.length);
expect(container.querySelectorAll('[data-hunk-actions]')).toHaveLength(changes.length);
expect(normalReads).toBe(2); expect(normalReads).toBe(2);
await click('button[title="Open this file in editor at change"]'); await click('button[title="Open this file in editor at change"]');
expect(openedPatch).toContain('+v2-changed1\n'); expect(openedPatch).toContain('+v2-changed1\n');
await openHunks(); await click('button[aria-label="Stage hunk 1"]');
await click('[role="menuitem"][aria-label="Stage hunk 1"]');
expect(mutations).toBe(1); expect(mutations).toBe(1);
return; return;
} }
expect(container.textContent).toContain('Hunks · 3'); await waitForActions(3);
expect(container.querySelectorAll('[data-hunk-actions]')).toHaveLength(3);
expect(normalReads).toBe(1); expect(normalReads).toBe(1);
fullContext = true; fullContext = true;
await render(); await render();
expect(container.textContent).toContain('Hunks · 3'); await waitForActions(3);
expect(container.querySelectorAll('[data-hunk-actions]')).toHaveLength(3);
expect(normalReads).toBe(1); expect(normalReads).toBe(1);
await openHunks(); await click('button[aria-label="Stage hunk 1"]');
await click('[role="menuitem"][aria-label="Stage hunk 1"]');
expect(mutations).toBe(1); expect(mutations).toBe(1);
expect(container.textContent).toContain('Hunks · 2'); await waitForActions(2);
expect(container.querySelectorAll('[data-hunk-actions]')).toHaveLength(2);
expect(normalReads).toBe(2); expect(normalReads).toBe(2);
failReads = true; failReads = true;
await openHunks(); await click('button[aria-label="Stage hunk 1"]');
await click('[role="menuitem"][aria-label="Stage hunk 1"]');
expect(mutations).toBe(2); expect(mutations).toBe(2);
expect(container.textContent).toContain('Refresh unavailable'); expect(container.textContent).toContain('Refresh unavailable');
expect(container.querySelector('button[aria-label="Hunks"]')).toBeNull(); expect(container.querySelector('[data-hunk-actions]')).toBeNull();
failReads = false; failReads = false;
const retry = Array.from(container.querySelectorAll('button')).find((button) => button.textContent === 'Retry'); const retry = Array.from(container.querySelectorAll('button')).find((button) => button.textContent === 'Retry');
if (!retry) throw new Error('Missing retry'); if (!retry) throw new Error('Missing retry');
await act(async () => retry.click()); await act(async () => retry.click());
expect(container.textContent).not.toContain('Refresh unavailable'); expect(container.textContent).not.toContain('Refresh unavailable');
expect(container.querySelector('button[aria-label="Hunks"]')).toBeNull(); await waitForActions(1);
expect(container.querySelectorAll('[data-hunk-actions]')).toHaveLength(1);
await click('button[aria-label="Stage hunk 1"]');
expect(mutations).toBe(3);
expect(container.querySelector('[data-hunk-actions]')).toBeNull();
remaining = [...changes]; remaining = [...changes];
historical = true; historical = true;
await render(); await render();
expect(container.querySelector('button[aria-label="Hunks"]')).toBeNull(); expect(container.querySelector('[data-hunk-actions]')).toBeNull();
expect(mutations).toBe(2); expect(mutations).toBe(3);
} finally { } finally {
await act(async () => root.unmount()); await act(async () => root.unmount());
const deadline = Date.now() + 5000;
while (pendingHighlightRequests.size > 0 && workerFailures.length === 0 && Date.now() < deadline) {
await act(async () => { await new Promise((resolve) => setTimeout(resolve, 10)); });
}
await act(async () => { await new Promise((resolve) => requestAnimationFrame(resolve)); });
globalThis.fetch = originalFetch; globalThis.fetch = originalFetch;
for (const [name, descriptor] of originals) { for (const [name, descriptor] of originals) {
if (descriptor) Object.defineProperty(globalThis, name, descriptor); if (descriptor) Object.defineProperty(globalThis, name, descriptor);
+18 -156
View File
@@ -37,7 +37,7 @@ import { useRuntimeAPIs } from '@/hooks/useRuntimeAPIs';
import { DiffViewToggle } from '@/components/chat/message/DiffViewToggle'; import { DiffViewToggle } from '@/components/chat/message/DiffViewToggle';
import type { DiffViewMode } from '@/components/chat/message/types'; import type { DiffViewMode } from '@/components/chat/message/types';
import { ReviewFlowDialog, type ReviewFlowExecution } from '@/components/session/ReviewFlowDialog'; import { ReviewFlowDialog, type ReviewFlowExecution } from '@/components/session/ReviewFlowDialog';
import { PierreDiffViewer } from './PierreDiffViewer'; import { PierreDiffViewer, type DiffHunkActions } from './PierreDiffViewer';
import { HunkActions, type HunkBusyState, type HunkDiffAction } from './git/HunkActions'; import { HunkActions, type HunkBusyState, type HunkDiffAction } from './git/HunkActions';
import { useDeviceInfo } from '@/lib/device'; import { useDeviceInfo } from '@/lib/device';
import { FileTypeIcon } from '@/components/icons/FileTypeIcon'; import { FileTypeIcon } from '@/components/icons/FileTypeIcon';
@@ -48,7 +48,7 @@ import { sessionEvents } from '@/lib/sessionEvents';
import { findDiffScrollAnchor, getRestoredDiffScrollTop, type DiffScrollAnchor } from './diffScrollAnchor'; import { findDiffScrollAnchor, getRestoredDiffScrollTop, type DiffScrollAnchor } from './diffScrollAnchor';
import { useI18n } from '@/lib/i18n'; import { useI18n } from '@/lib/i18n';
import type { I18nKey } from '@/lib/i18n/store'; import type { I18nKey } from '@/lib/i18n/store';
import { fileDiffFromPatch, isBinaryPatch, extractHunkPatch, haveMatchingPatchVersions } from '@/lib/diff/patchFileDiff'; import { fileDiffFromPatch, isBinaryPatch, extractHunkPatch, haveMatchingPatchVersions, getPatchHunkAnchors } from '@/lib/diff/patchFileDiff';
import { isVSCodeRuntime } from '@/lib/desktop'; import { isVSCodeRuntime } from '@/lib/desktop';
import { startReviewFlow } from '@/lib/reviewFlow'; import { startReviewFlow } from '@/lib/reviewFlow';
import { WALKTHROUGH_ACTION_CLASS } from '@/components/views/walkthrough/walkthroughAction'; import { WALKTHROUGH_ACTION_CLASS } from '@/components/views/walkthrough/walkthroughAction';
@@ -465,6 +465,7 @@ interface InlineDiffViewerProps {
diff: DiffData; diff: DiffData;
renderSideBySide: boolean; renderSideBySide: boolean;
wrapLines: boolean; wrapLines: boolean;
hunkActions?: DiffHunkActions;
} }
const InlineDiffViewer = React.memo<InlineDiffViewerProps>(({ const InlineDiffViewer = React.memo<InlineDiffViewerProps>(({
@@ -472,6 +473,7 @@ const InlineDiffViewer = React.memo<InlineDiffViewerProps>(({
diff, diff,
renderSideBySide, renderSideBySide,
wrapLines, wrapLines,
hunkActions,
}) => { }) => {
const language = React.useMemo( const language = React.useMemo(
() => getLanguageFromExtension(filePath) || 'text', () => getLanguageFromExtension(filePath) || 'text',
@@ -503,104 +505,12 @@ const InlineDiffViewer = React.memo<InlineDiffViewerProps>(({
renderSideBySide={renderSideBySide} renderSideBySide={renderSideBySide}
wrapLines={wrapLines} wrapLines={wrapLines}
layout="inline" layout="inline"
hunkActions={hunkActions}
/> />
</div> </div>
); );
}); });
type FileDiffAction = 'stage' | 'unstage' | 'discard';
interface FileDiffActionsProps {
filePath: string;
staged: boolean;
busyAction: FileDiffAction | null;
disabled: boolean;
onAction: (action: FileDiffAction) => void;
}
const FileDiffActions = React.memo<FileDiffActionsProps>(({
filePath,
staged,
busyAction,
disabled,
onAction,
}) => {
const { t } = useI18n();
return (
<div className="flex items-center gap-0.5 rounded-full border border-[var(--interactive-border)]/45 bg-[var(--surface-background)]/95 px-1 py-0.5 shadow-sm backdrop-blur-md">
{staged ? (
<FileDiffActionButton
label={t('gitView.changes.unstageFileAria', { path: filePath })}
icon="arrow-go-back"
loading={busyAction === 'unstage'}
disabled={disabled}
onClick={() => onAction('unstage')}
/>
) : (
<>
<FileDiffActionButton
label={t('gitView.changes.revertFileAria', { path: filePath })}
icon="arrow-go-back"
loading={busyAction === 'discard'}
disabled={disabled}
tone="failure"
onClick={() => onAction('discard')}
/>
<FileDiffActionButton
label={t('gitView.changes.stageFileAria', { path: filePath })}
icon="add"
loading={busyAction === 'stage'}
disabled={disabled}
tone="success"
onClick={() => onAction('stage')}
/>
</>
)}
</div>
);
});
interface FileDiffActionButtonProps {
label: string;
icon: 'add' | 'arrow-go-back';
loading: boolean;
disabled: boolean;
tone?: 'failure' | 'success';
onClick: () => void;
}
const FileDiffActionButton: React.FC<FileDiffActionButtonProps> = ({
label,
icon,
loading,
disabled,
tone,
onClick,
}) => (
<Button
variant="ghost"
size="sm"
className={cn(
'h-6 w-6 rounded-none bg-transparent p-0 text-muted-foreground opacity-70 hover:bg-transparent hover:text-foreground hover:opacity-100',
tone === 'failure' && 'text-[var(--status-error)] hover:text-[var(--status-error)]',
tone === 'success' && 'text-[var(--status-success)] hover:text-[var(--status-success)]'
)}
disabled={disabled}
title={label}
aria-label={label}
onClick={(event) => {
event.stopPropagation();
onClick();
}}
>
{loading ? (
<Icon name="loader-4" className="size-3.5 animate-spin" />
) : (
<Icon name={icon} className={icon === 'add' ? 'size-4' : 'size-3.5'} />
)}
</Button>
);
interface MultiFileDiffEntryProps { interface MultiFileDiffEntryProps {
directory: string; directory: string;
file: FileEntry; file: FileEntry;
@@ -664,7 +574,6 @@ export const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
const [isFetching, setIsLoading] = React.useState(false); const [isFetching, setIsLoading] = React.useState(false);
const diffLoadError = comparisonDiff ? (comparisonDiff.status === 'error' ? comparisonDiff.message : null) : localDiffLoadError; const diffLoadError = comparisonDiff ? (comparisonDiff.status === 'error' ? comparisonDiff.message : null) : localDiffLoadError;
const isLoading = comparisonDiff ? comparisonDiff.status === 'loading' : isFetching; const isLoading = comparisonDiff ? comparisonDiff.status === 'loading' : isFetching;
const [fileAction, setFileAction] = React.useState<FileDiffAction | null>(null);
const [hunkAction, setHunkAction] = React.useState<HunkBusyState>(null); const [hunkAction, setHunkAction] = React.useState<HunkBusyState>(null);
const mutationInFlight = React.useRef(false); const mutationInFlight = React.useRef(false);
const [canonicalPatch, setCanonicalPatch] = React.useState<{ scope: string; patch: string } | null>(null); const [canonicalPatch, setCanonicalPatch] = React.useState<{ scope: string; patch: string } | null>(null);
@@ -757,8 +666,8 @@ export const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
if (cancelled || runtimeKey !== getRuntimeKey()) return; if (cancelled || runtimeKey !== getRuntimeKey()) return;
const patch = 'diff' in response && !loadFullFiles ? response.diff : normalPatch; const patch = 'diff' in response && !loadFullFiles ? response.diff : normalPatch;
if (hunkEligible && loadFullFiles && patch !== null && (patch.match(/^@@\s/gm)?.length ?? 0) > 1 if (hunkEligible && loadFullFiles && patch !== null && /^@@\s/m.test(patch)
&& ('diff' in response && !haveMatchingPatchVersions(response.diff, patch))) { && ('diff' in response && response.diff !== patch && !haveMatchingPatchVersions(response.diff, patch))) {
setCanonicalPatch(null); setCanonicalPatch(null);
throw new Error(t('diffView.hunk.unavailable')); throw new Error(t('diffView.hunk.unavailable'));
} }
@@ -816,41 +725,8 @@ export const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
setDiffRetryNonce((nonce) => nonce + 1); setDiffRetryNonce((nonce) => nonce + 1);
}, []); }, []);
const handleFileAction = React.useCallback(async (action: FileDiffAction) => {
if (!directory || mutationInFlight.current || fileAction !== null) {
return;
}
mutationInFlight.current = true;
const runtimeKey = getRuntimeKey();
setFileAction(action);
try {
if (action === 'stage') {
await git.stageGitFile(directory, file.path);
} else if (action === 'unstage') {
await git.unstageGitFile(directory, file.path);
} else {
await git.revertGitFile(directory, file.path, { scope: 'working' });
}
if (runtimeKey !== getRuntimeKey()) return;
invalidatePatch();
sessionEvents.requestGitRefresh({ directory, paths: [file.path] });
await fetchStatus(directory, git);
} catch (error) {
const fallbackKey = action === 'unstage'
? 'gitView.toast.unstageFileFailed'
: action === 'stage'
? 'gitView.toast.stageFileFailed'
: 'gitView.toast.revertFailed';
toast.error(error instanceof Error ? error.message : t(fallbackKey));
} finally {
mutationInFlight.current = false;
setFileAction((current) => (current === action ? null : current));
}
}, [directory, fetchStatus, file.path, fileAction, git, invalidatePatch, t]);
const handleHunkAction = React.useCallback(async (hunkIndex: number, action: HunkDiffAction) => { const handleHunkAction = React.useCallback(async (hunkIndex: number, action: HunkDiffAction) => {
if (!directory || !hunkEligible || isLoading || diffLoadError || mutationInFlight.current || hunkAction !== null || fileAction !== null) { if (!directory || !hunkEligible || isLoading || diffLoadError || mutationInFlight.current || hunkAction !== null) {
return; return;
} }
@@ -887,7 +763,15 @@ export const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
mutationInFlight.current = false; mutationInFlight.current = false;
setHunkAction((current) => (current?.index === hunkIndex && current.action === action ? null : current)); setHunkAction((current) => (current?.index === hunkIndex && current.action === action ? null : current));
} }
}, [actionPatch, hunkEligible, isLoading, diffLoadError, directory, fetchStatus, file.path, fileAction, git, hunkAction, invalidatePatch, staged, t]); }, [actionPatch, hunkEligible, isLoading, diffLoadError, directory, fetchStatus, file.path, git, hunkAction, invalidatePatch, staged, t]);
const hunkAnchors = React.useMemo(() => hunkEligible && actionPatch !== null ? getPatchHunkAnchors(actionPatch) : [], [actionPatch, hunkEligible]);
const renderHunkActions = React.useCallback((index: number) => (
<HunkActions index={index} staged={staged} busyHunk={hunkAction}
disabled={isLoading || Boolean(diffLoadError)} onAction={handleHunkAction} />
), [diffLoadError, handleHunkAction, hunkAction, isLoading, staged]);
const diffHunkActions = React.useMemo<DiffHunkActions | undefined>(() => hunkAnchors.length > 0
? { anchors: hunkAnchors, render: renderHunkActions } : undefined, [hunkAnchors, renderHunkActions]);
return ( return (
<div ref={setSectionRef} className="scroll-mt-9 border-b border-[var(--interactive-border)]/40 last:border-b-0"> <div ref={setSectionRef} className="scroll-mt-9 border-b border-[var(--interactive-border)]/40 last:border-b-0">
@@ -1053,30 +937,8 @@ export const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
diff={diffData} diff={diffData}
renderSideBySide={renderSideBySide} renderSideBySide={renderSideBySide}
wrapLines={wrapLines} wrapLines={wrapLines}
hunkActions={diffHunkActions}
/> />
<div className="pointer-events-none absolute bottom-3 right-3 z-20">
<div className="pointer-events-auto flex items-center gap-1.5">
{hunkEligible && actionPatch !== null ? (
<HunkActions
filePath={file.path}
patch={actionPatch}
staged={staged}
busyHunk={hunkAction}
disabled={isLoading || Boolean(diffLoadError) || fileAction !== null || hunkAction !== null}
onAction={handleHunkAction}
/>
) : null}
{!readOnlyActions ? (
<FileDiffActions
filePath={file.path}
staged={staged}
busyAction={fileAction}
disabled={fileAction !== null || hunkAction !== null}
onAction={handleFileAction}
/>
) : null}
</div>
</div>
</> </>
) : null} ) : null}
</div> </div>
@@ -1,4 +1,5 @@
import React, { useMemo, useRef, useCallback, useEffect } from 'react'; import React, { useMemo, useRef, useCallback, useEffect } from 'react';
import { createPortal } from 'react-dom';
import { import {
areFilesEqual, areFilesEqual,
areOptionsEqual, areOptionsEqual,
@@ -29,6 +30,20 @@ import { getDefaultTheme } from '@/lib/theme/themes';
import { useDeviceInfo } from '@/lib/device'; import { useDeviceInfo } from '@/lib/device';
import { cn } from '@/lib/utils'; import { cn } from '@/lib/utils';
import type { PatchHunkAnchor } from '@/lib/diff/patchFileDiff';
export interface DiffHunkActions {
anchors: readonly PatchHunkAnchor[];
render: (index: number) => React.ReactNode;
}
type DiffAnnotation = PierreAnnotationData | { type: 'hunk-action'; index: number };
const EMPTY_HUNK_ANCHORS: readonly PatchHunkAnchor[] = [];
const HUNK_ACTION_OVERLAY_CSS = `
[data-gutter-buffer="annotation"] { min-height: 0; }
[data-code] { min-height: 2.5rem; align-content: start; }
`;
// Threshold (bytes) above which syntax highlighting is degraded for performance // Threshold (bytes) above which syntax highlighting is degraded for performance
@@ -44,6 +59,7 @@ interface PierreDiffViewerProps {
wrapLines?: boolean; wrapLines?: boolean;
layout?: 'fill' | 'inline'; layout?: 'fill' | 'inline';
enableComments?: boolean; enableComments?: boolean;
hunkActions?: DiffHunkActions;
} }
/** /**
@@ -444,7 +460,7 @@ function acquireSharedVirtualizer(container: HTMLElement): SharedVirtualizer | n
} }
const wakeVirtualizer = ( const wakeVirtualizer = (
instance: PierreFileDiff<PierreAnnotationData>, instance: PierreFileDiff<DiffAnnotation>,
sharedVirtualizer: SharedVirtualizer | null, sharedVirtualizer: SharedVirtualizer | null,
forceUpdate: () => void, forceUpdate: () => void,
): (() => void) => { ): (() => void) => {
@@ -491,6 +507,7 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
wrapLines, wrapLines,
layout = 'fill', layout = 'fill',
enableComments = true, enableComments = true,
hunkActions,
}) => { }) => {
const themeContext = useOptionalThemeSystem(); const themeContext = useOptionalThemeSystem();
@@ -499,6 +516,12 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
const darkTheme = themeContext?.availableThemes.find(t => t.metadata.id === themeContext.darkThemeId) ?? getDefaultTheme(true); const darkTheme = themeContext?.availableThemes.find(t => t.metadata.id === themeContext.darkThemeId) ?? getDefaultTheme(true);
const { isMobile } = useDeviceInfo(); const { isMobile } = useDeviceInfo();
const hunkAnchors = hunkActions?.anchors ?? EMPTY_HUNK_ANCHORS;
const [hunkTargets, setHunkTargets] = React.useState<{
fileDiff: FileDiffMetadata | undefined;
anchors: readonly PatchHunkAnchor[];
targets: ReadonlyMap<number, HTMLElement>;
}>(() => ({ fileDiff: undefined, anchors: EMPTY_HUNK_ANCHORS, targets: new Map() }));
const diffCommentController = useInlineCommentController<SelectedLineRange>({ const diffCommentController = useInlineCommentController<SelectedLineRange>({
source: 'diff', source: 'diff',
@@ -587,10 +610,16 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
cancel(); cancel();
}, [cancel]); }, [cancel]);
const renderAnnotation = useCallback((annotation: DiffLineAnnotation<PierreAnnotationData>) => { const renderAnnotation = useCallback((annotation: DiffLineAnnotation<DiffAnnotation>) => {
const div = document.createElement('div'); const div = document.createElement('div');
div.style.position = 'relative'; div.style.position = 'relative';
if (annotation.metadata.type === 'hunk-action') {
div.dataset.hunkActionTarget = String(annotation.metadata.index);
div.style.height = '0px';
return div;
}
const id = toPierreAnnotationId(annotation.metadata); const id = toPierreAnnotationId(annotation.metadata);
div.dataset.annotationId = id; div.dataset.annotationId = id;
@@ -599,6 +628,52 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
return div; return div;
}, []); }, []);
const captureHunkTargets = useCallback<NonNullable<FileDiffOptions<DiffAnnotation>['onPostRender']>>((node, instance, phase) => {
const targets = new Map<number, HTMLElement>();
if (phase !== 'unmount') {
const capsuleHeight = Number.parseFloat(getComputedStyle(document.documentElement).fontSize) * 2;
const columns = new Map<HTMLElement, DOMRect>();
const placements: Array<{ target: HTMLElement; offset: number }> = [];
// Only mounted virtual rows have slots. Avoid creating React controls
// for off-screen hunks or measuring every line on a scroll event.
for (const slot of node.shadowRoot?.querySelectorAll('slot') ?? []) {
for (const wrapper of slot.assignedElements()) {
const target = wrapper.querySelector<HTMLElement>('[data-hunk-action-target]');
const index = Number(target?.dataset.hunkActionTarget);
if (!target || !Number.isInteger(index) || index < 0) continue;
targets.set(index, target);
const column = slot.closest<HTMLElement>('[data-code]');
if (!column) continue;
let bounds = columns.get(column);
if (!bounds) {
bounds = column.getBoundingClientRect();
columns.set(column, bounds);
}
const markerTop = target.getBoundingClientRect().top;
// Float over the following context. At EOF, lift the capsule inside
// the code column so its vertical clipping cannot hide the buttons.
const top = Math.max(bounds.top + 4, Math.min(markerTop + 4, bounds.bottom - capsuleHeight - 4));
placements.push({ target, offset: top - markerTop });
}
}
// Finish all geometry reads before writing offsets to avoid layout
// recalculation between neighboring hunks.
for (const { target, offset } of placements) {
const value = `${offset}px`;
if (target.style.getPropertyValue('--oc-hunk-action-offset') !== value) {
target.style.setProperty('--oc-hunk-action-offset', value);
}
}
}
const renderedDiff = instance.fileDiff;
setHunkTargets((previous) => {
if (previous.fileDiff === renderedDiff && previous.anchors === hunkAnchors
&& previous.targets.size === targets.size
&& [...targets].every(([index, target]) => previous.targets.get(index) === target)) return previous;
return { fileDiff: renderedDiff, anchors: hunkAnchors, targets };
});
}, [hunkAnchors]);
const handleSaveComment = useCallback((textToSave: string, rangeOverride?: SelectedLineRange) => { const handleSaveComment = useCallback((textToSave: string, rangeOverride?: SelectedLineRange) => {
saveComment(textToSave, rangeOverride ?? selection ?? undefined); saveComment(textToSave, rangeOverride ?? selection ?? undefined);
}, [saveComment, selection]); }, [saveComment, selection]);
@@ -851,11 +926,11 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
const diffRootRef = useRef<HTMLDivElement | null>(null); const diffRootRef = useRef<HTMLDivElement | null>(null);
const diffContainerRef = useRef<HTMLDivElement | null>(null); const diffContainerRef = useRef<HTMLDivElement | null>(null);
const diffInstanceRef = useRef<PierreFileDiff<PierreAnnotationData> | null>(null); const diffInstanceRef = useRef<PierreFileDiff<DiffAnnotation> | null>(null);
const sharedVirtualizerRef = useRef<SharedVirtualizer | null>(null); const sharedVirtualizerRef = useRef<SharedVirtualizer | null>(null);
const instanceVirtualizerRef = useRef<Virtualizer | null>(null); const instanceVirtualizerRef = useRef<Virtualizer | null>(null);
const instanceWorkerPoolRef = useRef<unknown>(null); const instanceWorkerPoolRef = useRef<unknown>(null);
const instanceVirtualHunkSeparatorsRef = useRef<FileDiffOptions<PierreAnnotationData>['hunkSeparators'] | undefined>(undefined); const instanceVirtualHunkSeparatorsRef = useRef<FileDiffOptions<DiffAnnotation>['hunkSeparators'] | undefined>(undefined);
const instanceFileDiffRef = useRef<FileDiffMetadata | undefined>(undefined); const instanceFileDiffRef = useRef<FileDiffMetadata | undefined>(undefined);
const instanceOldFileRef = useRef<FileContents | undefined>(undefined); const instanceOldFileRef = useRef<FileContents | undefined>(undefined);
const instanceNewFileRef = useRef<FileContents | undefined>(undefined); const instanceNewFileRef = useRef<FileContents | undefined>(undefined);
@@ -940,46 +1015,47 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
}, [darkResolvedTheme, diffThemeKey, isDark, lightResolvedTheme]); }, [darkResolvedTheme, diffThemeKey, isDark, lightResolvedTheme]);
const options = useMemo(() => ({ const options = useMemo<FileDiffOptions<DiffAnnotation>>(() => ({
theme: { theme: {
dark: darkTheme.metadata.id, dark: darkTheme.metadata.id,
light: lightTheme.metadata.id, light: lightTheme.metadata.id,
}, },
themeType: isDark ? ('dark' as const) : ('light' as const), themeType: isDark ? 'dark' : 'light',
diffStyle: renderSideBySide ? ('split' as const) : ('unified' as const), diffStyle: renderSideBySide ? 'split' : 'unified',
diffIndicators: 'none' as const, diffIndicators: 'none',
hunkSeparators: 'line-info-basic' as const, hunkSeparators: 'line-info-basic',
// Perf: disable intra-line diff (word-level) globally. // Perf: disable intra-line diff (word-level) globally.
lineDiffType: 'none' as const, lineDiffType: 'none',
// Perf: degrade tokenization/highlighting for large files (>500KB) // Perf: degrade tokenization/highlighting for large files (>500KB)
maxLineDiffLength: isLargeContent ? 0 : 1000, maxLineDiffLength: isLargeContent ? 0 : 1000,
maxLineLengthForHighlighting: isLargeContent ? 1 : 1000, maxLineLengthForHighlighting: isLargeContent ? 1 : 1000,
tokenizeMaxLineLength: isLargeContent ? 1 : 1000, tokenizeMaxLineLength: isLargeContent ? 1 : 1000,
collapsedContextThreshold: 0, collapsedContextThreshold: 0,
expansionLineCount: 20, expansionLineCount: 20,
overflow: wrapLines ? ('wrap' as const) : ('scroll' as const), overflow: wrapLines ? 'wrap' : 'scroll',
disableFileHeader: true, disableFileHeader: true,
enableLineSelection: enableComments, enableLineSelection: enableComments,
enableGutterUtility: enableComments, enableGutterUtility: enableComments,
onGutterUtilityClick: enableComments ? handleGutterUtilityClick : undefined, onGutterUtilityClick: enableComments ? handleGutterUtilityClick : undefined,
onLineClick: enableComments ? handleLineClick : undefined, onLineClick: enableComments ? handleLineClick : undefined,
onLineSelected: enableComments ? handleSelectionChange : undefined, onLineSelected: enableComments ? handleSelectionChange : undefined,
unsafeCSS: WEBKIT_SCROLL_FIX_CSS, unsafeCSS: hunkAnchors.length > 0 ? `${WEBKIT_SCROLL_FIX_CSS}\n${HUNK_ACTION_OVERLAY_CSS}` : WEBKIT_SCROLL_FIX_CSS,
renderAnnotation: enableComments ? renderAnnotation : undefined, renderAnnotation: enableComments || hunkAnchors.length > 0 ? renderAnnotation : undefined,
}), [darkTheme.metadata.id, enableComments, isDark, isLargeContent, lightTheme.metadata.id, renderSideBySide, wrapLines, handleSelectionChange, handleGutterUtilityClick, handleLineClick, renderAnnotation]); onPostRender: hunkAnchors.length > 0 ? captureHunkTargets : undefined,
}), [captureHunkTargets, hunkAnchors.length, darkTheme.metadata.id, enableComments, isDark, isLargeContent, lightTheme.metadata.id, renderSideBySide, wrapLines, handleSelectionChange, handleGutterUtilityClick, handleLineClick, renderAnnotation]);
const lineAnnotations = useMemo(() => { const lineAnnotations = useMemo<DiffLineAnnotation<DiffAnnotation>[]>(() => {
if (!enableComments) { const annotations: DiffLineAnnotation<DiffAnnotation>[] = enableComments ? buildPierreLineAnnotations({
return [];
}
return buildPierreLineAnnotations({
drafts: fileDrafts, drafts: fileDrafts,
editingDraftId, editingDraftId,
selection, selection,
}); }) : [];
}, [editingDraftId, enableComments, fileDrafts, selection]); for (const anchor of hunkAnchors) {
annotations.push({ side: anchor.side, lineNumber: anchor.lineNumber, metadata: { type: 'hunk-action', index: anchor.index } });
}
return annotations;
}, [editingDraftId, enableComments, fileDrafts, hunkAnchors, selection]);
const lineAnnotationsRef = useRef(lineAnnotations); const lineAnnotationsRef = useRef(lineAnnotations);
@@ -1068,17 +1144,17 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
: false; : false;
if (!instance) { if (!instance) {
instance = sharedVirtualizer instance = sharedVirtualizer
? new VirtualizedFileDiff<PierreAnnotationData>( ? new VirtualizedFileDiff<DiffAnnotation>(
options as FileDiffOptions<PierreAnnotationData>, options,
sharedVirtualizer.virtualizer, sharedVirtualizer.virtualizer,
VIRTUAL_METRICS, VIRTUAL_METRICS,
workerPool, workerPool,
) )
: new PierreFileDiff(options as FileDiffOptions<PierreAnnotationData>, workerPool); : new PierreFileDiff(options, workerPool);
diffInstanceRef.current = instance; diffInstanceRef.current = instance;
lastAppliedSelectionRef.current = null; lastAppliedSelectionRef.current = null;
} else { } else {
instance.setOptions(options as FileDiffOptions<PierreAnnotationData>); instance.setOptions(options);
} }
instanceVirtualizerRef.current = virtualizer; instanceVirtualizerRef.current = virtualizer;
@@ -1291,6 +1367,12 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
/> />
) : null; ) : null;
// A new action snapshot must never land in annotation nodes belonging to
// the previously rendered diff, even for one frame before Pierre updates.
const hunkActionPortals = hunkActions && fileDiff && hunkTargets.fileDiff === fileDiff && hunkTargets.anchors === hunkAnchors
? [...hunkTargets.targets].map(([index, target]) => createPortal(hunkActions.render(index), target, `hunk-${index}`))
: null;
if (layout === 'fill') { if (layout === 'fill') {
return ( return (
<div className={cn("flex flex-col relative", "size-full")} data-diff-virtual-root> <div className={cn("flex flex-col relative", "size-full")} data-diff-virtual-root>
@@ -1306,6 +1388,7 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
</div> </div>
</ScrollableOverlay> </ScrollableOverlay>
{commentOverlays} {commentOverlays}
{hunkActionPortals}
</div> </div>
</div> </div>
); );
@@ -1318,6 +1401,7 @@ export const PierreDiffViewer: React.FC<PierreDiffViewerProps> = ({
<div ref={diffContainerRef} className="w-full" /> <div ref={diffContainerRef} className="w-full" />
</div> </div>
{commentOverlays} {commentOverlays}
{hunkActionPortals}
</div> </div>
); );
}; };
@@ -1,18 +1,15 @@
import React, { act } from 'react'; import React, { act } from 'react';
import { expect, test } from 'bun:test'; import { expect, test } from 'bun:test';
import { Window } from 'happy-dom'; import { Window } from 'happy-dom';
import type { HunkBusyState } from './HunkActions';
test('a long hunk menu counts header-like content and navigates to its last action', async () => { test('each compact capsule acts on its own hunk and shares the mutation lock', async () => {
const dom = new Window({ url: 'http://localhost' }); const dom = new Window({ url: 'http://localhost' });
const originals = new Map<string, PropertyDescriptor | undefined>(); const originals = new Map<string, PropertyDescriptor | undefined>();
for (const [name, value] of Object.entries({ for (const [name, value] of Object.entries({
window: dom, Window: dom.Window, document: dom.document, navigator: dom.navigator, window: dom, document: dom.document, navigator: dom.navigator,
Element: dom.Element, HTMLElement: dom.HTMLElement, Node: dom.Node, ShadowRoot: dom.ShadowRoot, Element: dom.Element, HTMLElement: dom.HTMLElement, Node: dom.Node,
customElements: dom.customElements, CSSStyleSheet: dom.CSSStyleSheet, Event: dom.Event, MouseEvent: dom.MouseEvent, IS_REACT_ACT_ENVIRONMENT: true,
Event: dom.Event, CustomEvent: dom.CustomEvent, KeyboardEvent: dom.KeyboardEvent,
MouseEvent: dom.MouseEvent, ResizeObserver: dom.ResizeObserver,
getComputedStyle: dom.getComputedStyle.bind(dom), requestAnimationFrame: dom.requestAnimationFrame.bind(dom),
cancelAnimationFrame: dom.cancelAnimationFrame.bind(dom), IS_REACT_ACT_ENVIRONMENT: true,
})) { })) {
originals.set(name, Object.getOwnPropertyDescriptor(globalThis, name)); originals.set(name, Object.getOwnPropertyDescriptor(globalThis, name));
Object.defineProperty(globalThis, name, { configurable: true, writable: true, value }); Object.defineProperty(globalThis, name, { configurable: true, writable: true, value });
@@ -20,38 +17,39 @@ test('a long hunk menu counts header-like content and navigates to its last acti
const { createRoot } = await import('react-dom/client'); const { createRoot } = await import('react-dom/client');
const { I18nProvider } = await import('@/lib/i18n'); const { I18nProvider } = await import('@/lib/i18n');
const { HunkActions } = await import('./HunkActions'); const { HunkActions } = await import('./HunkActions');
const patch = 'diff --git a/f b/f\n--- a/f\n+++ b/f\n' + Array.from({ length: 60 }, (_, i) => `@@ -${i * 10 + 1} +${i * 10 + 1} @@\n--- a/old\n+++ b/new\n`).join('');
const actions: string[] = [];
const container = document.createElement('div'); const container = document.createElement('div');
document.body.append(container); document.body.append(container);
const root = createRoot(container); const root = createRoot(container);
const key = async (name: string, ctrlKey = false) => act(async () => { const actions: string[] = [];
document.activeElement?.dispatchEvent(new KeyboardEvent('keydown', { key: name, ctrlKey, bubbles: true, cancelable: true })); const render = (staged = false, busyHunk: HunkBusyState = null) => act(async () => root.render(
}); <I18nProvider>{[0, 1, 2].map((index) => <HunkActions key={index} index={index} staged={staged}
busyHunk={busyHunk} disabled={false} onAction={(hunk, action) => actions.push(`${action}:${hunk}`)} />)}</I18nProvider>
));
const button = (label: string) => {
const target = container.querySelector<HTMLButtonElement>(`button[aria-label="${label}"]`);
if (!target) throw new Error(`Missing ${label}`);
return target;
};
try { try {
await act(async () => root.render(<I18nProvider><HunkActions filePath="f" patch={patch} staged={false} busyHunk={null} disabled={false} onAction={(index, action) => actions.push(`${action}:${index}`)} /></I18nProvider>)); await render();
const trigger = container.querySelector<HTMLButtonElement>('button[aria-label="Hunks"]'); expect(container.querySelectorAll('[data-hunk-actions]')).toHaveLength(3);
if (!trigger) throw new Error('Missing hunk trigger'); expect(container.querySelector('[role="menu"]')).toBeNull();
trigger.focus(); expect(button('Stage hunk 2').closest('[data-hunk-actions]')?.getAttribute('data-hunk-actions')).toBe('1');
await key('ArrowDown'); await act(async () => button('Stage hunk 2').click());
const menu = document.querySelector<HTMLElement>('[role="menu"]'); await act(async () => button('Discard hunk 3').click());
if (!menu) throw new Error('Missing hunk menu'); expect(actions).toEqual(['stage:1', 'discard:2']);
expect(menu.className).toContain('overflow-y-auto');
expect(menu.className).toContain('max-h-'); await render(false, { index: 1, action: 'stage' });
const first = menu.querySelector('[aria-label="Stage hunk 1"]'); expect([...container.querySelectorAll('button')].every((entry) => entry.disabled)).toBe(true);
expect(first?.textContent).toContain('+1'); expect(button('Stage hunk 2').querySelector('.animate-spin')).not.toBeNull();
expect(first?.textContent).toContain('-1'); await act(async () => button('Discard hunk 1').click());
await key('End'); expect(actions).toHaveLength(2);
expect(document.activeElement?.getAttribute('aria-label')).toBe('Discard hunk 60');
await key('p', true); await render(true);
expect(document.activeElement?.getAttribute('aria-label')).toBe('Stage hunk 60'); expect(container.querySelectorAll('button')).toHaveLength(3);
await key('n', true); expect(container.querySelector('button[aria-label="Stage hunk 1"]')).toBeNull();
expect(document.activeElement?.getAttribute('aria-label')).toBe('Discard hunk 60'); await act(async () => button('Unstage hunk 1').click());
expect(document.activeElement?.getAttribute('data-variant')).toBe('destructive'); expect(actions.at(-1)).toBe('unstage:0');
const last = document.activeElement;
if (!(last instanceof HTMLElement)) throw new Error('Missing final action');
await act(async () => last.click());
expect(actions).toEqual(['discard:59']);
} finally { } finally {
await act(async () => root.unmount()); await act(async () => root.unmount());
for (const [name, descriptor] of originals) { for (const [name, descriptor] of originals) {
@@ -1,17 +1,8 @@
import React, { useMemo } from 'react'; import React from 'react';
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuLabel,
DropdownMenuSeparator,
DropdownMenuTrigger,
} from '@/components/ui/dropdown-menu';
import { Icon } from '@/components/icon/Icon'; import { Icon } from '@/components/icon/Icon';
import { Button } from '@/components/ui/button'; import { Button } from '@/components/ui/button';
import { dropdownTriggerVariants } from '@/components/ui/dropdown-trigger';
import { useI18n } from '@/lib/i18n'; import { useI18n } from '@/lib/i18n';
import { splitPatchIntoHunks } from '@/lib/diff/patchFileDiff'; import { cn } from '@/lib/utils';
export type HunkDiffAction = 'stage' | 'unstage' | 'discard'; export type HunkDiffAction = 'stage' | 'unstage' | 'discard';
@@ -21,136 +12,50 @@ export type HunkBusyState = {
} | null; } | null;
interface HunkActionsProps { interface HunkActionsProps {
filePath: string; index: number;
patch: string;
staged: boolean; staged: boolean;
busyHunk: HunkBusyState; busyHunk: HunkBusyState;
disabled: boolean; disabled: boolean;
onAction: (hunkIndex: number, action: HunkDiffAction) => void; onAction: (hunkIndex: number, action: HunkDiffAction) => void;
} }
interface HunkSummary {
insertions: number;
deletions: number;
}
const summarizeHunks = (patch: string): HunkSummary[] =>
splitPatchIntoHunks(patch).map((hunkPatch) => {
let insertions = 0;
let deletions = 0;
let inBody = false;
for (const line of hunkPatch.split('\n')) {
if (line.startsWith('@@ ')) { inBody = true; continue; }
if (!inBody) continue;
if (line.startsWith('+')) insertions += 1;
else if (line.startsWith('-')) deletions += 1;
}
return { insertions, deletions };
});
const HunkCounts = React.memo<{ insertions: number; deletions: number }>(function HunkCounts({
insertions,
deletions,
}) {
if (insertions === 0 && deletions === 0) return null;
return (
<span className="ml-auto pl-3 typography-micro">
{insertions > 0 ? (
<span style={{ color: 'var(--status-success)' }}>+{insertions}</span>
) : null}
{insertions > 0 && deletions > 0 ? (
<span className="mx-0.5 text-muted-foreground">/</span>
) : null}
{deletions > 0 ? (
<span style={{ color: 'var(--status-error)' }}>-{deletions}</span>
) : null}
</span>
);
});
export const HunkActions = React.memo<HunkActionsProps>(function HunkActions({ export const HunkActions = React.memo<HunkActionsProps>(function HunkActions({
filePath, index, staged, busyHunk, disabled, onAction,
patch,
staged,
busyHunk,
disabled,
onAction,
}) { }) {
const { t } = useI18n(); const { t } = useI18n();
const hunks = useMemo(() => summarizeHunks(patch), [patch]); const actions: HunkDiffAction[] = staged ? ['unstage'] : ['discard', 'stage'];
if (hunks.length < 2) {
return null;
}
const primaryAction: HunkDiffAction = staged ? 'unstage' : 'stage';
return ( return (
<DropdownMenu> <div className="pointer-events-none absolute right-3 z-20" style={{ top: 'var(--oc-hunk-action-offset, 0.25rem)' }} data-hunk-actions={index}>
<DropdownMenuTrigger asChild> <div className="pointer-events-auto flex items-center gap-0.5 rounded-full border border-[var(--interactive-border)]/45 bg-[var(--surface-background)]/95 px-1 py-0.5 shadow-sm backdrop-blur-md">
<Button {actions.map((action) => {
type="button" const label = t(action === 'stage' ? 'diffView.hunk.stageTitle'
variant="outline" : action === 'unstage' ? 'diffView.hunk.unstageTitle' : 'diffView.hunk.discardTitle', { index: index + 1 });
size="xs" const busy = busyHunk?.index === index && busyHunk.action === action;
disabled={disabled}
className={dropdownTriggerVariants({ size: 'sm' })}
aria-label={t('diffView.hunk.label')}
title={t('diffView.hunk.label')}
>
<Icon name="stack" className="size-3.5" />
<span>{t('diffView.hunk.label')} · {hunks.length}</span>
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" sideOffset={6} className="w-64 max-h-[min(24rem,var(--available-height))] overflow-y-auto">
<DropdownMenuLabel className="max-w-full truncate" title={filePath}>
{t('diffView.hunk.label')} · {filePath}
</DropdownMenuLabel>
<DropdownMenuSeparator />
{hunks.map((hunk, index) => {
const displayIndex = index + 1;
const busyPrimary = busyHunk?.index === index && busyHunk.action === primaryAction;
const busyDiscard = busyHunk?.index === index && busyHunk.action === 'discard';
const primaryTitle = staged
? t('diffView.hunk.unstageTitle', { index: displayIndex })
: t('diffView.hunk.stageTitle', { index: displayIndex });
const discardTitle = t('diffView.hunk.discardTitle', { index: displayIndex });
return ( return (
<React.Fragment key={index}> <Button
{index > 0 ? <DropdownMenuSeparator /> : null} key={action}
<DropdownMenuItem variant="ghost"
disabled={disabled || busyDiscard} size="xs"
onSelect={() => onAction(index, primaryAction)} className={cn(
aria-label={primaryTitle} 'rounded-full bg-transparent text-muted-foreground hover:bg-transparent hover:text-foreground',
title={primaryTitle} action === 'discard' && 'text-[var(--status-error)] hover:text-[var(--status-error)]',
> action === 'stage' && 'text-[var(--status-success)] hover:text-[var(--status-success)]',
{busyPrimary ? ( )}
<Icon name="loader-4" className="size-3.5 animate-spin" /> disabled={disabled || busyHunk !== null}
) : ( title={label}
<Icon name={staged ? 'arrow-go-back' : 'add'} className="size-3.5" /> aria-label={label}
)} onPointerDown={(event) => event.stopPropagation()}
<span className="min-w-0 flex-1 truncate">{primaryTitle}</span> onClick={(event) => {
<HunkCounts insertions={hunk.insertions} deletions={hunk.deletions} /> event.stopPropagation();
</DropdownMenuItem> onAction(index, action);
{!staged ? ( }}
<DropdownMenuItem >
variant="destructive" <Icon name={busy ? 'loader-4' : action === 'stage' ? 'add' : 'arrow-go-back'}
disabled={disabled || busyPrimary} className={cn(action === 'stage' ? 'size-4' : 'size-3.5', busy && 'animate-spin')} />
onSelect={() => onAction(index, 'discard')} </Button>
aria-label={discardTitle}
title={discardTitle}
>
{busyDiscard ? (
<Icon name="loader-4" className="size-3.5 animate-spin" />
) : (
<Icon name="arrow-go-back" className="size-3.5" />
)}
<span className="min-w-0 flex-1 truncate">{discardTitle}</span>
</DropdownMenuItem>
) : null}
</React.Fragment>
); );
})} })}
</DropdownMenuContent> </div>
</DropdownMenu> </div>
); );
}); });
+29 -1
View File
@@ -1,5 +1,5 @@
import { describe, expect, test } from "bun:test"; import { describe, expect, test } from "bun:test";
import { extractHunkPatch, splitPatchIntoHunks, haveMatchingPatchVersions } from "./patchFileDiff"; import { extractHunkPatch, splitPatchIntoHunks, haveMatchingPatchVersions, getPatchHunkAnchors } from "./patchFileDiff";
const SAMPLE_PATCH = `diff --git a/foo.txt b/foo.txt const SAMPLE_PATCH = `diff --git a/foo.txt b/foo.txt
index 1111111..2222222 100644 index 1111111..2222222 100644
@@ -97,3 +97,31 @@ describe("extractHunkPatch", () => {
expect(extractHunkPatch("", 0)).toBeNull(); expect(extractHunkPatch("", 0)).toBeNull();
}); });
}); });
describe('hunk action anchors', () => {
const header = 'diff --git a/f b/f\n--- a/f\n+++ b/f\n';
test('anchors below trailing deletions rather than above them on the shorter new side', () => {
expect(getPatchHunkAnchors(header + '@@ -8,5 +8,3 @@\n line8\n line9\n line10\n-gone11\n-gone12\n')).toEqual([
{ index: 0, side: 'deletions', lineNumber: 12 },
]);
});
test('anchors before trailing context and preserves canonical hunk indices', () => {
expect(getPatchHunkAnchors(header + '@@ -1,2 +1,2 @@\n-old\n+new\n context\n@@ -20 +20 @@\n-before\n+after\n')).toEqual([
{ index: 0, side: 'additions', lineNumber: 1 },
{ index: 1, side: 'additions', lineNumber: 20 },
]);
});
test('supports added and fully deleted files with an empty opposite side', () => {
expect(getPatchHunkAnchors(header + '@@ -0,0 +1,2 @@\n+a\n+b\n')).toEqual([{ index: 0, side: 'additions', lineNumber: 2 }]);
expect(getPatchHunkAnchors(header + '@@ -1,2 +0,0 @@\n-a\n-b\n')).toEqual([{ index: 0, side: 'deletions', lineNumber: 2 }]);
});
test('ignores no-newline metadata when selecting the final row', () => {
expect(getPatchHunkAnchors(header + '@@ -1 +1 @@\n-before\r\n+after\r\n\\ No newline at end of file\n')).toEqual([
{ index: 0, side: 'additions', lineNumber: 1 },
]);
});
test('does not create controls for an empty or malformed patch', () => {
expect(getPatchHunkAnchors('')).toEqual([]);
expect(getPatchHunkAnchors(header + '@@ -0,0 +0,0 @@\n')).toEqual([]);
});
});
+33
View File
@@ -3,6 +3,7 @@ import {
parsePatchFiles, parsePatchFiles,
processFile, processFile,
trimPatchContext, trimPatchContext,
type AnnotationSide,
type FileDiffMetadata, type FileDiffMetadata,
} from '@pierre/diffs'; } from '@pierre/diffs';
@@ -177,3 +178,35 @@ export const extractHunkPatch = (patch: string, hunkIndex: number): string | nul
const hunks = splitPatchIntoHunks(patch); const hunks = splitPatchIntoHunks(patch);
return hunks[hunkIndex] ?? null; return hunks[hunkIndex] ?? null;
}; };
export interface PatchHunkAnchor {
index: number;
side: AnnotationSide;
lineNumber: number;
}
/** Anchor each action after the final changed row, before trailing context. */
export const getPatchHunkAnchors = (patch: string): PatchHunkAnchor[] => {
const anchors: PatchHunkAnchor[] = [];
for (const [index, hunk] of splitPatchIntoHunks(patch).entries()) {
const header = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@[^\n]*(?:\n|$)/m.exec(hunk);
if (!header) continue;
let deletionLine = Number(header[1]);
let additionLine = Number(header[3]);
let anchor: PatchHunkAnchor | undefined;
for (const line of hunk.slice(header.index + header[0].length).split('\n')) {
if (line.startsWith('-')) {
anchor = { index, side: 'deletions', lineNumber: deletionLine++ };
} else if (line.startsWith('+')) {
anchor = { index, side: 'additions', lineNumber: additionLine++ };
} else if (line.startsWith(' ')) {
deletionLine += 1;
additionLine += 1;
}
}
if (anchor && Number.isSafeInteger(anchor.lineNumber) && anchor.lineNumber > 0) {
anchors.push(anchor);
}
}
return anchors;
};
+15 -6
View File
@@ -137,15 +137,24 @@ The following functions are internal helpers used by exported functions:
- Commit comparison uses the same server boundary through optional `GitAPI.getGitCommitDiff`. Desktop Changes, mobile Changes, and the existing walkthrough surface share branch/commit comparison semantics. Mobile Changes uses the same selectors and `useGitComparison` file-list owner, with a read-only list-to-detail flow. VS Code keeps its existing modes because its Git bridge does not provide these comparison operations. The HTTP operations are available to web, Electron, hosted mobile, and Capacitor clients. - Commit comparison uses the same server boundary through optional `GitAPI.getGitCommitDiff`. Desktop Changes, mobile Changes, and the existing walkthrough surface share branch/commit comparison semantics. Mobile Changes uses the same selectors and `useGitComparison` file-list owner, with a read-only list-to-detail flow. VS Code keeps its existing modes because its Git bridge does not provide these comparison operations. The HTTP operations are available to web, Electron, hosted mobile, and Capacitor clients.
### Staged and unstaged change handling ### Staged and unstaged change handling
- Desktop Changes keeps a canonical three-line-context action patch separate - Desktop Changes floats a compact action capsule after each hunk's last changed row,
from its full-file display patch. Multi-hunk actions require identical file including single-hunk files. Whole-file controls remain in the Git panel.
headers and full blob identities for the display/action pair, including cached `getPatchHunkAnchors` uses the canonical patch's final changed row and side, so a hunk
action patches; mismatch or missing identity leaves actions unavailable until ending in deletions is anchored after those deletions rather than above them.
Retry obtains a matching pair. Successful file/hunk mutations invalidate Zero-height Pierre annotation slots anchor the capsule over following context
without a separate band. At EOF the capsule lifts inside the code column;
a one-line code column has a minimum hit-target height. React controls mount only
for currently rendered slots and only after their rendered diff and anchor
identities match the current props. Comment annotations remain independent.
- The canonical three-line-context action patch stays separate from the full-file
display patch. Their bytes must be identical, or their file headers and full
blob identities must match, including when reusing a cached action patch.
Mismatch leaves actions unavailable until Retry obtains a matching pair.
Successful hunk mutations invalidate
every mounted view of that path through `sessionEvents.requestGitRefresh`. every mounted view of that path through `sessionEvents.requestGitRefresh`.
Actions remain unavailable until the refresh succeeds. Last turn, Branch and Actions remain unavailable until the refresh succeeds. Last turn, Branch and
Commit snapshots never expose hunk mutations. Mobile uses its separate Changes Commit snapshots never expose hunk mutations. Mobile uses its separate Changes
surface and VS Code does not mount this menu. surface and VS Code does not mount these controls.
- Untracked patches from `getDiff` and `getUntrackedDiffs` use `git diff --no-index` with separate stdout, stderr, and process exit status. Exit codes 0 and 1 return stdout only, so line-ending warnings never become patch text or request failures. Other exits and process failures reject the single-file request; the batch keeps an empty entry for the failed path and preserves the other results. - Untracked patches from `getDiff` and `getUntrackedDiffs` use `git diff --no-index` with separate stdout, stderr, and process exit status. Exit codes 0 and 1 return stdout only, so line-ending warnings never become patch text or request failures. Other exits and process failures reject the single-file request; the batch keeps an empty entry for the failed path and preserves the other results.
- `status.files` exposes both `index` and `working_dir` codes. Shared UI uses these as separate scopes: staged rows are derived from non-empty `index` statuses, while unstaged rows are derived from `working_dir` statuses and untracked files. - `status.files` exposes both `index` and `working_dir` codes. Shared UI uses these as separate scopes: staged rows are derived from non-empty `index` statuses, while unstaged rows are derived from `working_dir` statuses and untracked files.
- A file with both staged and unstaged changes can appear in both UI sections. Staged rows request diffs with `staged: true`; unstaged rows request normal working-tree diffs. - A file with both staged and unstaged changes can appear in both UI sections. Staged rows request diffs with `staged: true`; unstaged rows request normal working-tree diffs.
+6 -2
View File
@@ -1,8 +1,12 @@
import { test } from 'vitest'; import { afterAll, test } from 'vitest';
import { exerciseDiffHunkActions } from '@openchamber/ui/components/views/DiffView.hunks.fixture'; import { closeDiffHunkWorkers, exerciseDiffHunkActions } from '@openchamber/ui/components/views/DiffView.hunks.fixture';
afterAll(closeDiffHunkWorkers);
// The actual view imports Vite asset globs. Run its UI-owned DOM fixture through // The actual view imports Vite asset globs. Run its UI-owned DOM fixture through
// the web renderer's transform pipeline instead of mocking those modules. // the web renderer's transform pipeline instead of mocking those modules.
test('hunk actions refresh unchanged-status patches, survive full context and exclude historical diffs', () => exerciseDiffHunkActions()); test('hunk actions refresh unchanged-status patches, survive full context and exclude historical diffs', () => exerciseDiffHunkActions());
test('full-context and action reads must describe the same file version', () => exerciseDiffHunkActions('cold')); test('full-context and action reads must describe the same file version', () => exerciseDiffHunkActions('cold'));
test('cached action patches cannot be paired with a newer full-context display', () => exerciseDiffHunkActions('cached')); test('cached action patches cannot be paired with a newer full-context display', () => exerciseDiffHunkActions('cached'));
test('single-hunk inline actions require matching display and action versions too', () => exerciseDiffHunkActions('cold-single'));
test('split-view hunk controls occupy the matching rendered annotation rows', () => exerciseDiffHunkActions(undefined, 'side-by-side'));