fix(app): keep diff rows aligned for blank lines
This commit is contained in:
@@ -57,7 +57,12 @@ import {
|
||||
import { WORKSPACE_SECONDARY_HEADER_HEIGHT } from "@/constants/layout";
|
||||
import { Fonts } from "@/constants/theme";
|
||||
import { shouldAnchorHeaderBeforeCollapse } from "@/utils/git-diff-scroll";
|
||||
import { buildSplitDiffRows, type SplitDiffDisplayLine, type SplitDiffRow } from "@/utils/diff-layout";
|
||||
import {
|
||||
buildSplitDiffRows,
|
||||
buildUnifiedDiffLines,
|
||||
type SplitDiffDisplayLine,
|
||||
type SplitDiffRow,
|
||||
} from "@/utils/diff-layout";
|
||||
import {
|
||||
DropdownMenu,
|
||||
DropdownMenuContent,
|
||||
@@ -75,6 +80,11 @@ import { openExternalUrl } from "@/utils/open-external-url";
|
||||
import { GitActionsSplitButton } from "@/components/git-actions-split-button";
|
||||
import { usePanelStore } from "@/stores/panel-store";
|
||||
import { buildWorkspaceExplorerStateKey } from "@/hooks/use-file-explorer-actions";
|
||||
import {
|
||||
formatDiffContentText,
|
||||
formatDiffGutterText,
|
||||
hasVisibleDiffTokens,
|
||||
} from "@/utils/diff-rendering";
|
||||
|
||||
export type { GitActionId, GitAction, GitActions } from "@/components/git-actions-policy";
|
||||
|
||||
@@ -165,7 +175,7 @@ function DiffGutterCell({
|
||||
type === "remove" && styles.removeLineNumberText,
|
||||
]}
|
||||
>
|
||||
{lineNumber != null ? String(lineNumber) : ""}
|
||||
{formatDiffGutterText(lineNumber)}
|
||||
</Text>
|
||||
</View>
|
||||
);
|
||||
@@ -178,10 +188,12 @@ function DiffTextLine({
|
||||
line: DiffLine;
|
||||
wrapLines: boolean;
|
||||
}) {
|
||||
const visibleTokens = hasVisibleDiffTokens(line.tokens) ? line.tokens : null;
|
||||
|
||||
return (
|
||||
<View style={[styles.textLineContainer, lineTypeBackground(line.type)]}>
|
||||
{line.tokens && line.type !== "header" ? (
|
||||
<HighlightedText tokens={line.tokens} wrapLines={wrapLines} />
|
||||
{line.type !== "header" && visibleTokens ? (
|
||||
<HighlightedText tokens={visibleTokens} wrapLines={wrapLines} />
|
||||
) : (
|
||||
<Text
|
||||
style={[
|
||||
@@ -193,7 +205,7 @@ function DiffTextLine({
|
||||
line.type === "context" && styles.contextLineText,
|
||||
]}
|
||||
>
|
||||
{line.content || " "}
|
||||
{formatDiffContentText(line.content)}
|
||||
</Text>
|
||||
)}
|
||||
</View>
|
||||
@@ -207,10 +219,12 @@ function SplitTextLine({
|
||||
line: SplitDiffDisplayLine | null;
|
||||
wrapLines: boolean;
|
||||
}) {
|
||||
const visibleTokens = line && hasVisibleDiffTokens(line.tokens) ? line.tokens : null;
|
||||
|
||||
return (
|
||||
<View style={[styles.textLineContainer, lineTypeBackground(line?.type)]}>
|
||||
{line?.tokens ? (
|
||||
<HighlightedText tokens={line.tokens} wrapLines={wrapLines} />
|
||||
{visibleTokens ? (
|
||||
<HighlightedText tokens={visibleTokens} wrapLines={wrapLines} />
|
||||
) : (
|
||||
<Text
|
||||
style={[
|
||||
@@ -222,7 +236,7 @@ function SplitTextLine({
|
||||
!line && styles.emptySplitCellText,
|
||||
]}
|
||||
>
|
||||
{line?.content ?? ""}
|
||||
{formatDiffContentText(line?.content)}
|
||||
</Text>
|
||||
)}
|
||||
</View>
|
||||
@@ -240,6 +254,8 @@ function DiffLineView({
|
||||
gutterWidth: number;
|
||||
wrapLines: boolean;
|
||||
}) {
|
||||
const visibleTokens = hasVisibleDiffTokens(line.tokens) ? line.tokens : null;
|
||||
|
||||
return (
|
||||
<View
|
||||
style={[
|
||||
@@ -255,11 +271,11 @@ function DiffLineView({
|
||||
line.type === "remove" && styles.removeLineNumberText,
|
||||
]}
|
||||
>
|
||||
{lineNumber != null ? String(lineNumber) : ""}
|
||||
{formatDiffGutterText(lineNumber)}
|
||||
</Text>
|
||||
</View>
|
||||
{line.tokens && line.type !== "header" ? (
|
||||
<HighlightedText tokens={line.tokens} wrapLines={wrapLines} />
|
||||
{line.type !== "header" && visibleTokens ? (
|
||||
<HighlightedText tokens={visibleTokens} wrapLines={wrapLines} />
|
||||
) : (
|
||||
<Text
|
||||
style={[
|
||||
@@ -271,7 +287,7 @@ function DiffLineView({
|
||||
line.type === "context" && styles.contextLineText,
|
||||
]}
|
||||
>
|
||||
{line.content || " "}
|
||||
{formatDiffContentText(line.content)}
|
||||
</Text>
|
||||
)}
|
||||
</View>
|
||||
@@ -287,6 +303,8 @@ function SplitDiffLine({
|
||||
gutterWidth: number;
|
||||
wrapLines: boolean;
|
||||
}) {
|
||||
const visibleTokens = line && hasVisibleDiffTokens(line.tokens) ? line.tokens : null;
|
||||
|
||||
return (
|
||||
<View
|
||||
style={[
|
||||
@@ -302,11 +320,11 @@ function SplitDiffLine({
|
||||
line?.type === "remove" && styles.removeLineNumberText,
|
||||
]}
|
||||
>
|
||||
{line?.lineNumber != null ? String(line.lineNumber) : ""}
|
||||
{formatDiffGutterText(line?.lineNumber ?? null)}
|
||||
</Text>
|
||||
</View>
|
||||
{line?.tokens ? (
|
||||
<HighlightedText tokens={line.tokens} wrapLines={wrapLines} />
|
||||
{visibleTokens ? (
|
||||
<HighlightedText tokens={visibleTokens} wrapLines={wrapLines} />
|
||||
) : (
|
||||
<Text
|
||||
style={[
|
||||
@@ -318,7 +336,7 @@ function SplitDiffLine({
|
||||
!line && styles.emptySplitCellText,
|
||||
]}
|
||||
>
|
||||
{line?.content ?? ""}
|
||||
{formatDiffContentText(line?.content)}
|
||||
</Text>
|
||||
)}
|
||||
</View>
|
||||
@@ -535,26 +553,7 @@ function DiffFileBody({
|
||||
);
|
||||
}
|
||||
|
||||
const computedLines: { line: DiffLine; lineNumber: number | null; key: string }[] = [];
|
||||
for (const [hunkIndex, hunk] of file.hunks.entries()) {
|
||||
let oldLineNo = hunk.oldStart;
|
||||
let newLineNo = hunk.newStart;
|
||||
for (const [lineIndex, line] of hunk.lines.entries()) {
|
||||
let lineNumber: number | null = null;
|
||||
if (line.type === "remove") {
|
||||
lineNumber = oldLineNo;
|
||||
oldLineNo++;
|
||||
} else if (line.type === "add") {
|
||||
lineNumber = newLineNo;
|
||||
newLineNo++;
|
||||
} else if (line.type === "context") {
|
||||
lineNumber = newLineNo;
|
||||
oldLineNo++;
|
||||
newLineNo++;
|
||||
}
|
||||
computedLines.push({ line, lineNumber, key: `${hunkIndex}-${lineIndex}` });
|
||||
}
|
||||
}
|
||||
const computedLines = buildUnifiedDiffLines(file);
|
||||
|
||||
if (wrapLines) {
|
||||
return (
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { buildSplitDiffRows } from "./diff-layout";
|
||||
import { buildSplitDiffRows, buildUnifiedDiffLines } from "./diff-layout";
|
||||
import type { ParsedDiffFile } from "@/hooks/use-checkout-diff-query";
|
||||
|
||||
function makeFile(lines: ParsedDiffFile["hunks"][number]["lines"]): ParsedDiffFile {
|
||||
@@ -79,3 +79,76 @@ describe("buildSplitDiffRows", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildUnifiedDiffLines", () => {
|
||||
it("computes line numbers per line type within a hunk", () => {
|
||||
const lines = buildUnifiedDiffLines(
|
||||
makeFile([
|
||||
{ type: "header", content: "@@ -10,3 +10,4 @@" },
|
||||
{ type: "context", content: "before" },
|
||||
{ type: "add", content: "inserted" },
|
||||
{ type: "remove", content: "removed" },
|
||||
{ type: "context", content: "after" },
|
||||
]),
|
||||
);
|
||||
|
||||
expect(
|
||||
lines.map(({ line, lineNumber }) => ({
|
||||
type: line.type,
|
||||
lineNumber,
|
||||
content: line.content,
|
||||
})),
|
||||
).toEqual([
|
||||
{ type: "header", lineNumber: null, content: "@@ -10,3 +10,4 @@" },
|
||||
{ type: "context", lineNumber: 10, content: "before" },
|
||||
{ type: "add", lineNumber: 11, content: "inserted" },
|
||||
{ type: "remove", lineNumber: 11, content: "removed" },
|
||||
{ type: "context", lineNumber: 12, content: "after" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("restarts numbering at each hunk boundary", () => {
|
||||
const file: ParsedDiffFile = {
|
||||
path: "example.ts",
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: 1,
|
||||
deletions: 0,
|
||||
status: "ok",
|
||||
hunks: [
|
||||
{
|
||||
oldStart: 75,
|
||||
oldCount: 2,
|
||||
newStart: 75,
|
||||
newCount: 3,
|
||||
lines: [
|
||||
{ type: "header", content: "@@ -75,2 +75,3 @@" },
|
||||
{ type: "context", content: "first" },
|
||||
{ type: "add", content: "inserted" },
|
||||
{ type: "context", content: "second" },
|
||||
],
|
||||
},
|
||||
{
|
||||
oldStart: 165,
|
||||
oldCount: 2,
|
||||
newStart: 166,
|
||||
newCount: 2,
|
||||
lines: [
|
||||
{ type: "header", content: "@@ -165,2 +166,2 @@" },
|
||||
{ type: "context", content: "third" },
|
||||
{ type: "context", content: "fourth" },
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const lines = buildUnifiedDiffLines(file);
|
||||
|
||||
expect(lines[0]?.lineNumber).toBeNull();
|
||||
expect(lines[1]?.lineNumber).toBe(75);
|
||||
expect(lines[3]?.lineNumber).toBe(77);
|
||||
expect(lines[4]?.lineNumber).toBeNull();
|
||||
expect(lines[5]?.lineNumber).toBe(166);
|
||||
expect(lines[6]?.lineNumber).toBe(167);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,6 +7,12 @@ export interface SplitDiffDisplayLine {
|
||||
lineNumber: number | null;
|
||||
}
|
||||
|
||||
export interface UnifiedDiffDisplayLine {
|
||||
key: string;
|
||||
line: DiffLine;
|
||||
lineNumber: number | null;
|
||||
}
|
||||
|
||||
export type SplitDiffRow =
|
||||
| {
|
||||
kind: "header";
|
||||
@@ -61,6 +67,39 @@ function toDisplayLine(input: {
|
||||
};
|
||||
}
|
||||
|
||||
export function buildUnifiedDiffLines(file: ParsedDiffFile): UnifiedDiffDisplayLine[] {
|
||||
const lines: UnifiedDiffDisplayLine[] = [];
|
||||
|
||||
for (const [hunkIndex, hunk] of file.hunks.entries()) {
|
||||
let oldLineNo = hunk.oldStart;
|
||||
let newLineNo = hunk.newStart;
|
||||
|
||||
for (const [lineIndex, line] of hunk.lines.entries()) {
|
||||
let lineNumber: number | null = null;
|
||||
|
||||
if (line.type === "remove") {
|
||||
lineNumber = oldLineNo;
|
||||
oldLineNo += 1;
|
||||
} else if (line.type === "add") {
|
||||
lineNumber = newLineNo;
|
||||
newLineNo += 1;
|
||||
} else if (line.type === "context") {
|
||||
lineNumber = newLineNo;
|
||||
oldLineNo += 1;
|
||||
newLineNo += 1;
|
||||
}
|
||||
|
||||
lines.push({
|
||||
key: `${hunkIndex}-${lineIndex}`,
|
||||
line,
|
||||
lineNumber,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return lines;
|
||||
}
|
||||
|
||||
export function buildSplitDiffRows(file: ParsedDiffFile): SplitDiffRow[] {
|
||||
const rows: SplitDiffRow[] = [];
|
||||
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { formatDiffContentText, formatDiffGutterText, hasVisibleDiffTokens } from "./diff-rendering";
|
||||
|
||||
describe("diff-rendering", () => {
|
||||
it("keeps header gutters tall even when they do not show a line number", () => {
|
||||
expect(formatDiffGutterText(null)).toBe(" ");
|
||||
expect(formatDiffGutterText(82)).toBe("82");
|
||||
});
|
||||
|
||||
it("keeps empty split cells tall even when they have no visible content", () => {
|
||||
expect(formatDiffContentText(undefined)).toBe(" ");
|
||||
expect(formatDiffContentText("")).toBe(" ");
|
||||
expect(formatDiffContentText("const value = 1;")).toBe("const value = 1;");
|
||||
});
|
||||
|
||||
it("treats empty highlighted token rows as blank lines instead of visible content", () => {
|
||||
expect(hasVisibleDiffTokens(undefined)).toBe(false);
|
||||
expect(hasVisibleDiffTokens([])).toBe(false);
|
||||
expect(hasVisibleDiffTokens([{ text: "" }])).toBe(false);
|
||||
expect(hasVisibleDiffTokens([{ text: "const value = 1;" }])).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,16 @@
|
||||
interface HighlightLikeToken {
|
||||
text: string;
|
||||
}
|
||||
|
||||
// Preserve row height when a gutter or diff cell is intentionally blank.
|
||||
export function formatDiffGutterText(lineNumber: number | null): string {
|
||||
return lineNumber == null ? " " : String(lineNumber);
|
||||
}
|
||||
|
||||
export function formatDiffContentText(content: string | null | undefined): string {
|
||||
return content && content.length > 0 ? content : " ";
|
||||
}
|
||||
|
||||
export function hasVisibleDiffTokens(tokens: HighlightLikeToken[] | null | undefined): boolean {
|
||||
return Boolean(tokens?.some((token) => token.text.length > 0));
|
||||
}
|
||||
Reference in New Issue
Block a user