Support “diff against” in the file list
Bug: Issue 3936
Change-Id: I07f242a8cb7be063a2d56dbe3f4b6a28143588c0
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 8c6c216..1f5d9e1 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -273,7 +273,7 @@
</section>
<gr-file-list id="fileList"
change-num="[[_changeNum]]"
- patch-num="{{_patchRange.patchNum}}"
+ patch-range="[[_patchRange]]"
comments="[[_comments]]"
drafts="[[_diffDrafts]]"
revisions="[[_change.revisions]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index a8012a0..a3d2146 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -179,7 +179,14 @@
basePatchNum: value.basePatchNum || 'PARENT',
};
- this._resetStateIfNecessary();
+ // If the change number or patch range is different, then reset the
+ // selected file index.
+ var patchRangeState = this.viewState.patchRange;
+ if (this.viewState.changeNum !== this._changeNum ||
+ patchRangeState.basePatchNum !== this._patchRange.basePatchNum ||
+ patchRangeState.patchNum !== this._patchRange.patchNum) {
+ this._resetFileListViewState();
+ }
if (!this._changeNum) {
return;
@@ -212,17 +219,10 @@
}.bind(this));
},
- _resetStateIfNecessary: function() {
- // If the change number or patch range is different, then reset the
- // selected file index.
- if (this.viewState.changeNum !== this._changeNum ||
- this.viewState.patchRange.basePatchNum !==
- this._patchRange.basePatchNum ||
- this.viewState.patchRange.patchNum !== this._patchRange.patchNum) {
- this.set('viewState.selectedFileIndex', 0);
- this.set('viewState.changeNum', this._changeNum);
- this.set('viewState.patchRange', this._patchRange);
- }
+ _resetFileListViewState: function() {
+ this.set('viewState.selectedFileIndex', 0);
+ this.set('viewState.changeNum', this._changeNum);
+ this.set('viewState.patchRange', this._patchRange);
},
_changeChanged: function(change) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 104b27f2..7ce687c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -34,7 +34,7 @@
justify-content: space-between;
margin-bottom: .5em;
}
- header label {
+ .diffAgainst {
font-weight: normal;
}
.positionIndicator,
@@ -125,12 +125,22 @@
</style>
<header>
<div>Files</div>
- <label hidden>
- Diff against
- <select></select>
- </label>
+ <div class="diffAgainst">
+ <label>
+ Diff against
+ <select on-change="_handlePatchChange">
+ <option value="PARENT">Base</option>
+ <template is="dom-repeat" items="[[_computePatchSets(revisions, patchRange.*)]]" as="patchNum">
+ <option
+ value$="[[patchNum]]"
+ selected$="[[_computePatchSetSelected(patchNum, patchRange.basePatchNum)]]"
+ disabled$="[[_computePatchSetDisabled(patchNum, patchRange.patchNum)]]">[[patchNum]]</option>
+ </template>
+ </select>
+ </label>
+ </div>
</header>
- <template is="dom-repeat" items="{{_files}}" as="file">
+ <template is="dom-repeat" items="[[_files]]" as="file">
<div class="row" selected$="[[_computeFileSelected(index, selectedIndex)]]">
<div class="positionIndicator">▶</div>
<div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
@@ -140,7 +150,7 @@
<div class$="[[_computeClass('status', file.__path)]]">
[[_computeFileStatus(file.status)]]
</div>
- <a class="path" href$="[[_computeDiffURL(changeNum, patchNum, file.__path)]]">
+ <a class="path" href$="[[_computeDiffURL(changeNum, patchRange, file.__path)]]">
<div title$="[[_computeFileDisplayName(file.__path)]]">
[[_computeFileDisplayName(file.__path)]]
</div>
@@ -150,8 +160,8 @@
</div>
</a>
<div class="comments">
- <span class="drafts">[[_computeDraftsString(drafts, patchNum, file.__path)]]</span>
- [[_computeCommentsString(comments, patchNum, file.__path)]]
+ <span class="drafts">[[_computeDraftsString(drafts, patchRange.patchNum, file.__path)]]</span>
+ [[_computeCommentsString(comments, patchRange.patchNum, file.__path)]]
</div>
<div class$="[[_computeClass('stats', file.__path)]]">
<span class="added">+[[file.lines_inserted]]</span>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 47b6abd..a9ce44e 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -20,6 +20,7 @@
is: 'gr-file-list',
properties: {
+ patchRange: Object,
patchNum: String,
changeNum: String,
comments: Object,
@@ -50,7 +51,7 @@
],
reload: function() {
- if (!this.changeNum || !this.patchNum) {
+ if (!this.changeNum || !this.patchRange.patchNum) {
return Promise.resolve();
}
@@ -73,6 +74,28 @@
return Promise.all(promises);
},
+ _computePatchSets: function(revisions) {
+ var patchNums = [];
+ for (var commit in revisions) {
+ patchNums.push(revisions[commit]._number);
+ }
+ return patchNums.sort(function(a, b) { return a - b; });
+ },
+
+ _computePatchSetDisabled: function(patchNum, currentPatchNum) {
+ return parseInt(patchNum, 10) >= parseInt(currentPatchNum, 10);
+ },
+
+ _computePatchSetSelected: function(patchNum, basePatchNum) {
+ return parseInt(patchNum, 10) === parseInt(basePatchNum, 10);
+ },
+
+ _handlePatchChange: function(e) {
+ this.set('patchRange.basePatchNum', Polymer.dom(e).rootTarget.value);
+ page.show('/c/' + encodeURIComponent(this.changeNum) + '/' +
+ encodeURIComponent(this._patchRangeStr(this.patchRange)));
+ },
+
_computeCommentsString: function(comments, patchNum, path) {
return this._computeCountString(comments, patchNum, path, 'comment');
},
@@ -114,8 +137,8 @@
},
_saveReviewedState: function(path, reviewed) {
- return this.$.restAPI.saveFileReviewed(this.changeNum, this.patchNum,
- path, reviewed);
+ return this.$.restAPI.saveFileReviewed(this.changeNum,
+ this.patchRange.patchNum, path, reviewed);
},
_getLoggedIn: function() {
@@ -123,12 +146,13 @@
},
_getReviewedFiles: function() {
- return this.$.restAPI.getReviewedFiles(this.changeNum, this.patchNum);
+ return this.$.restAPI.getReviewedFiles(this.changeNum,
+ this.patchRange.patchNum);
},
_getFiles: function() {
return this.$.restAPI.getChangeFilesAsSpeciallySortedArray(
- this.changeNum, this.patchNum);
+ this.changeNum, this.patchRange);
},
_handleKey: function(e) {
@@ -164,7 +188,7 @@
if (opt_index != null) {
this.selectedIndex = opt_index;
}
- page.show(this._computeDiffURL(this.changeNum, this.patchNum,
+ page.show(this._computeDiffURL(this.changeNum, this.patchRange,
this._files[this.selectedIndex].__path));
},
@@ -176,8 +200,19 @@
return status || 'M';
},
- _computeDiffURL: function(changeNum, patchNum, path) {
- return '/c/' + changeNum + '/' + patchNum + '/' + path;
+ _computeDiffURL: function(changeNum, patchRange, path) {
+ return '/c/' +
+ encodeURIComponent(changeNum) +
+ '/' +
+ encodeURIComponent(this._patchRangeStr(patchRange)) +
+ '/' +
+ path;
+ },
+
+ _patchRangeStr: function(patchRange) {
+ return patchRange.basePatchNum !== 'PARENT' ?
+ patchRange.basePatchNum + '..' + patchRange.patchNum :
+ patchRange.patchNum + '';
},
_computeFileDisplayName: function(path) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 64ef91f..f80d211 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -89,7 +89,10 @@
{__path: 'myfile.txt'},
];
element.changeNum = '42';
- element.patchNum = '2';
+ element.patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: '2',
+ };
element.selectedIndex = 0;
flushAsynchronousOperations();
@@ -181,7 +184,10 @@
];
element._reviewed = ['/COMMIT_MSG', 'myfile.txt'];
element.changeNum = '42';
- element.patchNum = '2';
+ element.patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: '2',
+ };
element.selectedIndex = 0;
flushAsynchronousOperations();
@@ -205,5 +211,52 @@
saveStub.restore();
});
+
+ test('patch set from revisions', function() {
+ var patchNums = element._computePatchSets({
+ rev3: {_number: 3},
+ rev1: {_number: 1},
+ rev4: {_number: 4},
+ rev2: {_number: 2},
+ });
+ assert.deepEqual(patchNums, [1, 2, 3, 4]);
+ });
+
+ test('patch range string', function() {
+ assert.equal(
+ element._patchRangeStr({basePatchNum: 'PARENT', patchNum: '1'}),
+ '1');
+ assert.equal(
+ element._patchRangeStr({basePatchNum: '1', patchNum: '3'}),
+ '1..3');
+ });
+
+ test('diff against dropdown', function(done) {
+ var showStub = sinon.stub(page, 'show');
+ element.changeNum = '42';
+ element.patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: '3',
+ };
+ element.revisions = {
+ rev1: {_number: 1},
+ rev2: {_number: 2},
+ rev3: {_number: 3},
+ };
+ flush(function() {
+ var selectEl = element.$$('select');
+ assert.equal(selectEl.value, 'PARENT');
+ assert.isTrue(element.$$('option[value="3"]').hasAttribute('disabled'));
+ selectEl.addEventListener('change', function() {
+ assert.equal(selectEl.value, '2');
+ assert(showStub.lastCall.calledWithExactly('/c/42/2..3'),
+ 'Should navigate to /c/42/2..3');
+ showStub.restore();
+ done();
+ });
+ selectEl.value = '2';
+ element.fire('change', {}, {node: selectEl});
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index 41bf824..9145ba1 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -74,7 +74,6 @@
// Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>].
page(/^\/c\/(\d+)\/?(((\d+)(\.\.(\d+))?))?$/, function(ctx) {
// Parameter order is based on the regex group number matched.
- console.info(ctx)
var params = {
changeNum: ctx.params[0],
basePatchNum: ctx.params[3],
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 4480319..b916571 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -123,7 +123,7 @@
}
</style>
<h3>
- <a href$="[[_computeChangePath(_changeNum, _patchRange.patchNum, _change.revisions)]]">
+ <a href$="[[_computeChangePath(_changeNum, _patchRange.*, _change.revisions)]]">
[[_changeNum]]</a><span>:</span>
<span>[[_change.subject]]</span>
<span class="dash">—</span>
@@ -140,7 +140,7 @@
<iron-dropdown id="dropdown" vertical-align="top" vertical-offset="25">
<div class="dropdown-content">
<template is="dom-repeat" items="[[_fileList]]" as="path">
- <a href$="[[_computeDiffURL(_changeNum, _patchRange, path)]]"
+ <a href$="[[_computeDiffURL(_changeNum, _patchRange.*, path)]]"
selected$="[[_computeFileSelected(path, _path)]]"
data-key-nav$="[[_computeKeyNav(path, _path, _fileList)]]"
on-tap="_handleFileTap">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 6932d10..4b0ecdd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -83,7 +83,7 @@
observers: [
'_getChangeDetail(_changeNum)',
'_getProjectConfig(_change.project)',
- '_getFiles(_changeNum, _patchRange.patchNum)',
+ '_getFiles(_changeNum, _patchRange.*)',
'_updateModeSelect(_diffMode)',
],
@@ -127,9 +127,10 @@
}.bind(this));
},
- _getFiles: function(changeNum, patchNum) {
+ _getFiles: function(changeNum, patchRangeRecord) {
+ var patchRange = patchRangeRecord.base;
return this.$.restAPI.getChangeFilePathsAsSpeciallySortedArray(
- changeNum, patchNum).then(function(files) {
+ changeNum, patchRange).then(function(files) {
this._fileList = files;
}.bind(this));
},
@@ -196,9 +197,9 @@
case 85: // 'u'
if (this._changeNum && this._patchRange.patchNum) {
e.preventDefault();
- page.show(this._computeChangePath(
+ page.show(this._getChangePath(
this._changeNum,
- this._patchRange.patchNum,
+ this._patchRange,
this._change && this._change.revisions));
}
break;
@@ -221,15 +222,15 @@
var idx = fileList.indexOf(this._path) + direction;
if (idx < 0 || idx > fileList.length - 1) {
- page.show(this._computeChangePath(
+ page.show(this._getChangePath(
this._changeNum,
- this._patchRange.patchNum,
+ this._patchRange,
this._change && this._change.revisions));
return;
}
- page.show(this._computeDiffURL(this._changeNum,
- this._patchRange,
- fileList[idx]));
+ page.show(this._getDiffURL(this._changeNum,
+ this._patchRange,
+ fileList[idx]));
},
_paramsChanged: function(value) {
@@ -282,13 +283,22 @@
}
},
- _computeDiffURL: function(changeNum, patchRange, path) {
+ _getDiffURL: function(changeNum, patchRange, path) {
+ return '/c/' + changeNum + '/' + this._patchRangeStr(patchRange) + '/' +
+ path;
+ },
+
+ _computeDiffURL: function(changeNum, patchRangeRecord, path) {
+ return this._getDiffURL(changeNum, patchRangeRecord.base, path);
+ },
+
+ _patchRangeStr: function(patchRange) {
var patchStr = patchRange.patchNum;
if (patchRange.basePatchNum != null &&
patchRange.basePatchNum != 'PARENT') {
patchStr = patchRange.basePatchNum + '..' + patchRange.patchNum;
}
- return '/c/' + changeNum + '/' + patchStr + '/' + path;
+ return patchStr;
},
_computeAvailablePatches: function(revisions) {
@@ -299,25 +309,30 @@
return patchNums.sort(function(a, b) { return a - b; });
},
- _computeChangePath: function(changeNum, patchNum, revisions) {
+ _getChangePath: function(changeNum, patchRange, revisions) {
var base = '/c/' + changeNum + '/';
// The change may not have loaded yet, making revisions unavailable.
if (!revisions) {
- return base + patchNum;
+ return base + this._patchRangeStr(patchRange);
}
var latestPatchNum = -1;
for (var rev in revisions) {
latestPatchNum = Math.max(latestPatchNum, revisions[rev]._number);
}
- if (parseInt(patchNum, 10) != latestPatchNum) {
- return base + patchNum;
+ if (patchRange.basePatchNum !== 'PARENT' ||
+ parseInt(patchRange.patchNum, 10) !== latestPatchNum) {
+ return base + this._patchRangeStr(patchRange);
}
return base;
},
+ _computeChangePath: function(changeNum, patchRangeRecord, revisions) {
+ return this._getChangePath(changeNum, patchRangeRecord.base, revisions);
+ },
+
_computeFileDisplayName: function(path) {
return path == COMMIT_MESSAGE_PATH ? 'Commit message' : path;
},
@@ -347,8 +362,7 @@
_handleMobileSelectChange: function(e) {
var path = Polymer.dom(e).rootTarget.value;
- page.show(
- this._computeDiffURL(this._changeNum, this._patchRange, path));
+ page.show(this._getDiffURL(this._changeNum, this._patchRange, path));
},
_showDropdownTapHandler: function(e) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 4167424..e0d0735 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -50,6 +50,7 @@
test('keyboard shortcuts', function() {
element._changeNum = '42';
element._patchRange = {
+ basePatchNum: 'PARENT',
patchNum: '10',
};
element._change = {
@@ -143,12 +144,12 @@
MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a'
assert.isTrue(element.changeViewState.showReplyDialog);
- assert(showStub.lastCall.calledWithExactly('/c/42/'),
- 'Should navigate to /c/42/');
+ assert(showStub.lastCall.calledWithExactly('/c/42/5..10'),
+ 'Should navigate to /c/42/5..10');
MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u'
- assert(showStub.lastCall.calledWithExactly('/c/42/'),
- 'Should navigate to /c/42/');
+ assert(showStub.lastCall.calledWithExactly('/c/42/5..10'),
+ 'Should navigate to /c/42/5..10');
MockInteractions.pressAndReleaseKeyOn(element, 221); // ']'
assert(showStub.lastCall.calledWithExactly('/c/42/5..10/wheatley.md'),
@@ -166,8 +167,8 @@
element._path = 'chell.go';
MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
- assert(showStub.lastCall.calledWithExactly('/c/42/'),
- 'Should navigate to /c/42/');
+ assert(showStub.lastCall.calledWithExactly('/c/42/5..10'),
+ 'Should navigate to /c/42/5..10');
showStub.restore();
});
@@ -175,6 +176,7 @@
test('keyboard shortcuts with old patch number', function() {
element._changeNum = '42';
element._patchRange = {
+ basePatchNum: 'PARENT',
patchNum: '1',
};
element._change = {
@@ -230,6 +232,7 @@
test('go up to change via kb without change loaded', function() {
element._changeNum = '42';
element._patchRange = {
+ basePatchNum: 'PARENT',
patchNum: '1',
};
@@ -280,6 +283,7 @@
test('jump to file dropdown', function() {
element._changeNum = '42';
element._patchRange = {
+ basePatchNum: 'PARENT',
patchNum: '10',
};
element._fileList = ['chell.go', 'glados.txt', 'wheatley.md'];
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index a2fa039..3cb37b6 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -315,18 +315,22 @@
this.getChangeActionURL(changeNum, patchNum, '/commit?links'));
},
- getChangeFiles: function(changeNum, patchNum) {
+ getChangeFiles: function(changeNum, patchRange) {
+ var endpoint = '/files';
+ if (patchRange.basePatchNum !== 'PARENT') {
+ endpoint += '?base=' + encodeURIComponent(patchRange.basePatchNum);
+ }
return this.fetchJSON(
- this.getChangeActionURL(changeNum, patchNum, '/files'));
+ this.getChangeActionURL(changeNum, patchRange.patchNum, endpoint));
},
- getChangeFilesAsSpeciallySortedArray: function(changeNum, patchNum) {
- return this.getChangeFiles(changeNum, patchNum).then(
+ getChangeFilesAsSpeciallySortedArray: function(changeNum, patchRange) {
+ return this.getChangeFiles(changeNum, patchRange).then(
this._normalizeChangeFilesResponse.bind(this));
},
- getChangeFilePathsAsSpeciallySortedArray: function(changeNum, patchNum) {
- return this.getChangeFiles(changeNum, patchNum).then(function(files) {
+ getChangeFilePathsAsSpeciallySortedArray: function(changeNum, patchRange) {
+ return this.getChangeFiles(changeNum, patchRange).then(function(files) {
return Object.keys(files).sort(this._specialFilePathCompare.bind(this));
}.bind(this));
},
