Don't die on diffing file deletions (again) (#1028)

Makes `null` content (file deletion): 1. not identify as binary (so default diff style is nicer) 2. not break binary diff if it is Addresses regression(?) of #950
yeoman · Jul 24, 2017 · f8e46b0 · f8e46b0
1 parent edc2bf2
commit f8e46b0
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 1 deletion.
diff --git a/lib/util/binary-diff.js b/lib/util/binary-diff.js
@@ -8,7 +8,8 @@ const Table = require('cli-table');
 
 exports.isBinary = (existingFilePath, newFileContents) => {
   const existingHeader = readChunk.sync(existingFilePath, 0, 512);
-  return istextorbinary.isBinarySync(undefined, existingHeader) || istextorbinary.isBinarySync(undefined, newFileContents);
+  return istextorbinary.isBinarySync(undefined, existingHeader) ||
+    (newFileContents && istextorbinary.isBinarySync(undefined, newFileContents));
 };
 
 exports.diff = (existingFilePath, newFileContents) => {
@@ -19,6 +20,10 @@ exports.diff = (existingFilePath, newFileContents) => {
 
   let sizeDiff;
 
+  if (!newFileContents) {
+    newFileContents = Buffer.from([]);
+  }
+
   if (existingStat.size > newFileContents.length) {
     sizeDiff = '-';
   } else {

diff --git a/test/conflicter.js b/test/conflicter.js
@@ -155,6 +155,32 @@ describe('Conflicter', () => {
       });
     });
 
+    it('shows old content for deleted text files', done => {
+      const testAdapter = new TestAdapter({action: 'diff'});
+      const conflicter = new Conflicter(testAdapter);
+      const _prompt = testAdapter.prompt.bind(testAdapter);
+      const promptStub = sinon.stub(testAdapter, 'prompt', (prompts, resultHandler) => {
+        if (promptStub.calledTwice) {
+          const stubbedResultHandler = result => {
+            result.action = 'write';
+            return resultHandler(result);
+          };
+
+          return _prompt(prompts, stubbedResultHandler);
+        }
+        return _prompt(prompts, resultHandler);
+      });
+
+      conflicter.collision({
+        path: path.join(__dirname, 'fixtures/foo.js'),
+        contents: null
+      }, () => {
+        sinon.assert.neverCalledWithMatch(testAdapter.log.writeln, /Existing.*Replacement.*Diff/);
+        sinon.assert.called(testAdapter.diff);
+        done();
+      });
+    });
+
     it('displays custom diff for binary files', done => {
       const testAdapter = new TestAdapter({action: 'diff'});
       const conflicter = new Conflicter(testAdapter);
@@ -180,5 +206,31 @@ describe('Conflicter', () => {
         done();
       });
     });
+
+    it('displays custom diff for deleted binary files', done => {
+      const testAdapter = new TestAdapter({action: 'diff'});
+      const conflicter = new Conflicter(testAdapter);
+      const _prompt = testAdapter.prompt.bind(testAdapter);
+      const promptStub = sinon.stub(testAdapter, 'prompt', (prompts, resultHandler) => {
+        if (promptStub.calledTwice) {
+          const stubbedResultHandler = result => {
+            result.action = 'write';
+            return resultHandler(result);
+          };
+
+          return _prompt(prompts, stubbedResultHandler);
+        }
+        return _prompt(prompts, resultHandler);
+      });
+
+      conflicter.collision({
+        path: path.join(__dirname, 'fixtures/yeoman-logo.png'),
+        contents: null
+      }, () => {
+        sinon.assert.calledWithMatch(testAdapter.log.writeln, /Existing.*Replacement.*Diff/);
+        sinon.assert.notCalled(testAdapter.diff);
+        done();
+      });
+    });
   });
 });