commit c6894439c6ad9d1286f4bff335ddfa4cf31d2323
author Viktar Donich <viktard@google.com> Tue Feb 21 15:45:46 2017 -0800
committer Viktar Donich <viktard@google.com> Wed Feb 22 14:56:22 2017 -0800
tree 09d0baedd6d816a39273f38ffdb5857b51734c9a
parent 28670125754bcf274cd81711ecff434b3010fa04

Take tab size into account for line-wrapping

Previously, the logic was only checking exact line wrap boundaries and
missed tabs advancing over the boundary.

Also, line breaks were added inside span elements wrapping tabs, which
prevented correct rendering.

Bug: Issue 5597
Change-Id: I750547cb574c02965d5a30ba57f791841779297a

polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index d62dbf8..54a72f6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -480,20 +480,38 @@
     return index + 1;
   };
 
+  GrDiffBuilder.prototype._advancePastTagClose = function(html, index) {
+    while (index < html.length &&
+           html.charCodeAt(index) !== GrDiffBuilder.GREATER_THAN_CODE) {
+      index++;
+    }
+    return index + 1;
+  };
+
   GrDiffBuilder.prototype._addNewlines = function(text, html) {
     var htmlIndex = 0;
     var indices = [];
     var numChars = 0;
+    var prevHtmlIndex = 0;
     for (var i = 0; i < text.length; i++) {
       if (numChars > 0 && numChars % this._prefs.line_length === 0) {
         indices.push(htmlIndex);
       }
       htmlIndex = this._advanceChar(html, htmlIndex);
       if (text[i] === '\t') {
+        // Advance past tab closing tag.
+        htmlIndex = this._advancePastTagClose(html, htmlIndex);
+        // ~~ is a faster Math.floor
+        if (~~(numChars / this._prefs.line_length) !==
+            ~~((numChars + this._prefs.tab_size) / this._prefs.line_length)) {
+          // Tab crosses line limit - push it to the next line.
+          indices.push(prevHtmlIndex);
+        }
         numChars += this._prefs.tab_size;
       } else {
         numChars++;
       }
+      prevHtmlIndex = htmlIndex;
     }
     var result = html;
     // Since the result string is being altered in place, start from the end

polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index e5fe1d7..88ce477 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -97,7 +97,7 @@
       assert.equal(buttons[2].textContent, '+10↓');
     });
 
-    test('newlines', function() {
+    test('newlines 1', function() {
       var text = 'abcdef';
       assert.equal(builder._addNewlines(text, text), text);
       text = 'a'.repeat(20);
@@ -105,8 +105,10 @@
           'a'.repeat(10) +
           GrDiffBuilder.LINE_FEED_HTML +
           'a'.repeat(10));
+    });
 
-      text = '<span class="thumbsup">👍</span>';
+    test('newlines 2', function() {
+      var text = '<span class="thumbsup">👍</span>';
       var html = '&lt;span class=&quot;thumbsup&quot;&gt;👍&lt;&#x2F;span&gt;';
       assert.equal(builder._addNewlines(text, html),
           '&lt;span clas' +
@@ -116,10 +118,13 @@
           'p&quot;&gt;👍&lt;&#x2F;spa' +
           GrDiffBuilder.LINE_FEED_HTML +
           'n&gt;');
+    });
 
-      text = '01234\t56789';
-      assert.equal(builder._addNewlines(text, text),
-          '01234\t5' +
+    test('newlines 3', function() {
+      var text = '01234\t56789';
+      var html = '01234<span>\t</span>56789';
+      assert.equal(builder._addNewlines(text, html),
+          '01234<span>\t</span>5' +
           GrDiffBuilder.LINE_FEED_HTML +
           '6789');
     });
@@ -154,6 +159,17 @@
       });
     });
 
+    test('_createTextEl linewrap with tabs', function() {
+      var text = _.times(7, _.constant('\t')).join('') + '!';
+      var line = {text: text, highlights: []};
+      var el = builder._createTextEl(line);
+      var tabEl = el.querySelector('.contentText > .br');
+      assert.isOk(tabEl);
+      assert.equal(
+          el.querySelector('.contentText .tab:nth-child(2)').nextSibling,
+          tabEl);
+    });
+
     test('text length with tabs and unicode', function() {
       assert.equal(builder._textLength('12345', 4), 5);
       assert.equal(builder._textLength('\t\t12', 4), 10);