opendev/gerrit
Code Issues Proposed changes

Extract helpers for isOnParent and patchNum

Browse Source 
Splitting is a prerequisite for first threading comments, then grouping
them by line.

Change-Id: I6b58d8bac9ba21244152b4e3596732362233b832
This commit is contained in:
Ole Rehmsen
2018-09-24 14:54:27 +02:00
parent 45365b9bc2
commit f0b33aa489
1 changed files with 46 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -398,6 +398,46 @@
return threads;
};
/**
* Returns the patch number that new comment threads should be attached to.
*
* @param {GrDiffLine} line The line new thread will be attached to.
* @param {string=} opt_side Set to LEFT to force adding it to the LEFT side -
* will be ignored if the left is a parent or a merge parent
* @return {number} Patch set to attach the new thread to
*/
GrDiffBuilder.prototype._determinePatchNumForNewThreads = function(
patchRange, line, opt_side) {
if ((line.type === GrDiffLine.Type.REMOVE ||
opt_side === GrDiffBuilder.Side.LEFT) &&
patchRange.basePatchNum !== 'PARENT' &&
!Gerrit.PatchSetBehavior.isMergeParent(patchRange.basePatchNum)) {
return patchRange.basePatchNum;
} else {
return patchRange.patchNum;
}
};
/**
* Returns whether the comments on the given line are on a (merge) parent.
*
* @param {string} firstCommentSide
* @param {{basePatchNum: number, patchNum: number}} patchRange
* @param {GrDiffLine} line The line the comments are on.
* @param {string=} opt_side
* @return {boolean} True iff the comments on the given line are on a (merge)
* parent.
*/
GrDiffBuilder.prototype._determineIsOnParent = function(
firstCommentSide, patchRange, line, opt_side) {
return ((line.type === GrDiffLine.Type.REMOVE ||
opt_side === GrDiffBuilder.Side.LEFT) &&
(patchRange.basePatchNum === 'PARENT' ||
Gerrit.PatchSetBehavior.isMergeParent(
patchRange.basePatchNum))) ||
firstCommentSide === 'PARENT';
};
/**
* @param {GrDiffLine} line
* @param {string=} opt_side
@@ -406,23 +446,16 @@
GrDiffBuilder.prototype._commentThreadGroupForLine = function(
line, opt_side) {
const comments =
this._getCommentsForLine(this._comments, line, opt_side);
this._getCommentsForLine(this._comments, line, opt_side);
if (!comments || comments.length === 0) {
return null;
}
let patchNum = this._comments.meta.patchRange.patchNum;
let isOnParent = comments[0].side === 'PARENT' || false;
if (line.type === GrDiffLine.Type.REMOVE ||
opt_side === GrDiffBuilder.Side.LEFT) {
if (this._comments.meta.patchRange.basePatchNum === 'PARENT' ||
Gerrit.PatchSetBehavior.isMergeParent(
this._comments.meta.patchRange.basePatchNum)) {
isOnParent = true;
} else {
patchNum = this._comments.meta.patchRange.basePatchNum;
}
}
const patchNum = this._determinePatchNumForNewThreads(
this._comments.meta.patchRange, line, opt_side);
const isOnParent = this._determineIsOnParent(
comments[0].side, this._comments.meta.patchRange, line, opt_side);
const threadGroupEl = this._createThreadGroupFn(patchNum, isOnParent,
opt_side);
threadGroupEl.threads = this._getThreads(comments, patchNum);