opendev/gerrit
Code Issues Proposed changes

Merge "Let GrDiffHost determine whether to show newline warnings"

Browse Source
This commit is contained in:
Hermann Loose
2020-02-21 13:48:17 +00:00
committed by Gerrit Code Review
parent b11cffe35c 234616a862
commit 71632f535b
5 changed files with 230 additions and 191 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
View File

@@ -51,7 +51,9 @@ limitations under the License.
coverage-ranges="[[_coverageRanges]]"
blame="[[_blame]]"
layers="[[_layers]]"
diff="[[diff]]">
diff="[[diff]]"
show-newline-warning-left="[[_showNewlineWarningLeft(diff)]]"
show-newline-warning-right="[[_showNewlineWarningRight(diff)]]">
</gr-diff>
<gr-syntax-layer
id="syntaxLayer"

67
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
View File

@@ -1037,6 +1037,73 @@
'diff-context-expanded', {numLines: event.detail.numLines}
);
}
/**
* 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 || !chunk.a.length)) ||
(!leftSide && (!chunk.b || !chunk.b.length))));
// 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] === '';
}
_showNewlineWarningLeft(diff) {
return this._hasTrailingNewlines(diff, true) === false;
}
_showNewlineWarningRight(diff) {
return this._hasTrailingNewlines(diff, false) === false;
}
}
customElements.define(GrDiffHost.is, GrDiffHost);

97
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
View File

@@ -1530,5 +1530,102 @@ limitations under the License.
});
});
});
suite('trailing newlines', () => {
setup(() => {
});
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 with a undefined', () => {
const diff = {content: [
{b: ['foo', 'bar', 'baz']},
]};
assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
assert.isNull(element._lastChunkForSide(diff, true));
});
test('addition with a empty', () => {
const diff = {content: [
{a: [], b: ['foo', 'bar', 'baz']},
]};
assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
assert.isNull(element._lastChunkForSide(diff, true));
});
test('deletion with b undefined', () => {
const diff = {content: [
{a: ['foo', 'bar', 'baz']},
]};
assert.isNull(element._lastChunkForSide(diff, false));
assert.equal(element._lastChunkForSide(diff, true), diff.content[0]);
});
test('deletion with b empty', () => {
const diff = {content: [
{a: ['foo', 'bar', 'baz'], b: []},
]};
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));
});
});
});
});
</script>

82
polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
View File

@@ -249,9 +249,19 @@
parentIndex: Number,
showNewlineWarningLeft: {
type: Boolean,
value: false,
},
showNewlineWarningRight: {
type: Boolean,
value: false,
},
_newlineWarning: {
type: String,
computed: '_computeNewlineWarning(diff)',
computed: '_computeNewlineWarning(' +
'showNewlineWarningLeft, showNewlineWarningRight)',
},
_diffLength: Number,
@@ -916,76 +926,16 @@
}
/**
* 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 || !chunk.a.length)) ||
(!leftSide && (!chunk.b || !chunk.b.length))));
// 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
* @param {!boolean} warnLeft
* @param {!boolean} warnRight
* @return {string|null}
*/
_computeNewlineWarning(diff) {
const hasLeft = this._hasTrailingNewlines(diff, true);
const hasRight = this._hasTrailingNewlines(diff, false);
_computeNewlineWarning(warnLeft, warnRight) {
const messages = [];
if (hasLeft === false) {
if (warnLeft) {
messages.push(NO_NEWLINE_BASE);
}
if (hasRight === false) {
if (warnRight) {
messages.push(NO_NEWLINE_REVISION);
}
if (!messages.length) { return null; }

171
polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
View File

@@ -834,137 +834,60 @@ limitations under the License.
});
});
suite('trailing newlines', () => {
suite('trailing newline warnings', () => {
const NO_NEWLINE_BASE = 'No newline at end of base file.';
const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
const getWarning = element =>
element.shadowRoot.querySelector('.newlineWarning').textContent;
setup(() => {
element = fixture('basic');
element.showNewlineWarningLeft = false;
element.showNewlineWarningRight = false;
});
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 with a undefined', () => {
const diff = {content: [
{b: ['foo', 'bar', 'baz']},
]};
assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
assert.isNull(element._lastChunkForSide(diff, true));
});
test('addition with a empty', () => {
const diff = {content: [
{a: [], b: ['foo', 'bar', 'baz']},
]};
assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
assert.isNull(element._lastChunkForSide(diff, true));
});
test('deletion with b undefined', () => {
const diff = {content: [
{a: ['foo', 'bar', 'baz']},
]};
assert.isNull(element._lastChunkForSide(diff, false));
assert.equal(element._lastChunkForSide(diff, true), diff.content[0]);
});
test('deletion with b empty', () => {
const diff = {content: [
{a: ['foo', 'bar', 'baz'], b: []},
]};
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),
test('shows combined warning if both sides set to warn', () => {
element.showNewlineWarningLeft = true;
element.showNewlineWarningRight = true;
assert.include(getWarning(element),
NO_NEWLINE_BASE + ' — ' + NO_NEWLINE_REVISION);
});
suite('showNewlineWarningLeft', () => {
test('show warning if true', () => {
element.showNewlineWarningLeft = true;
assert.include(getWarning(element), NO_NEWLINE_BASE);
});
test('hide warning if false', () => {
element.showNewlineWarningLeft = false;
assert.notInclude(getWarning(element), NO_NEWLINE_BASE);
});
test('hide warning if undefined', () => {
element.showNewlineWarningLeft = undefined;
assert.notInclude(getWarning(element), NO_NEWLINE_BASE);
});
});
suite('showNewlineWarningRight', () => {
test('show warning if true', () => {
element.showNewlineWarningRight = true;
assert.include(getWarning(element), NO_NEWLINE_REVISION);
});
test('hide warning if false', () => {
element.showNewlineWarningRight = false;
assert.notInclude(getWarning(element), NO_NEWLINE_REVISION);
});
test('hide warning if undefined', () => {
element.showNewlineWarningRight = undefined;
assert.notInclude(getWarning(element), NO_NEWLINE_REVISION);
});
});
test('_computeNewlineWarningClass', () => {
const hidden = 'newlineWarning hidden';
const shown = 'newlineWarning';