Sort out behaviour of newlineIsToken and ignoreWhitespace

README.md

@@ -48,21 +48,13 @@ Broadly, jsdiff's diff functions all take an old text and a new text and perform
 * `Diff.diffLines(oldStr, newStr[, options])` - diffs two blocks of text, treating each line as a token.
 
     Options
-    * `ignoreWhitespace`: `true` to strip all leading and trailing whitespace characters from each line before performing the diff. Defaults to `false`.
+    * `ignoreWhitespace`: `true` to ignore leading and trailing whitespace characters when checking if two lines are equal. Defaults to `false`.
     * `stripTrailingCr`: `true` to remove all trailing CR (`\r`) characters before performing the diff. Defaults to `false`.
       This helps to get a useful diff when diffing UNIX text files against Windows text files.
     * `newlineIsToken`: `true` to treat the newline character at the end of each line as its own token. This allows for changes to the newline structure to occur independently of the line content and to be treated as such. In general this is the more human friendly form of `diffLines`; the default behavior with this option turned off is better suited for patches and other computer friendly output. Defaults to `false`.
 
     Returns a list of [change objects](#change-objects).
 
-* `Diff.diffTrimmedLines(oldStr, newStr[, options])` - diffs two blocks of text, comparing line by line, after stripping leading and trailing whitespace. Equivalent to calling `diffLines` with `ignoreWhitespace: true`.
-
-    Options
-    * `stripTrailingCr`: Same as in `diffLines`. Defaults to `false`.
-    * `newlineIsToken`: Same as in `diffLines`. Defaults to `false`.
-
-    Returns a list of [change objects](#change-objects).
-
 * `Diff.diffSentences(oldStr, newStr[, options])` - diffs two blocks of text, treating each sentence as a token.
 
     Returns a list of [change objects](#change-objects).

src/diff/line.js

@@ -23,17 +23,40 @@ lineDiff.tokenize = function(value) {
     if (i % 2 && !this.options.newlineIsToken) {
       retLines[retLines.length - 1] += line;
     } else {
-      if (this.options.ignoreWhitespace) {
-        line = line.trim();
-      }
       retLines.push(line);
     }
   }
 
   return retLines;
 };
 
+lineDiff.equals = function(left, right) {
+  // If we're ignoring whitespace, we need to normalise lines by stripping
+  // whitespace before checking equality. (This has an annoying interaction
+  // with newlineIsToken that requires special handling: if newlines get their
+  // own token, then we DON'T want to trim the *newlines* tokens down to empty
+  // strings, since this would cause us to treat whitespace-only line content
+  // as equal to a separator between lines, which would be weird and
+  // inconsistent with the documented behavior of the options.)
+  if (this.options.ignoreWhitespace) {
+    if (!this.options.newlineIsToken || !left.includes('\n')) {
+      left = left.trim();
+    }
+    if (!this.options.newlineIsToken || !right.includes('\n')) {
+      right = right.trim();
+    }
+  }
+  return Diff.prototype.equals.call(this, left, right);
+};
+
 export function diffLines(oldStr, newStr, callback) { return lineDiff.diff(oldStr, newStr, callback); }
+
+// Kept for backwards compatibility. This is a rather arbitrary wrapper method
+// that just calls `diffLines` with `ignoreWhitespace: true`. It's confusing to
+// have two ways to do exactly the same thing in the API, so we no longer
+// document this one (library users should explicitly use `diffLines` with
+// `ignoreWhitespace: true` instead) but we keep it around to maintain
+// compatibility with code that used old versions.
 export function diffTrimmedLines(oldStr, newStr, callback) {
   let options = generateOptions(callback, {ignoreWhitespace: true});
   return lineDiff.diff(oldStr, newStr, options);

test/diff/line.js

@@ -12,7 +12,7 @@ describe('diff/line', function() {
         'line\nnew value\nline');
       expect(convertChangesToXML(diffResult)).to.equal('line\n<del>old value\n</del><ins>new value\n</ins>line');
     });
-    it('should the same lines in diff', function() {
+    it('should treat identical lines as equal', function() {
       const diffResult = diffLines(
         'line\nvalue\nline',
         'line\nvalue\nline');
@@ -128,7 +128,7 @@ describe('diff/line', function() {
         'line\nnew value\nline');
       expect(convertChangesToXML(diffResult)).to.equal('line\n<del>old value\n</del><ins>new value\n</ins>line');
     });
-    it('should the same lines in diff', function() {
+    it('should treat identical lines as equal', function() {
       const diffResult = diffTrimmedLines(
         'line\nvalue\nline',
         'line\nvalue\nline');
@@ -138,16 +138,39 @@ describe('diff/line', function() {
     it('should ignore leading and trailing whitespace', function() {
       const diffResult = diffTrimmedLines(
         'line\nvalue \nline',
-        'line\nvalue\nline');
-      expect(convertChangesToXML(diffResult)).to.equal('line\nvalue\nline');
+        'line \nvalue\nline');
+      expect(convertChangesToXML(diffResult)).to.equal('line \nvalue\nline');
+    });
+
+    it('should not consider adding whitespace to an empty line an insertion', function() {
+      const diffResult = diffTrimmedLines('foo\n\nbar', 'foo\n \nbar');
+      expect(convertChangesToXML(diffResult)).to.equal('foo\n \nbar');
     });
 
     it('should handle windows line endings', function() {
       const diffResult = diffTrimmedLines(
         'line\r\nold value \r\nline',
         'line\r\nnew value\r\nline');
-      expect(convertChangesToXML(diffResult)).to.equal('line\r\n<del>old value\r\n</del><ins>new value\r\n</ins>line');
+      expect(convertChangesToXML(diffResult)).to.equal('line\r\n<del>old value \r\n</del><ins>new value\r\n</ins>line');
     });
+
+    it('should be compatible with newlineIsToken', function() {
+      const diffResult = diffTrimmedLines(
+        'line1\nline2\n   \nline4\n \n',
+        'line1\nline2\n\n\nline4\n   \n',
+        {newlineIsToken: true}
+      );
+      expect(convertChangesToXML(diffResult)).to.equal('line1\nline2\n<del>   </del>\n<ins>\n</ins>line4\n   \n');
+    });
+
+    it(
+      'should not consider adding whitespace to an empty line an insertion ' +
+      'even in newlineIsToken mode where a token may be an empty string',
+      function() {
+        const diffResult = diffTrimmedLines('foo\n\nbar', 'foo\n \nbar', {newlineIsToken: true});
+        expect(convertChangesToXML(diffResult)).to.equal('foo\n \nbar');
+      }
+    );
   });
 
   describe('#diffLinesNL', function() {