Merge "Show whether diff sides lack trailing newlines"

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

@@ -249,6 +249,13 @@ limitations under the License.
         background-position: var(--line-limit) 0;
         background-repeat: repeat-y;
       }
+      .newlineWarning {
+        color: var(--deemphasized-text-color);
+        text-align: center;
+      }
+      .newlineWarning.hidden {
+        display: none;
+      }
     </style>
     <style include="gr-syntax-theme"></style>
     <div id="diffHeader" hidden$="[[_computeDiffHeaderHidden(_diffHeaderItems)]]">
@@ -288,6 +295,9 @@ limitations under the License.
         </gr-diff-highlight>
       </gr-diff-selection>
     </div>
+    <div class$="[[_computeNewlineWarningClass(_newlineWarning, _loading)]]">
+      [[_newlineWarning]]
+    </div>
     <div id="sizeWarning" class$="[[_computeWarningClass(_showWarning)]]">
       <p>
         Prevented render because "Whole file" is enabled and this diff is very

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

@@ -27,6 +27,9 @@
   const EVENT_ZERO_REBASE = 'rebase-percent-zero';
   const EVENT_NONZERO_REBASE = 'rebase-percent-nonzero';
 
+  const NO_NEWLINE_BASE = 'No newline at end of base file.';
+  const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
+
   const DiffViewMode = {
     SIDE_BY_SIDE: 'SIDE_BY_SIDE',
     UNIFIED: 'UNIFIED_DIFF',
@@ -120,6 +123,11 @@
        */
       lineOfInterest: Object,
 
+      _loading: {
+        type: Boolean,
+        value: false,
+      },
+
       _loggedIn: {
         type: Boolean,
         value: false,
@@ -169,6 +177,11 @@
         type: Number,
         computed: '_computeParentIndex(patchRange.*)',
       },
+
+      _newlineWarning: {
+        type: String,
+        computed: '_computeNewlineWarning(_diff)',
+      },
     },
 
     behaviors: [
@@ -196,6 +209,7 @@
 
     /** @return {!Promise} */
     reload() {
+      this._loading = true;
       this.cancel();
       this.clearBlame();
       this._safetyBypass = null;
@@ -214,6 +228,8 @@
           return this._renderDiffTable();
         }
         return Promise.resolve();
+      }).then(() => {
+        this._loading = false;
       });
     },
 
@@ -857,5 +873,89 @@
     expandAllContext() {
       this._handleFullBypass();
     },
+
+    /**
+     * Find the last chunk for the given side.
+     * @param {!Object} diff
+     * @param {boolean} leftSide true if checking the base of the diff,
+     *     false if testing the revision.
+     * @return {Object|null} returns the chunk object or null if there was
+     *     no chunk for that side.
+     */
+    _lastChunkForSide(diff, leftSide) {
+      if (!diff.content.length) { return null; }
+
+      let chunkIndex = diff.content.length;
+      let chunk;
+
+      // Walk backwards until we find a chunk for the given side.
+      do {
+        chunkIndex--;
+        chunk = diff.content[chunkIndex];
+      } while (
+          // We haven't reached the beginning.
+          chunkIndex >= 0 &&
+
+          // The chunk doesn't have both sides.
+          !chunk.ab &&
+
+          // The chunk doesn't have the given side.
+          ((leftSide && !chunk.a) || (!leftSide && !chunk.b)));
+
+      // If we reached the beginning of the diff and failed to find a chunk
+      // with the given side, return null.
+      if (chunkIndex === -1) { return null; }
+
+      return chunk;
+    },
+
+    /**
+     * Check whether the specified side of the diff has a trailing newline.
+     * @param {!Object} diff
+     * @param {boolean} leftSide true if checking the base of the diff,
+     *     false if testing the revision.
+     * @return {boolean|null} Return true if the side has a trailing newline.
+     *     Return false if it doesn't. Return null if not applicable (for
+     *     example, if the diff has no content on the specified side).
+     */
+    _hasTrailingNewlines(diff, leftSide) {
+      const chunk = this._lastChunkForSide(diff, leftSide);
+      if (!chunk) { return null; }
+      let lines;
+      if (chunk.ab) {
+        lines = chunk.ab;
+      } else {
+        lines = leftSide ? chunk.a : chunk.b;
+      }
+      return lines[lines.length - 1] === '';
+    },
+
+    /**
+     * @param {!Object} diff
+     * @return {string|null}
+     */
+    _computeNewlineWarning(diff) {
+      const hasLeft = this._hasTrailingNewlines(diff, true);
+      const hasRight = this._hasTrailingNewlines(diff, false);
+      const messages = [];
+      if (hasLeft === false) {
+        messages.push(NO_NEWLINE_BASE);
+      }
+      if (hasRight === false) {
+        messages.push(NO_NEWLINE_REVISION);
+      }
+      if (!messages.length) { return null; }
+      return messages.join(' — ');
+    },
+
+    /**
+     * @param {string} warning
+     * @param {boolean} loading
+     * @return {string}
+     */
+    _computeNewlineWarningClass(warning, loading) {
+      if (loading || !warning) { return 'newlineWarning hidden'; }
+      return 'newlineWarning';
+    },
   });
 })();

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

@@ -1266,6 +1266,131 @@ limitations under the License.
         assert.isUndefined(reportStub.lastCall.args[1]);
       });
     });
+
+    suite('trailing newlines', () => {
+      setup(() => {
+        element = fixture('basic');
+      });
+
+      suite('_lastChunkForSide', () => {
+        test('deltas', () => {
+          const diff = {content: [
+            {a: ['foo', 'bar'], b: ['baz']},
+            {ab: ['foo', 'bar', 'baz']},
+            {b: ['foo']},
+          ]};
+          assert.equal(element._lastChunkForSide(diff, false), diff.content[2]);
+          assert.equal(element._lastChunkForSide(diff, true), diff.content[1]);
+
+          diff.content.push({a: ['foo'], b: ['bar']});
+          assert.equal(element._lastChunkForSide(diff, false), diff.content[3]);
+          assert.equal(element._lastChunkForSide(diff, true), diff.content[3]);
+        });
+
+        test('addition', () => {
+          const diff = {content: [
+            {b: ['foo', 'bar', 'baz']},
+          ]};
+          assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
+          assert.isNull(element._lastChunkForSide(diff, true));
+        });
+
+        test('deletion', () => {
+          const diff = {content: [
+            {a: ['foo', 'bar', 'baz']},
+          ]};
+          assert.isNull(element._lastChunkForSide(diff, false));
+          assert.equal(element._lastChunkForSide(diff, true), diff.content[0]);
+        });
+
+        test('empty', () => {
+          const diff = {content: []};
+          assert.isNull(element._lastChunkForSide(diff, false));
+          assert.isNull(element._lastChunkForSide(diff, true));
+        });
+      });
+
+      suite('_hasTrailingNewlines', () => {
+        test('shared no trailing', () => {
+          const diff = undefined;
+          sandbox.stub(element, '_lastChunkForSide')
+              .returns({ab: ['foo', 'bar']});
+          assert.isFalse(element._hasTrailingNewlines(diff, false));
+          assert.isFalse(element._hasTrailingNewlines(diff, true));
+        });
+
+        test('delta trailing in right', () => {
+          const diff = undefined;
+          sandbox.stub(element, '_lastChunkForSide')
+              .returns({a: ['foo', 'bar'], b: ['baz', '']});
+          assert.isTrue(element._hasTrailingNewlines(diff, false));
+          assert.isFalse(element._hasTrailingNewlines(diff, true));
+        });
+
+        test('addition', () => {
+          const diff = undefined;
+          sandbox.stub(element, '_lastChunkForSide', (diff, leftSide) => {
+            if (leftSide) { return null; }
+            return {b: ['foo', '']};
+          });
+          assert.isTrue(element._hasTrailingNewlines(diff, false));
+          assert.isNull(element._hasTrailingNewlines(diff, true));
+        });
+
+        test('deletion', () => {
+          const diff = undefined;
+          sandbox.stub(element, '_lastChunkForSide', (diff, leftSide) => {
+            if (!leftSide) { return null; }
+            return {a: ['foo']};
+          });
+          assert.isNull(element._hasTrailingNewlines(diff, false));
+          assert.isFalse(element._hasTrailingNewlines(diff, true));
+        });
+      });
+
+      test('_computeNewlineWarning', () => {
+        const NO_NEWLINE_BASE = 'No newline at end of base file.';
+        const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
+
+        let hasLeft;
+        let hasRight;
+        sandbox.stub(element, '_hasTrailingNewlines', (diff, left) => {
+          if (left) { return hasLeft; }
+          return hasRight;
+        });
+        const diff = undefined;
+
+        // The revision has a trailing newline, but the base doesn't.
+        hasLeft = true;
+        hasRight = false;
+        assert.equal(element._computeNewlineWarning(diff), NO_NEWLINE_REVISION);
+
+        // Trailing newlines were undetermined in the revision.
+        hasLeft = true;
+        hasRight = null;
+        assert.isNull(element._computeNewlineWarning(diff));
+
+        // Missing trailing newlines in the base.
+        hasLeft = false;
+        hasRight = null;
+        assert.equal(element._computeNewlineWarning(diff), NO_NEWLINE_BASE);
+
+        // Missing trailing newlines in both.
+        hasLeft = false;
+        hasRight = false;
+        assert.equal(element._computeNewlineWarning(diff),
+            NO_NEWLINE_BASE + ' — ' + NO_NEWLINE_REVISION);
+      });
+
+      test('_computeNewlineWarningClass', () => {
+        const hidden = 'newlineWarning hidden';
+        const shown = 'newlineWarning';
+        assert.equal(element._computeNewlineWarningClass(null, true), hidden);
+        assert.equal(element._computeNewlineWarningClass('foo', true), hidden);
+        assert.equal(element._computeNewlineWarningClass(null, false), hidden);
+        assert.equal(element._computeNewlineWarningClass('foo', false), shown);
+      });
+    });
   });
 
   a11ySuite('basic');