Skip to content

Commit 077a9cb

Browse files
danvkdanvk
authored
Diffhunk selection and keyboard shortcuts for next/prev (#200)
* render diffhunk selection * I really want non-standard scrollIntoViewIfNeeded * scrollIntoViewIfNeeded is better when available * good enough * factor out DiffRow/SkipRow * factor out selection movement logic * use critique color
1 parent 8d2eff7 commit 077a9cb

File tree

5 files changed

+222
-125
lines changed

5 files changed

+222
-125
lines changed

‎ts/codediff/DiffRow.tsx‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import React from 'react';
2+
import {addCharacterDiffs} from './char-diffs';
3+
import {DiffRange} from './codes';
4+
import {scrollIntoViewIfNeeded} from './dom-utils';
5+
6+
// TODO: factor out a {text, html} type
7+
interface DiffRowProps {
8+
type: DiffRange['type'];
9+
beforeLineNum: number | null;
10+
afterLineNum: number | null;
11+
beforeText: string | undefined;
12+
beforeHTML?: string;
13+
afterText: string | undefined;
14+
afterHTML?: string;
15+
isSelected: boolean;
16+
}
17+
18+
function escapeHtml(unsafe: string) {
19+
return unsafe
20+
.replaceAll('&', '&')
21+
.replaceAll('<', '&lt;')
22+
.replaceAll('>', '&gt;')
23+
.replaceAll('"', '&quot;')
24+
.replaceAll("'", '&#039;');
25+
}
26+
27+
const makeCodeTd = (type: string, text: string | undefined, html: string | undefined) => {
28+
if (text === undefined) {
29+
return {text: '', html: '', className: 'empty code'};
30+
}
31+
if (html === undefined) {
32+
html = escapeHtml(text);
33+
}
34+
text = text.replaceAll('\t', '\u00a0\u00a0\u00a0\u00a0');
35+
html = html.replaceAll('\t', '\u00a0\u00a0\u00a0\u00a0');
36+
const className = 'code ' + type;
37+
return {className, html, text};
38+
};
39+
40+
export function DiffRow(props: DiffRowProps) {
41+
const {beforeLineNum, afterLineNum, type, isSelected} = props;
42+
const cells = [
43+
makeCodeTd(type, props.beforeText, props.beforeHTML),
44+
makeCodeTd(type, props.afterText, props.afterHTML),
45+
];
46+
let [beforeHtml, afterHtml] = [cells[0].html, cells[1].html];
47+
if (type === 'replace') {
48+
[beforeHtml, afterHtml] = addCharacterDiffs(
49+
cells[0].text,
50+
cells[0].html,
51+
cells[1].text,
52+
cells[1].html,
53+
);
54+
}
55+
const rowRef = React.useRef<HTMLTableRowElement>(null);
56+
React.useEffect(() => {
57+
if (isSelected && rowRef.current) {
58+
scrollIntoViewIfNeeded(rowRef.current);
59+
}
60+
}, [isSelected]);
61+
return (
62+
<tr ref={rowRef} className={isSelected ? 'selected' : undefined}>
63+
<td className="line-no">{beforeLineNum ?? ''}</td>
64+
<td
65+
className={cells[0].className + ' before'}
66+
dangerouslySetInnerHTML={{__html: beforeHtml}}></td>
67+
<td
68+
className={cells[1].className + ' after'}
69+
dangerouslySetInnerHTML={{__html: afterHtml}}></td>
70+
<td className="line-no">{afterLineNum ?? ''}</td>
71+
</tr>
72+
);
73+
}

‎ts/codediff/SkipRow.tsx‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import React from 'react';
2+
import {scrollIntoViewIfNeeded} from './dom-utils';
3+
4+
export interface SkipRange {
5+
beforeStartLine: number;
6+
afterStartLine: number;
7+
numRows: number;
8+
}
9+
10+
export interface SkipRowProps extends SkipRange {
11+
header: string | null;
12+
expandLines: number;
13+
/** positive num = expand down, negative num = expand up */
14+
onShowMore: (existing: SkipRange, num: number) => void;
15+
isSelected: boolean;
16+
}
17+
18+
export function SkipRow(props: SkipRowProps) {
19+
const {expandLines, header, onShowMore, isSelected, ...range} = props;
20+
const {numRows} = range;
21+
const showAll = (e: React.MouseEvent) => {
22+
e.preventDefault();
23+
onShowMore(range, numRows);
24+
};
25+
const arrows =
26+
numRows <= expandLines ? (
27+
<span className="skip" title={`show ${numRows} skipped lines`} onClick={showAll}>
28+
↕
29+
</span>
30+
) : (
31+
<>
32+
<span
33+
className="skip expand-up"
34+
title={`show ${expandLines} more lines above`}
35+
onClick={() => {
36+
onShowMore(range, -expandLines);
37+
}}>
38+
↥
39+
</span>
40+
<span
41+
className="skip expand-down"
42+
title={`show ${expandLines} more lines below`}
43+
onClick={() => {
44+
onShowMore(range, expandLines);
45+
}}>
46+
↧
47+
</span>
48+
</>
49+
);
50+
const showMore = (
51+
<a href="#" onClick={showAll}>
52+
Show {numRows} more lines
53+
</a>
54+
);
55+
const headerHTML = header ? <span className="hunk-header">${header}</span> : '';
56+
57+
const rowRef = React.useRef<HTMLTableRowElement>(null);
58+
React.useEffect(() => {
59+
if (isSelected && rowRef.current) {
60+
scrollIntoViewIfNeeded(rowRef.current);
61+
}
62+
}, [isSelected]);
63+
return (
64+
<tr ref={rowRef} className={'skip-row' + (isSelected ? ` selected` : '')}>
65+
<td colSpan={4} className="skip code">
66+
<span className="arrows-left">{arrows}</span>
67+
{showMore} {headerHTML}
68+
<span className="arrows-right">{arrows}</span>
69+
</td>
70+
</tr>
71+
);
72+
}

‎ts/codediff/codediff.tsx‎

Lines changed: 54 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import React from 'react';
22

33
import {DiffRange} from './codes';
44
import {closest, copyOnlyMatching, distributeSpans} from './dom-utils';
5-
import {addCharacterDiffs} from './char-diffs';
65
import {stringAsLines} from './string-utils';
6+
import {isLegitKeypress} from '../file_diff';
7+
import {DiffRow} from './DiffRow';
8+
import {SkipRange, SkipRow} from './SkipRow';
79

810
export interface PatchOptions {
911
/** Minimum number of skipped lines to elide into a "jump" row */
@@ -88,6 +90,38 @@ export function CodeDiff(props: Props) {
8890
);
8991
}
9092

93+
function moveUpDown(
94+
dir: 'up' | 'down',
95+
selectedLine: number | undefined,
96+
ops: readonly DiffRange[],
97+
): number | undefined {
98+
if (dir === 'up') {
99+
if (selectedLine === undefined) {
100+
return 0;
101+
} else {
102+
for (const range of ops) {
103+
const {after} = range;
104+
const afterStart = after[0];
105+
if (selectedLine < afterStart) {
106+
return afterStart;
107+
}
108+
}
109+
// TODO: if the last hunk was already selected, advance to the next file.
110+
}
111+
} else {
112+
if (selectedLine !== undefined) {
113+
for (let i = ops.length - 1; i >= 0; i--) {
114+
const range = ops[i];
115+
const {after} = range;
116+
const afterStart = after[0];
117+
if (selectedLine > afterStart) {
118+
return afterStart;
119+
}
120+
}
121+
}
122+
}
123+
}
124+
91125
interface CodeDiffViewProps {
92126
beforeLines: readonly string[];
93127
afterLines: readonly string[];
@@ -114,6 +148,7 @@ const CodeDiffView = React.memo((props: CodeDiffViewProps) => {
114148
// this will blow away all "show more lines" actions
115149
setOps(initOps);
116150
}, [initOps]);
151+
const [selectedLine, setSelectedLine] = React.useState<number | undefined>();
117152
const handleShowMore = (existing: SkipRange, num: number) => {
118153
setOps(oldOps =>
119154
oldOps.flatMap(op => {
@@ -150,6 +185,21 @@ const CodeDiffView = React.memo((props: CodeDiffViewProps) => {
150185
);
151186
};
152187

188+
React.useEffect(() => {
189+
const handleKeydown = (e: KeyboardEvent) => {
190+
if (!isLegitKeypress(e)) return;
191+
if (e.code !== 'KeyN' && e.code !== 'KeyP') return;
192+
const newSelectedLine = moveUpDown(e.code === 'KeyN' ? 'up' : 'down', selectedLine, ops);
193+
if (newSelectedLine !== undefined) {
194+
setSelectedLine(newSelectedLine);
195+
}
196+
};
197+
document.addEventListener('keydown', handleKeydown);
198+
return () => {
199+
document.removeEventListener('keydown', handleKeydown);
200+
};
201+
}, [ops, selectedLine]);
202+
153203
const diffRows = [];
154204
for (const range of ops) {
155205
const {type} = range;
@@ -159,6 +209,7 @@ const CodeDiffView = React.memo((props: CodeDiffViewProps) => {
159209
const numRows = Math.max(numBeforeRows, numAfterRows);
160210
const beforeStartLine = before[0];
161211
const afterStartLine = after[0];
212+
const isSelected = afterStartLine === selectedLine;
162213
if (type == 'skip') {
163214
diffRows.push(
164215
<SkipRow
@@ -169,6 +220,7 @@ const CodeDiffView = React.memo((props: CodeDiffViewProps) => {
169220
header={range.header ?? null}
170221
expandLines={expandLines}
171222
onShowMore={handleShowMore}
223+
isSelected={isSelected}
172224
/>,
173225
);
174226
} else {
@@ -193,6 +245,7 @@ const CodeDiffView = React.memo((props: CodeDiffViewProps) => {
193245
beforeHTML={beforeHTML}
194246
afterText={afterText}
195247
afterHTML={afterHTML}
248+
isSelected={j === 0 && isSelected}
196249
/>,
197250
);
198251
}
@@ -237,127 +290,3 @@ const CodeDiffView = React.memo((props: CodeDiffViewProps) => {
237290
</div>
238291
);
239292
});
240-
241-
interface SkipRange {
242-
beforeStartLine: number;
243-
afterStartLine: number;
244-
numRows: number;
245-
}
246-
247-
export interface SkipRowProps extends SkipRange {
248-
header: string | null;
249-
expandLines: number;
250-
/** positive num = expand down, negative num = expand up */
251-
onShowMore: (existing: SkipRange, num: number) => void;
252-
}
253-
254-
function SkipRow(props: SkipRowProps) {
255-
const {expandLines, header, onShowMore, ...range} = props;
256-
const {numRows} = range;
257-
const showAll = (e: React.MouseEvent) => {
258-
e.preventDefault();
259-
onShowMore(range, numRows);
260-
};
261-
const arrows =
262-
numRows <= expandLines ? (
263-
<span className="skip" title={`show ${numRows} skipped lines`} onClick={showAll}>
264-
↕
265-
</span>
266-
) : (
267-
<>
268-
<span
269-
className="skip expand-up"
270-
title={`show ${expandLines} more lines above`}
271-
onClick={() => {
272-
onShowMore(range, -expandLines);
273-
}}>
274-
↥
275-
</span>
276-
<span
277-
className="skip expand-down"
278-
title={`show ${expandLines} more lines below`}
279-
onClick={() => {
280-
onShowMore(range, expandLines);
281-
}}>
282-
↧
283-
</span>
284-
</>
285-
);
286-
const showMore = (
287-
<a href="#" onClick={showAll}>
288-
Show {numRows} more lines
289-
</a>
290-
);
291-
const headerHTML = header ? <span className="hunk-header">${header}</span> : '';
292-
return (
293-
<tr className="skip-row">
294-
<td colSpan={4} className="skip code">
295-
<span className="arrows-left">{arrows}</span>
296-
{showMore} {headerHTML}
297-
<span className="arrows-right">{arrows}</span>
298-
</td>
299-
</tr>
300-
);
301-
}
302-
303-
// TODO: factor out a {text, html} type
304-
interface DiffRowProps {
305-
type: DiffRange['type'];
306-
beforeLineNum: number | null;
307-
afterLineNum: number | null;
308-
beforeText: string | undefined;
309-
beforeHTML?: string;
310-
afterText: string | undefined;
311-
afterHTML?: string;
312-
}
313-
314-
function escapeHtml(unsafe: string) {
315-
return unsafe
316-
.replaceAll('&', '&amp;')
317-
.replaceAll('<', '&lt;')
318-
.replaceAll('>', '&gt;')
319-
.replaceAll('"', '&quot;')
320-
.replaceAll("'", '&#039;');
321-
}
322-
323-
const makeCodeTd = (type: string, text: string | undefined, html: string | undefined) => {
324-
if (text === undefined) {
325-
return {text: '', html: '', className: 'empty code'};
326-
}
327-
if (html === undefined) {
328-
html = escapeHtml(text);
329-
}
330-
text = text.replaceAll('\t', '\u00a0\u00a0\u00a0\u00a0');
331-
html = html.replaceAll('\t', '\u00a0\u00a0\u00a0\u00a0');
332-
const className = 'code ' + type;
333-
return {className, html, text};
334-
};
335-
336-
function DiffRow(props: DiffRowProps) {
337-
const {beforeLineNum, afterLineNum, type} = props;
338-
const cells = [
339-
makeCodeTd(type, props.beforeText, props.beforeHTML),
340-
makeCodeTd(type, props.afterText, props.afterHTML),
341-
];
342-
let [beforeHtml, afterHtml] = [cells[0].html, cells[1].html];
343-
if (type === 'replace') {
344-
[beforeHtml, afterHtml] = addCharacterDiffs(
345-
cells[0].text,
346-
cells[0].html,
347-
cells[1].text,
348-
cells[1].html,
349-
);
350-
}
351-
return (
352-
<tr>
353-
<td className="line-no">{beforeLineNum ?? ''}</td>
354-
<td
355-
className={cells[0].className + ' before'}
356-
dangerouslySetInnerHTML={{__html: beforeHtml}}></td>
357-
<td
358-
className={cells[1].className + ' after'}
359-
dangerouslySetInnerHTML={{__html: afterHtml}}></td>
360-
<td className="line-no">{afterLineNum ?? ''}</td>
361-
</tr>
362-
);
363-
}

‎ts/codediff/dom-utils.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,13 @@ export function copyOnlyMatching(e: ClipboardEvent, selector: string) {
7676
e.clipboardData?.setData('text', text);
7777
e.preventDefault();
7878
}
79+
80+
interface WebkitElement extends Element {
81+
scrollIntoViewIfNeeded?: () => void;
82+
}
83+
84+
/** scrollIntoViewIfNeeded has nicer behavior than the web standard, but is non-standard. */
85+
export function scrollIntoViewIfNeeded(el: Element) {
86+
const wkEl = el as WebkitElement;
87+
wkEl.scrollIntoViewIfNeeded?.() ?? wkEl.scrollIntoView({block: 'nearest'});
88+
}

0 commit comments

Comments
 (0)