Show diffstats (#204)

* thread through counts for diffstats

* keyboard shortcut to toggle file list/dropdown

* thread through filePair

* style headers

* fix test, delete then add

* add changes

Author: danvk

testdata/unified/manyfiles.txt

@@ -148,7 +148,7 @@ def test_add_replaces():
 
 
 def test_parse_raw_diff_many():
-    # git diff --no-index --raw -z testdata/manyfiles/{left,right}
+    # git diff --no-index --raw -z --numstat testdata/manyfiles/{left,right}
     diff = open('testdata/unified/manyfiles.txt').read()
     mod644 = ['100644', '100644', '0000000', '0000000']
     assert parse_raw_diff(diff) == [
@@ -161,20 +161,22 @@ def test_parse_raw_diff_many():
             'testdata/manyfiles/left/d.txt',
             score=100,
             dst_path='testdata/manyfiles/right/a.txt',
+            num_add=0,
+            num_delete=0,
         ),
-        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/b.txt'),
-        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/c.txt'),
-        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/e.txt'),
-        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/f.txt'),
-        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/g.txt'),
-        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/h.txt'),
-        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/i.txt'),
-        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/j.txt'),
+        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/b.txt', num_add=0, num_delete=1),
+        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/c.txt', num_add=0, num_delete=1),
+        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/e.txt', num_add=0, num_delete=1),
+        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/f.txt', num_add=0, num_delete=1),
+        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/g.txt', num_add=0, num_delete=1),
+        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/h.txt', num_add=0, num_delete=1),
+        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/i.txt', num_add=0, num_delete=1),
+        RawDiffLine(*mod644, 'M', 'testdata/manyfiles/left/j.txt', num_add=0, num_delete=1),
     ]
 
 
 def test_parse_raw_diff_rename():
-    # git diff --no-index --raw -z testdata/rename+change/{left,right}
+    # git diff --no-index --raw -z --numstat testdata/rename+change/{left,right}
     diff = open('testdata/unified/rename+change.txt').read()
     assert parse_raw_diff(diff) == [
         RawDiffLine(
@@ -186,6 +188,8 @@ def test_parse_raw_diff_rename():
             'testdata/rename+change/left/huckfinn.txt',
             score=90,
             dst_path='testdata/rename+change/right/huckfinn.md',
+            num_add=2,
+            num_delete=2,
         ),
     ]
 

testdata/unified/rename+change.txt

@@ -14,6 +14,8 @@ interface BaseFilePair {
   type: 'add' | 'delete' | 'move' | 'change'; // XXX check "change"
   /** Are there any changes to the file? Only set for "thick" diffs. */
   no_changes?: boolean;
+  num_add: number | null;
+  num_delete: number | null;
 }
 
 interface TextFilePair extends BaseFilePair {
@@ -121,8 +123,7 @@ export function CodeDiffContainer(props: {filePair: FilePair; diffOptions: Parti
       <div ref={codediffRef} key={filePair.idx}>
         {contents ? (
           <FileDiff
-            pathBefore={filePair.a}
-            pathAfter={filePair.b}
+            filePair={filePair}
             contentsBefore={contents.before}
             contentsAfter={contents.after}
             diffOps={contents.diffOps}
@@ -136,8 +137,7 @@ export function CodeDiffContainer(props: {filePair: FilePair; diffOptions: Parti
 }
 
 interface FileDiffProps {
-  pathBefore: string;
-  pathAfter: string;
+  filePair: FilePair;
   contentsBefore: string | null;
   contentsAfter: string | null;
   diffOps: DiffRange[];
@@ -155,7 +155,9 @@ function lengthOrZero(data: unknown[] | string | null | undefined) {
 }
 
 function FileDiff(props: FileDiffProps) {
-  const {pathBefore, pathAfter, contentsBefore, contentsAfter, diffOps} = props;
+  const {filePair, contentsBefore, contentsAfter, diffOps} = props;
+  const pathBefore = filePair.a;
+  const pathAfter = filePair.b;
   // build the diff view and add it to the current DOM
 
   const lastOp = diffOps[diffOps.length - 1];
@@ -182,18 +184,21 @@ function FileDiff(props: FileDiffProps) {
 
   const opts = React.useMemo(
     (): Partial<PatchOptions> => ({
-      // set the display titles for each resource
-      beforeName: pathBefore || '(none)',
-      afterName: pathAfter || '(none)',
       language,
       // TODO: thread through minJumpSize
     }),
-    [language, pathAfter, pathBefore],
+    [language],
   );
 
   return (
     <div className="diff">
-      <CodeDiff beforeText={contentsBefore} afterText={contentsAfter} ops={diffOps} params={opts} />
+      <CodeDiff
+        beforeText={contentsBefore}
+        afterText={contentsAfter}
+        filePair={filePair}
+        ops={diffOps}
+        params={opts}
+      />
     </div>
   );
 }

tests/unified_diff_test.py

@@ -15,6 +15,10 @@ export interface Props {
 export function FileList(props: Props) {
   const {filePairs, selectedIndex, fileChangeHandler} = props;
 
+  const maxDelta = React.useMemo(() => {
+    return Math.max(1, ...filePairs.map(fp => (fp.num_add ?? 0) + (fp.num_delete ?? 0)));
+  }, [filePairs]);
+
   const lis = filePairs.map((filePair, idx) => {
     const displayName = filePairDisplayName(filePair);
     const content =
@@ -31,10 +35,36 @@ export function FileList(props: Props) {
       );
     return (
       <li key={idx}>
+        <SparkChart maxDelta={maxDelta} numAdd={filePair.num_add} numDelete={filePair.num_delete} />
         <span title={filePair.type} className={`diff ${filePair.type}`} />
         {content}
       </li>
     );
   });
   return <ul className="file-list">{lis}</ul>;
 }
+
+interface SparkChartProps {
+  maxDelta: number;
+  numAdd: number | null;
+  numDelete: number | null;
+}
+
+function SparkChart(props: SparkChartProps) {
+  const {numAdd, numDelete, maxDelta} = props;
+  if (numAdd === null || numDelete === null) {
+    return null;
+  }
+  return (
+    <div className="spark">
+      {numDelete > 0 && (
+        <div
+          className="delete"
+          style={{width: `${Math.round((100 * numDelete) / maxDelta)}%`}}></div>
+      )}
+      {numAdd > 0 && (
+        <div className="add" style={{width: `${Math.round((100 * numAdd) / maxDelta)}%`}}></div>
+      )}
+    </div>
+  );
+}

ts/CodeDiffContainer.tsx

@@ -1,8 +1,9 @@
 import React from 'react';
+import {FileSelectorMode} from './FileSelector';
 
 export interface Props {
-  mode: 'list' | 'dropdown';
-  changeHandler: (mode: 'list' | 'dropdown') => void;
+  mode: FileSelectorMode;
+  changeHandler: (mode: FileSelectorMode) => void;
 }
 
 /** A widget for toggling between file selection modes. */

ts/FileList.tsx

@@ -8,16 +8,15 @@ export interface Props {
   filePairs: FilePair[];
   selectedFileIndex: number;
   fileChangeHandler: (newIndex: number) => void;
+  mode: FileSelectorMode;
+  onChangeMode: (mode: FileSelectorMode) => void;
 }
 
-type Mode = 'list' | 'dropdown';
+export type FileSelectorMode = 'list' | 'dropdown';
 
 /** Shows a list of files in one of two possible modes (list or dropdown). */
 export function FileSelector(props: Props) {
-  const {filePairs, selectedFileIndex, fileChangeHandler} = props;
-
-  // An explicit list is better, unless there are a ton of files.
-  const [mode, setMode] = React.useState<Mode>(filePairs.length <= 6 ? 'list' : 'dropdown');
+  const {filePairs, selectedFileIndex, fileChangeHandler, mode, onChangeMode} = props;
 
   // For single file diffs, a file selector is a waste of space.
   if (filePairs.length === 1) {
@@ -46,7 +45,7 @@ export function FileSelector(props: Props) {
   return (
     <div className="file-selector">
       {selector}
-      {filePairs.length > 3 ? <FileModeSelector mode={mode} changeHandler={setMode} /> : null}
+      {filePairs.length > 3 ? <FileModeSelector mode={mode} changeHandler={onChangeMode} /> : null}
     </div>
   );
 }

ts/FileModeSelector.tsx

@@ -3,7 +3,7 @@ import {RouteComponentProps, useHistory} from 'react-router';
 import {FilePair} from './CodeDiffContainer';
 import {DiffOptions} from './diff-options';
 import {DiffView, PerceptualDiffMode} from './DiffView';
-import {FileSelector} from './FileSelector';
+import {FileSelector, FileSelectorMode} from './FileSelector';
 import {isLegitKeypress} from './file_diff';
 import {ImageDiffMode} from './ImageDiffModeSelector';
 import {filePairDisplayName} from './utils';
@@ -22,6 +22,10 @@ export function Root(props: Props) {
   const [diffOptions, setDiffOptions] = React.useState<Partial<DiffOptions>>({});
   const [showKeyboardHelp, setShowKeyboardHelp] = React.useState(false);
   const [showOptions, setShowOptions] = React.useState(false);
+  // An explicit list is better, unless there are a ton of files.
+  const [fileSelectorMode, setFileSelectorMode] = React.useState<FileSelectorMode>(
+    pairs.length <= 6 ? 'list' : 'dropdown',
+  );
 
   const history = useHistory();
   const selectIndex = React.useCallback(
@@ -49,6 +53,8 @@ export function Root(props: Props) {
         if (idx < pairs.length - 1) {
           selectIndex(idx + 1);
         }
+      } else if (e.code == 'KeyV') {
+        setFileSelectorMode(mode => (mode === 'dropdown' ? 'list' : 'dropdown'));
       } else if (e.code === 'Slash' && e.shiftKey) {
         setShowKeyboardHelp(val => !val);
       } else if (e.code === 'Escape') {
@@ -71,7 +77,13 @@ export function Root(props: Props) {
         isVisible={showOptions}
         setIsVisible={setShowOptions}
       />
-      <FileSelector selectedFileIndex={idx} filePairs={pairs} fileChangeHandler={selectIndex} />
+      <FileSelector
+        selectedFileIndex={idx}
+        filePairs={pairs}
+        fileChangeHandler={selectIndex}
+        mode={fileSelectorMode}
+        onChangeMode={setFileSelectorMode}
+      />
       {showKeyboardHelp ? (
         <KeyboardShortcuts
           onClose={() => {

ts/FileSelector.tsx

@@ -8,11 +8,13 @@ test('util.filePairDisplayName', () => {
       type: 'delete',
       a: 'dir/file.json',
       b: '',
+      num_add: 0,
+      num_delete: 0,
     }),
   ).toEqual('dir/file.json');
 
   const rename = (a: string, b: string) => {
-    return filePairDisplayName({...props, type: 'move', a, b});
+    return filePairDisplayName({...props, type: 'move', a, b, num_add: 0, num_delete: 0});
   };
   expect(rename('file.json', 'renamed.json')).toEqual('{file → renamed}.json');
   expect(rename('dir/file.json', 'dir/renamed.json')).toEqual('dir/{file → renamed}.json');

ts/Root.tsx

@@ -65,6 +65,9 @@ export function KeyboardShortcuts(props: KeyboardShortcutsProps) {
             <li>
               <kbd>p</kbd> Previous Diffhunk
             </li>
+            <li>
+              <kbd>v</kbd> Toggle file list / dropdown menu
+            </li>
             <li>
               <kbd>.</kbd> Show diff options
             </li>