Simplify highlight for leading and trailing spaces in jest-diff (jest…

…js#4553)
tabrindle · Oct 2, 2017 · 78488e9 · 78488e9
1 parent 2a212fc
commit 78488e9
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 56 deletions.
diff --git a/packages/jest-diff/src/__tests__/__snapshots__/diff.test.js.snap b/packages/jest-diff/src/__tests__/__snapshots__/diff.test.js.snap
@@ -8,10 +8,10 @@ exports[`background color of spaces cyan for inchanged (expanded) 1`] = `
 <red>+   <p></>
 <cyan>      <span></>
 <cyan>        following string consists of a space:</>
-<cyan>        <bgCyan> </></>
-<cyan>        <bgCyan> </>line has preceding space only</>
-<cyan>        <bgCyan> </>line has both preceding and following space<bgCyan> </></>
-<cyan>        line has following space only<bgCyan> </></>
+<cyan>        <inverse> </></>
+<cyan>        <inverse> </>line has preceding space only</>
+<cyan>        <inverse> </>line has both preceding and following space<inverse> </></>
+<cyan>        line has following space only<inverse> </></>
 <cyan>      </span></>
 <red>+   </p></>
 <dim>  </div></>"
@@ -24,45 +24,45 @@ exports[`background color of spaces green for removed (expanded) 1`] = `
 <dim>  <div></>
 <dim>    <span></>
 <green>-     following string consists of a space:</>
-<green>-     <bgGreen> </></>
-<green>-     <bgGreen> </>line has preceding space only</>
-<green>-     <bgGreen> </>line has both preceding and following space<bgGreen> </></>
-<green>-     line has following space only<bgGreen> </></>
+<green>-     <inverse> </></>
+<green>-     <inverse> </>line has preceding space only</>
+<green>-     <inverse> </>line has both preceding and following space<inverse> </></>
+<green>-     line has following space only<inverse> </></>
 <red>+     </>
 <dim>    </span></>
 <dim>  </div></>"
 `;
 
-exports[`background color of spaces no color for unchanged (expanded) 1`] = `
+exports[`background color of spaces red for added (expanded) 1`] = `
 "<green>- Expected</>
 <red>+ Received</>
 
 <dim>  <div></>
-<green>-   <span></>
-<red>+   <p></>
-<dim>      following string consists of a space:</>
-<dim>       </>
-<dim>       line has preceding space only</>
-<dim>       line has both preceding and following space </>
-<dim>      line has following space only </>
-<green>-   </span></>
-<red>+   </p></>
+<dim>    <span></>
+<green>-     </>
+<red>+     following string consists of a space:</>
+<red>+     <inverse> </></>
+<red>+     <inverse> </>line has preceding space only</>
+<red>+     <inverse> </>line has both preceding and following space<inverse> </></>
+<red>+     line has following space only<inverse> </></>
+<dim>    </span></>
 <dim>  </div></>"
 `;
 
-exports[`background color of spaces red for added (expanded) 1`] = `
+exports[`background color of spaces yellow for unchanged (expanded) 1`] = `
 "<green>- Expected</>
 <red>+ Received</>
 
 <dim>  <div></>
-<dim>    <span></>
-<green>-     </>
-<red>+     following string consists of a space:</>
-<red>+     <bgRed> </></>
-<red>+     <bgRed> </>line has preceding space only</>
-<red>+     <bgRed> </>line has both preceding and following space<bgRed> </></>
-<red>+     line has following space only<bgRed> </></>
-<dim>    </span></>
+<green>-   <span></>
+<red>+   <p></>
+<dim>      following string consists of a space:</>
+<dim>      <bgYellow> </></>
+<dim>      <bgYellow> </>line has preceding space only</>
+<dim>      <bgYellow> </>line has both preceding and following space<bgYellow> </></>
+<dim>      line has following space only<bgYellow> </></>
+<green>-   </span></>
+<red>+   </p></>
 <dim>  </div></>"
 `;
 

diff --git a/packages/jest-diff/src/__tests__/diff.test.js b/packages/jest-diff/src/__tests__/diff.test.js
@@ -741,22 +741,22 @@ describe('background color of spaces', () => {
       expect(diff(examples, baseline, unexpanded)).toBe(received);
     });
   });
-  describe('no color for unchanged', () => {
-    const received = diff(examples, unchanged, expanded);
+  describe('red for added', () => {
+    const received = diff(baseline, examples, expanded);
     test('(expanded)', () => {
       expect(received).toMatchSnapshot();
     });
     test('(unexpanded)', () => {
-      expect(diff(examples, unchanged, unexpanded)).toBe(received);
+      expect(diff(baseline, examples, unexpanded)).toBe(received);
     });
   });
-  describe('red for added', () => {
-    const received = diff(baseline, examples, expanded);
+  describe('yellow for unchanged', () => {
+    const received = diff(examples, unchanged, expanded);
     test('(expanded)', () => {
       expect(received).toMatchSnapshot();
     });
     test('(unexpanded)', () => {
-      expect(diff(baseline, examples, unexpanded)).toBe(received);
+      expect(diff(examples, unchanged, unexpanded)).toBe(received);
     });
   });
 });

diff --git a/packages/jest-diff/src/diff_strings.js b/packages/jest-diff/src/diff_strings.js
@@ -64,23 +64,19 @@ const getColor = (digit: DIFF_DIGIT, onlyIndentationChanged?: boolean) => {
 // Do NOT color leading or trailing spaces if original lines are equal:
 
 // Background color for leading or trailing spaces.
-const getBgColor = (digit: DIFF_DIGIT) => {
-  if (digit === -1) {
-    return chalk.bgGreen; // removed
-  }
-  if (digit === 1) {
-    return chalk.bgRed; // added
-  }
-  return chalk.bgCyan; // only indentation changed
-};
+const getBgColor = (digit: DIFF_DIGIT, onlyIndentationChanged?: boolean) =>
+  digit === 0 && !onlyIndentationChanged ? chalk.bgYellow : chalk.inverse;
 
 // ONLY trailing if expected value is snapshot or multiline string.
-const highlightTrailingWhitespace = (line: string, bgColor: Function): string =>
+const highlightTrailingSpaces = (line: string, bgColor: Function): string =>
   line.replace(/\s+$/, bgColor('$&'));
 
 // BOTH leading AND trailing if expected value is data structure.
-const highlightWhitespace = (line: string, bgColor: Function): string =>
-  highlightTrailingWhitespace(line.replace(/^\s+/, bgColor('$&')), bgColor);
+const highlightLeadingTrailingSpaces = (
+  line: string,
+  bgColor: Function,
+): string =>
+  highlightTrailingSpaces(line.replace(/^\s+/, bgColor('$&')), bgColor);
 
 const getAnnotation = (options: ?DiffOptions): string =>
   chalk.green('- ' + ((options && options.aAnnotation) || 'Expected')) +
@@ -119,19 +115,16 @@ const formatLine = (
         ' ' +
         // Prepend indentation spaces from original to compared line.
         lineOriginal.slice(0, lineOriginal.length - lineCompared.length) +
-        (digit !== 0 || onlyIndentationChanged
-          ? highlightWhitespace(lineCompared, getBgColor(digit))
-          : lineCompared),
+        highlightLeadingTrailingSpaces(
+          lineCompared,
+          getBgColor(digit, onlyIndentationChanged),
+        ),
     );
   }
 
   // Format compared line when expected is snapshot or multiline string.
   return getColor(digit)(
-    char +
-      ' ' +
-      (digit !== 0
-        ? highlightTrailingWhitespace(lineCompared, getBgColor(digit))
-        : lineCompared),
+    char + ' ' + highlightTrailingSpaces(lineCompared, getBgColor(digit)),
   );
 };
 

diff --git a/packages/pretty-format/src/plugins/convert_ansi.js b/packages/pretty-format/src/plugins/convert_ansi.js
@@ -23,7 +23,7 @@ const toHumanReadableAnsi = text => {
       case style.yellow.close:
       case style.bgRed.close:
       case style.bgGreen.close:
-      case style.bgCyan.close:
+      case style.bgYellow.close:
       case style.inverse.close:
       case style.dim.close:
       case style.bold.close:
@@ -46,8 +46,8 @@ const toHumanReadableAnsi = text => {
         return '<bgRed>';
       case style.bgGreen.open:
         return '<bgGreen>';
-      case style.bgCyan.open:
-        return '<bgCyan>';
+      case style.bgYellow.open:
+        return '<bgYellow>';
       case style.inverse.open:
         return '<inverse>';
       case style.dim.open: