Add preference for line wrapping in diff preferences

Previously in Polygerrit, diff views were always displayed in the width
specified in diff preferences. This change gives the option to wrap
lines instead, which takes precedence over column width (the column
width option is hidden when line wrapping is selected), and fits the
diff view to screen.

The gerrit API already supports the 'lineWrapping' preference so this
change uses that already existing option.

Feature: Issue 4809
Change-Id: I0d9e292739b5910abfd04af63ec4c745bf06e446

polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html

@@ -546,6 +546,7 @@ limitations under the License.
                   assert.isTrue(scrollStub.called);
                   document.body.style.height =
                       originalHeight + 'px';
+                  scrollStub.restore();
                   done();
                 });
             document.body.style.height = '10000px';

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

@@ -34,6 +34,29 @@
     return sectionEl;
   };
 
+  GrDiffBuilderSideBySide.prototype.addColumns = function(outputEl, fontSize) {
+    var width = fontSize * 4;
+    var colgroup = document.createElement('colgroup');
+
+    // Add left-side line number.
+    var col = document.createElement('col');
+    col.setAttribute('width', width);
+    colgroup.appendChild(col);
+
+    // Add left-side content.
+    colgroup.appendChild(document.createElement('col'));
+
+    // Add right-side line number.
+    col = document.createElement('col');
+    col.setAttribute('width', width);
+    colgroup.appendChild(col);
+
+    // Add right-side content.
+    colgroup.appendChild(document.createElement('col'));
+
+    outputEl.appendChild(colgroup);
+  };
+
   GrDiffBuilderSideBySide.prototype._createRow = function(section, leftLine,
       rightLine) {
     var row = this._createElement('tr');

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

@@ -33,6 +33,26 @@
     return sectionEl;
   };
 
+  GrDiffBuilderUnified.prototype.addColumns = function(outputEl, fontSize) {
+    var width = fontSize * 4;
+    var colgroup = document.createElement('colgroup');
+
+    // Add left-side line number.
+    var col = document.createElement('col');
+    col.setAttribute('width', width);
+    colgroup.appendChild(col);
+
+    // Add right-side line number.
+    col = document.createElement('col');
+    col.setAttribute('width', width);
+    colgroup.appendChild(col);
+
+    // Add the content.
+    colgroup.appendChild(document.createElement('col'));
+
+    outputEl.appendChild(colgroup);
+  };
+
   GrDiffBuilderUnified.prototype._createRow = function(section, line) {
     var row = this._createElement('tr', line.type);
     var lineEl = this._createLineEl(line, line.beforeNumber,

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

@@ -126,6 +126,7 @@ limitations under the License.
           this.$.processor.keyLocations = this._getCommentLocations(comments);
 
           this._clearDiffContent();
+          this._builder.addColumns(this.diffElement, prefs.font_size);
 
           var reporting = this.$.reporting;
 

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

@@ -65,7 +65,20 @@
 
   var PARTIAL_CONTEXT_AMOUNT = 10;
 
-  GrDiffBuilder.prototype.buildSectionElement = function(group) {
+  /**
+   * Abstract method
+   * @param {string} outputEl
+   * @param {number} fontSize
+   */
+  GrDiffBuilder.prototype.addColumns = function() {
+    throw Error('Subclasses must implement addColumns');
+  };
+
+  /**
+   * Abstract method
+   * @param {Object} group
+   */
+  GrDiffBuilder.prototype.buildSectionElement = function() {
     throw Error('Subclasses must implement buildGroupElement');
   };
 
@@ -377,7 +390,8 @@
     var html = util.escapeHTML(text);
     html = this._addTabWrappers(html, this._prefs.tab_size);
 
-    if (this._textLength(text, this._prefs.tab_size) >
+    if (!this._prefs.line_wrapping &&
+        this._textLength(text, this._prefs.tab_size) >
         this._prefs.line_length) {
       html = this._addNewlines(text, html);
     }

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

@@ -124,6 +124,36 @@ limitations under the License.
           '6789');
     });
 
+    test('_addNewlines not called if line_wrapping is true', function(done) {
+      builder._prefs = {line_wrapping: true, tab_size: 4, line_length: 50};
+      var text = (new Array(52)).join('a');
+
+      var line = {text: text, highlights: []};
+      var newLineStub = sinon.stub(builder, '_addNewlines');
+      builder._createTextEl(line);
+      flush(function() {
+        assert.isFalse(newLineStub.called);
+        newLineStub.restore();
+        done();
+      });
+    });
+
+    test('_addNewlines called if line_wrapping is true and meets other ' +
+        'conditions', function(done) {
+      builder._prefs = {line_wrapping: false, tab_size: 4, line_length: 50};
+      var text = (new Array(52)).join('a');
+
+      var line = {text: text, highlights: []};
+      var newLineStub = sinon.stub(builder, '_addNewlines');
+      builder._createTextEl(line);
+
+      flush(function() {
+        assert.isTrue(newLineStub.called);
+        newLineStub.restore();
+        done();
+      });
+    });
+
     test('text length with tabs and unicode', function() {
       assert.equal(builder._textLength('12345', 4), 5);
       assert.equal(builder._textLength('\t\t12', 4), 10);
@@ -531,8 +561,10 @@ limitations under the License.
     suite('rendering', function() {
       var content;
       var outputEl;
+      var sandbox;
 
       setup(function(done) {
+        sandbox = sinon.sandbox.create();
         var prefs = {
           line_length: 10,
           show_tabs: true,
@@ -553,19 +585,15 @@ limitations under the License.
           },
         ];
         stub('gr-reporting', {
-          time: sinon.stub(),
-          timeEnd: sinon.stub(),
+          time: sandbox.stub(),
+          timeEnd: sandbox.stub(),
         });
         element = fixture('basic');
         outputEl = element.queryEffectiveChildren('#diffTable');
-        var renderHandler = function() {
-          element.removeEventListener('render', renderHandler);
-          done();
-        };
-        element.addEventListener('render', renderHandler);
-        sinon.stub(element, '_getDiffBuilder', function() {
+        sandbox.stub(element, '_getDiffBuilder', function() {
           var builder = new GrDiffBuilder(
               {content: content}, {left: [], right: []}, prefs, outputEl);
+          sandbox.stub(builder, 'addColumns');
           builder.buildSectionElement = function(group) {
             var section = document.createElement('stub');
             section.textContent = group.lines.reduce(function(acc, line) {
@@ -576,21 +604,23 @@ limitations under the License.
           return builder;
         });
         element.diff = {content: content};
-        element.render({left: [], right: []}, prefs);
+        element.render({left: [], right: []}, prefs).then(done);
+      });
+
+      teardown(function() {
+        sandbox.restore();
       });
 
       test('reporting', function(done) {
         var timeStub = element.$.reporting.time;
         var timeEndStub = element.$.reporting.timeEnd;
-        flush(function() {
-          assert.isTrue(timeStub.calledWithExactly('Diff Total Render'));
-          assert.isTrue(timeStub.calledWithExactly('Diff Content Render'));
-          assert.isTrue(timeStub.calledWithExactly('Diff Syntax Render'));
-          assert.isTrue(timeEndStub.calledWithExactly('Diff Total Render'));
-          assert.isTrue(timeEndStub.calledWithExactly('Diff Content Render'));
-          assert.isTrue(timeEndStub.calledWithExactly('Diff Syntax Render'));
-          done();
-        });
+        assert.isTrue(timeStub.calledWithExactly('Diff Total Render'));
+        assert.isTrue(timeStub.calledWithExactly('Diff Content Render'));
+        assert.isTrue(timeStub.calledWithExactly('Diff Syntax Render'));
+        assert.isTrue(timeEndStub.calledWithExactly('Diff Total Render'));
+        assert.isTrue(timeEndStub.calledWithExactly('Diff Content Render'));
+        assert.isTrue(timeEndStub.calledWithExactly('Diff Syntax Render'));
+        done();
       });
 
       test('renderSection', function() {
@@ -602,6 +632,11 @@ limitations under the License.
         assert.equal(section.innerHTML, prevInnerHTML);
       });
 
+      test('addColumns is called', function(done) {
+        element.render({left: [], right: []}, {}).then(done);
+        assert.isTrue(element._builder.addColumns.called);
+      });
+
       test('getSectionsByLineRange one line', function() {
         var section = outputEl.querySelector('stub:nth-of-type(2)');
         var sections = element._builder.getSectionsByLineRange(1, 1, 'left');
@@ -622,8 +657,7 @@ limitations under the License.
 
       test('render-start and render are fired', function(done) {
         var fireStub = sinon.stub(element, 'fire');
-        element.render({left: [], right: []}, {});
-        flush(function() {
+        element.render({left: [], right: []}, {}).then(function() {
           assert.isTrue(fireStub.calledWithExactly('render-start'));
           assert.isTrue(fireStub.calledWithExactly('render'));
           done();

polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html

@@ -87,7 +87,15 @@ limitations under the License.
         </select>
       </div>
       <div class="pref">
-        <label for="columnsInput">Columns</label>
+        <label for="lineWrappingInput">Fit to Screen</label>
+        <input
+            is="iron-input"
+            type="checkbox"
+            id="lineWrappingInput"
+            on-tap="_handlelineWrappingTap">
+      </div>
+      <div class="pref" id="columnsPref" hidden$="[[_newPrefs.line_wrapping]]">
+        <label for="columnsInput">Diff Width</label>
         <input is="iron-input" type="number" id="columnsInput"
             prevent-invalid-input
             allowed-pattern="[0-9]"

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

@@ -72,6 +72,7 @@
       this._newPrefs = Object.assign({}, prefs);
       this.$.contextSelect.value = prefs.context;
       this.$.showTabsInput.checked = prefs.show_tabs;
+      this.$.lineWrappingInput.checked = prefs.line_wrapping;
       this.$.syntaxHighlightInput.checked = prefs.syntax_highlighting;
     },
 
@@ -95,6 +96,10 @@
           Polymer.dom(e).rootTarget.checked);
     },
 
+    _handlelineWrappingTap: function(e) {
+      this.set('_newPrefs.line_wrapping', Polymer.dom(e).rootTarget.checked);
+    },
+
     _handleSave: function() {
       this.prefs = this._newPrefs;
       this.localPrefs = this._newLocalPrefs;

polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences_test.html

@@ -56,15 +56,29 @@ limitations under the License.
       element.$.tabSizeInput.bindValue = 4;
       MockInteractions.tap(element.$.showTabsInput);
       MockInteractions.tap(element.$.syntaxHighlightInput);
+      MockInteractions.tap(element.$.lineWrappingInput);
 
       assert.equal(element._newPrefs.context, 50);
       assert.equal(element._newPrefs.font_size, 10);
       assert.equal(element._newPrefs.line_length, 80);
       assert.equal(element._newPrefs.tab_size, 4);
       assert.isFalse(element._newPrefs.show_tabs);
+      assert.isTrue(element._newPrefs.line_wrapping);
       assert.isFalse(element._newPrefs.syntax_highlighting);
     });
 
+    test('clicking fit to screen hides line length input', function() {
+      element.prefs = {line_wrapping: false};
+
+      assert.isFalse(element.$.columnsPref.hidden);
+
+      MockInteractions.tap(element.$.lineWrappingInput);
+      assert.isTrue(element.$.columnsPref.hidden);
+
+      MockInteractions.tap(element.$.lineWrappingInput);
+      assert.isFalse(element.$.columnsPref.hidden);
+    });
+
     test('clicking save button calls _handleSave function', function() {
       var savePrefs = sinon.stub(element, '_handleSave');
       MockInteractions.tap(element.$.saveButton);

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

@@ -87,8 +87,13 @@ limitations under the License.
       .content {
         background-color: #fff;
       }
+      .full-width {
+        width: 100%;
+      }
       .lineNum,
       .content {
+        /* Set font size based the user's diff preference. */
+        font-size: var(--font-size, 12px);
         vertical-align: top;
         white-space: pre;
       }
@@ -113,11 +118,7 @@ limitations under the License.
            allows them to shrink. */
         max-width: var(--content-width, 80ch);
         min-width: var(--content-width, 80ch);
-      }
-      .content,
-      .lineNum {
-        /* Set font size based the user's diff preference. */
-        font-size: var(--font-size, 12px);
+        width: var(--content-width, 80ch);
       }
       .content.add .intraline,
       .content.add.darkHighlight {
@@ -137,6 +138,10 @@ limitations under the License.
         background-color: #fef;
         color: #849;
       }
+      .contentText {
+        white-space: pre-wrap;
+        word-wrap: break-word;
+      }
       .contextControl gr-button {
         display: inline-block;
         font-family: var(--monospace-font-family);
@@ -171,10 +176,11 @@ limitations under the License.
               comments="[[_comments]]"
               diff="[[_diff]]"
               view-mode="[[viewMode]]"
+              line-wrapping="[[lineWrapping]]"
               is-image-diff="[[isImageDiff]]"
               base-image="[[_baseImage]]"
               revision-image="[[_revisionImage]]">
-            <table id="diffTable"></table>
+            <table id="diffTable" class$="[[_diffTableClass]]"></table>
           </gr-diff-builder>
         </gr-diff-highlight>
       </gr-diff-selection>

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

@@ -66,12 +66,21 @@
         type: Boolean,
         value: false,
       },
+      lineWrapping: {
+        type: Boolean,
+        value: false,
+        observer: '_lineWrappingObserver',
+      },
       viewMode: {
         type: String,
         value: DiffViewMode.SIDE_BY_SIDE,
         observer: '_viewModeObserver',
       },
       _diff: Object,
+      _diffTableClass: {
+        type: String,
+        value: '',
+      },
       _comments: Object,
       _baseImage: Object,
       _revisionImage: Object,
@@ -356,9 +365,21 @@
       this._prefsChanged(this.prefs);
     },
 
+    _lineWrappingObserver: function() {
+      this._prefsChanged(this.prefs);
+    },
+
     _prefsChanged: function(prefs) {
       if (!prefs) { return; }
-      this.customStyle['--content-width'] = prefs.line_length + 'ch';
+      if (prefs.line_wrapping) {
+        this._diffTableClass = 'full-width';
+        if (this.viewMode === 'SIDE_BY_SIDE') {
+          this.customStyle['--content-width'] = 'none';
+        }
+      } else {
+        this._diffTableClass = '';
+        this.customStyle['--content-width'] = prefs.line_length + 'ch';
+      }
 
       if (!!prefs.font_size) {
         this.customStyle['--font-size'] = prefs.font_size + 'px';

polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html

@@ -210,7 +210,17 @@ limitations under the License.
             </span>
           </section>
           <section>
-            <span class="title">Columns</span>
+            <span class="title">Fit to Screen</span>
+            <span class="value">
+              <input
+                  id="lineWrapping"
+                  type="checkbox"
+                  checked$="[[_diffPrefs.line_wrapping]]"
+                  on-change="_handleLineWrappingChanged">
+            </span>
+          </section>
+          <section id="columnsPref" hidden$="[[_diffPrefs.line_wrapping]]">
+            <span class="title">Diff Width</span>
             <span class="value">
               <input
                   is="iron-input"

polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js

@@ -206,6 +206,10 @@
       }.bind(this));
     },
 
+    _handleLineWrappingChanged: function() {
+      this.set('_diffPrefs.line_wrapping', this.$.lineWrapping.checked);
+    },
+
     _handleShowTabsChanged: function() {
       this.set('_diffPrefs.show_tabs', this.$.showTabs.checked);
     },

polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html

@@ -95,6 +95,7 @@ limitations under the License.
         font_size: 12,
         line_length: 100,
         cursor_blink_rate: 0,
+        line_wrapping: false,
         intraline_difference: true,
         show_line_endings: true,
         show_tabs: true,
@@ -189,7 +190,7 @@ limitations under the License.
       // Rendered with the expected preferences selected.
       assert.equal(valueOf('Context', 'diffPreferences')
           .firstElementChild.bindValue, diffPreferences.context);
-      assert.equal(valueOf('Columns', 'diffPreferences')
+      assert.equal(valueOf('Diff Width', 'diffPreferences')
           .firstElementChild.bindValue, diffPreferences.line_length);
       assert.equal(valueOf('Tab Width', 'diffPreferences')
           .firstElementChild.bindValue, diffPreferences.tab_size);
@@ -197,6 +198,8 @@ limitations under the License.
           .firstElementChild.bindValue, diffPreferences.font_size);
       assert.equal(valueOf('Show Tabs', 'diffPreferences')
           .firstElementChild.checked, diffPreferences.show_tabs);
+      assert.equal(valueOf('Fit to Screen', 'diffPreferences')
+          .firstElementChild.checked, diffPreferences.line_wrapping);
 
       assert.isFalse(element._diffPrefsChanged);
 
@@ -221,6 +224,16 @@ limitations under the License.
       });
     });
 
+    test('columns input is hidden with fit to scsreen is selected', function() {
+      assert.isFalse(element.$.columnsPref.hidden);
+
+      MockInteractions.tap(element.$.lineWrapping);
+      assert.isTrue(element.$.columnsPref.hidden);
+
+      MockInteractions.tap(element.$.lineWrapping);
+      assert.isFalse(element.$.columnsPref.hidden);
+    });
+
     test('menu', function(done) {
       assert.isFalse(element._menuChanged);
       assert.isFalse(element._prefsChanged);

polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js

@@ -195,6 +195,7 @@
           ignore_whitespace: 'IGNORE_NONE',
           intraline_difference: true,
           line_length: 100,
+          line_wrapping: false,
           show_line_endings: true,
           show_tabs: true,
           show_whitespace_errors: true,