Merge changes from topic "comment-context"
* changes:
Show the lines of code around a comment as context
Request comment context along with the comments from server
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 5d7125c..7ee68b7 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -178,6 +178,11 @@
theme?: string;
}
+export declare interface RenderPreferences {
+ hide_left_side?: boolean;
+ disable_context_control_buttons?: boolean;
+}
+
/**
* Whether whitespace changes should be ignored and if yes, which whitespace
* changes should be ignored
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index a0a8253..85bfed2 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -682,6 +682,7 @@
logged-in="[[_loggedIn]]"
only-show-robot-comments-with-human-reply=""
unresolved-only
+ show-comment-context
></gr-thread-list>
</template>
<template
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
index 6515af0..0409a42 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
@@ -259,6 +259,7 @@
change-num="[[changeNum]]"
logged-in="[[_loggedIn]]"
hide-toggle-buttons
+ show-comment-context
>
</gr-thread-list>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
index 22b2e43..28bccc6 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
@@ -77,6 +77,9 @@
@property({type: Array})
_sortedThreads: CommentThread[] = [];
+ @property({type: Boolean})
+ showCommentContext = false;
+
@property({
computed:
'_computeDisplayedThreads(_sortedThreads.*, unresolvedOnly, ' +
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
index 8a52555..1be2da1 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
@@ -25,7 +25,6 @@
gr-comment-thread {
display: block;
margin-bottom: var(--spacing-m);
- max-width: 80ch;
}
.header {
align-items: center;
@@ -172,6 +171,7 @@
<gr-comment-thread
show-file-path=""
show-ported-comment="[[thread.ported]]"
+ show-comment-context="[[showCommentContext]]"
change-num="[[changeNum]]"
comments="[[thread.comments]]"
diff-side="[[thread.diffSide]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 7946061..7c32bf7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -66,7 +66,10 @@
// @ts-ignore
import * as shadow from 'shadow-selection-polyfill/shadow.js';
-import {CreateCommentEventDetail as CreateCommentEventDetailApi} from '../../../api/diff';
+import {
+ CreateCommentEventDetail as CreateCommentEventDetailApi,
+ RenderPreferences,
+} from '../../../api/diff';
import {isSafari} from '../../../utils/dom-util';
const NO_NEWLINE_BASE = 'No newline at end of base file.';
@@ -158,6 +161,9 @@
@property({type: Object, observer: '_prefsObserver'})
prefs?: DiffPreferencesInfo;
+ @property({type: Object, observer: '_renderPrefsChanged'})
+ renderPrefs?: RenderPreferences;
+
@property({type: Boolean})
displayLine = false;
@@ -727,6 +733,16 @@
}
}
+ _renderPrefsChanged(renderPrefs?: RenderPreferences) {
+ if (!renderPrefs) return;
+ if (renderPrefs.hide_left_side) {
+ this.classList.add('no-left');
+ }
+ if (renderPrefs.disable_context_control_buttons) {
+ this.updateStyles({'--context-control-display': 'none'});
+ }
+ }
+
_diffChanged(newValue?: DiffInfo) {
this._setLoading(true);
this._cleanup();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index b0f48ce..4b477cd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -272,6 +272,7 @@
/* Context controls */
.contextControl {
+ display: var(--context-control-display, table-row-group);
background-color: transparent;
border: none;
--divider-height: var(--spacing-s);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 6ee82c9..1af2adf 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -17,6 +17,7 @@
import '../../../styles/shared-styles';
import '../gr-storage/gr-storage';
import '../gr-comment/gr-comment';
+import '../../diff/gr-diff/gr-diff';
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
@@ -24,6 +25,7 @@
import {htmlTemplate} from './gr-comment-thread_html';
import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {
+ computeDiffFromContext,
isDraft,
isRobot,
sortComments,
@@ -33,9 +35,14 @@
} from '../../../utils/comment-util';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {appContext} from '../../../services/app-context';
-import {CommentSide, Side, SpecialFilePath} from '../../../constants/constants';
+import {
+ CommentSide,
+ createDefaultDiffPrefs,
+ Side,
+ SpecialFilePath,
+} from '../../../constants/constants';
import {computeDisplayPath} from '../../../utils/path-list-util';
-import {customElement, observe, property} from '@polymer/decorators';
+import {computed, customElement, observe, property} from '@polymer/decorators';
import {
CommentRange,
ConfigInfo,
@@ -50,6 +57,9 @@
import {CustomKeyboardEvent} from '../../../types/events';
import {LineNumber, FILE} from '../../diff/gr-diff/gr-diff-line';
import {GrButton} from '../gr-button/gr-button';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
+import {RenderPreferences} from '../../../api/diff';
const UNRESOLVED_EXPAND_COUNT = 5;
const NEWLINE_PATTERN = /\n/g;
@@ -122,7 +132,7 @@
patchNum?: PatchSetNum;
@property({type: String})
- path?: string;
+ path: string | undefined;
@property({type: String, observer: '_projectNameChanged'})
projectName?: RepoName;
@@ -164,6 +174,15 @@
@property({type: Object})
_projectConfig?: ConfigInfo;
+ @property({type: Object})
+ _prefs: DiffPreferencesInfo = createDefaultDiffPrefs();
+
+ @property({type: Object})
+ _renderPrefs: RenderPreferences = {
+ hide_left_side: true,
+ disable_context_control_buttons: true,
+ };
+
@property({type: Boolean, reflectToAttribute: true})
isRobotComment = false;
@@ -176,6 +195,9 @@
@property({type: Boolean})
showPatchset = true;
+ @property({type: Boolean})
+ showCommentContext = false;
+
get keyBindings() {
return {
'e shift+e': '_handleEKey',
@@ -188,6 +210,10 @@
readonly storage = new GrStorage();
+ private isCommentContextExperimentEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.COMMENT_CONTEXT
+ );
+
readonly restApiService = appContext.restApiService;
/** @override */
@@ -204,9 +230,34 @@
this._getLoggedIn().then(loggedIn => {
this._showActions = loggedIn;
});
+ this.restApiService.getDiffPreferences().then(prefs => {
+ if (!prefs) return;
+ this._prefs = {
+ ...prefs,
+ show_file_comment_button: false,
+ // override explicitly so that diff doesn't take too much width
+ // compared to the context
+ line_wrapping: false,
+ };
+ });
this._setInitialExpandedState();
}
+ @computed('comments', 'path')
+ get _diff() {
+ if (this.comments === undefined || this.path === undefined) return;
+ if (!this.comments[0]?.context_lines?.length) return;
+ return computeDiffFromContext(this.comments[0].context_lines, this.path);
+ }
+
+ _shouldShowCommentContext(diff?: DiffInfo) {
+ return (
+ this.isCommentContextExperimentEnabled &&
+ this.showCommentContext &&
+ !!diff
+ );
+ }
+
addOrEditDraft(lineNum?: LineNumber, rangeParam?: CommentRange) {
const lastComment = this.comments[this.comments.length - 1] || {};
if (isDraft(lastComment)) {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
index 1a89a79..7d5ba11 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
@@ -23,6 +23,15 @@
font-size: var(--font-size-normal);
font-weight: var(--font-weight-normal);
line-height: var(--line-height-normal);
+ /* Explicitly set the background color of the diff to be white. We
+ * cannot use the diff content type ab because of the skip chunk preceding
+ * it, diff processor assumes the chunk of type skip/ab can be collapsed
+ * and hides our diff behind context control buttons.
+ * */
+ --dark-add-highlight-color: white;
+ --diff-container-styles: {
+ border: 1px solid var(--border-color);
+ }
}
gr-button {
margin-left: var(--spacing-m);
@@ -34,24 +43,29 @@
margin-left: auto;
padding: var(--spacing-s) var(--spacing-m);
}
- #container {
+ .comment-box {
+ width: 80ch;
+ max-width: 100%;
background-color: var(--comment-background-color);
color: var(--comment-text-color);
- display: var(--gr-comment-thread-display, block);
- margin: 0 var(--spacing-s) var(--spacing-s);
- white-space: normal;
box-shadow: var(--elevation-level-2);
border-radius: var(--border-radius);
+ }
+ #container {
+ display: var(--gr-comment-thread-display, flex);
+ align-items: flex-start;
+ margin: 0 var(--spacing-s) var(--spacing-s);
+ white-space: normal;
/** This is required for firefox to continue the inheritance */
-webkit-user-select: inherit;
-moz-user-select: inherit;
-ms-user-select: inherit;
user-select: inherit;
}
- #container.unresolved {
+ .comment-box.unresolved {
background-color: var(--unresolved-comment-background-color);
}
- #container.robotComment {
+ .comment-box.robotComment {
background-color: var(--robot-comment-background-color);
}
#commentInfoContainer {
@@ -75,7 +89,11 @@
.fileName {
padding: var(--spacing-m) var(--spacing-s) var(--spacing-m);
}
+ gr-diff {
+ margin-left: var(--spacing-l);
+ }
</style>
+
<template is="dom-if" if="[[showFilePath]]">
<template is="dom-if" if="[[showFileName]]">
<div class="fileName">
@@ -100,71 +118,81 @@
</template>
</div>
</template>
- <div
- id="container"
- class$="[[_computeHostClass(unresolved, isRobotComment)]]"
- >
- <template
- id="commentList"
- is="dom-repeat"
- items="[[_orderedComments]]"
- as="comment"
- >
- <gr-comment
- comment="{{comment}}"
- comments="{{comments}}"
- robot-button-disabled="[[_shouldDisableAction(_showActions, _lastComment)]]"
- change-num="[[changeNum]]"
- project-name="[[projectName]]"
- patch-num="[[patchNum]]"
- draft="[[_isDraft(comment)]]"
- show-actions="[[_showActions]]"
- show-patchset="[[showPatchset]]"
- show-ported-comment="[[_computeShowPortedComment(comment)]]"
- side="[[comment.side]]"
- project-config="[[_projectConfig]]"
- on-create-fix-comment="_handleCommentFix"
- on-comment-discard="_handleCommentDiscard"
- on-comment-save="_handleCommentSavedOrDiscarded"
- ></gr-comment>
- </template>
- <div
- id="commentInfoContainer"
- hidden$="[[_hideActions(_showActions, _lastComment)]]"
- >
- <span id="unresolvedLabel">[[_getUnresolvedLabel(unresolved)]]</span>
- <div id="actions">
- <gr-button
- id="replyBtn"
- link=""
- class="action reply"
- on-click="_handleCommentReply"
- >Reply</gr-button
- >
- <gr-button
- id="quoteBtn"
- link=""
- class="action quote"
- on-click="_handleCommentQuote"
- >Quote</gr-button
- >
- <template is="dom-if" if="[[unresolved]]">
+ <div id="container">
+ <div class$="[[_computeHostClass(unresolved, isRobotComment)]] comment-box">
+ <template
+ id="commentList"
+ is="dom-repeat"
+ items="[[_orderedComments]]"
+ as="comment"
+ >
+ <gr-comment
+ comment="{{comment}}"
+ comments="{{comments}}"
+ robot-button-disabled="[[_shouldDisableAction(_showActions, _lastComment)]]"
+ change-num="[[changeNum]]"
+ project-name="[[projectName]]"
+ patch-num="[[patchNum]]"
+ draft="[[_isDraft(comment)]]"
+ show-actions="[[_showActions]]"
+ show-patchset="[[showPatchset]]"
+ show-ported-comment="[[_computeShowPortedComment(comment)]]"
+ side="[[comment.side]]"
+ project-config="[[_projectConfig]]"
+ on-create-fix-comment="_handleCommentFix"
+ on-comment-discard="_handleCommentDiscard"
+ on-comment-save="_handleCommentSavedOrDiscarded"
+ ></gr-comment>
+ </template>
+ <div
+ id="commentInfoContainer"
+ hidden$="[[_hideActions(_showActions, _lastComment)]]"
+ >
+ <span id="unresolvedLabel">[[_getUnresolvedLabel(unresolved)]]</span>
+ <div id="actions">
<gr-button
- id="ackBtn"
+ id="replyBtn"
link=""
- class="action ack"
- on-click="_handleCommentAck"
- >Ack</gr-button
+ class="action reply"
+ on-click="_handleCommentReply"
+ >Reply</gr-button
>
<gr-button
- id="doneBtn"
+ id="quoteBtn"
link=""
- class="action done"
- on-click="_handleCommentDone"
- >Done</gr-button
+ class="action quote"
+ on-click="_handleCommentQuote"
+ >Quote</gr-button
>
- </template>
+ <template is="dom-if" if="[[unresolved]]">
+ <gr-button
+ id="ackBtn"
+ link=""
+ class="action ack"
+ on-click="_handleCommentAck"
+ >Ack</gr-button
+ >
+ <gr-button
+ id="doneBtn"
+ link=""
+ class="action done"
+ on-click="_handleCommentDone"
+ >Done</gr-button
+ >
+ </template>
+ </div>
</div>
</div>
+ <template is="dom-if" if="[[_shouldShowCommentContext(_diff)]]">
+ <gr-diff
+ id="diff"
+ change-num="[[changeNum]]"
+ diff="[[_diff]]"
+ path="[[path]]"
+ prefs="[[_prefs]]"
+ render-prefs="[[_renderPrefs]]"
+ >
+ </gr-diff>
+ </template>
</div>
`;
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index 6e17c75..20d2d1d 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -2232,11 +2232,14 @@
path?: string
) {
if (!basePatchNum && !patchNum && !path) {
- return this._getDiffComments(changeNum, '/comments');
+ return this._getDiffComments(changeNum, '/comments', {
+ 'enable-context': true,
+ });
}
return this._getDiffComments(
changeNum,
'/comments',
+ {'enable-context': true},
basePatchNum,
patchNum,
path
@@ -2267,6 +2270,7 @@
return this._getDiffComments(
changeNum,
'/robotcomments',
+ undefined,
basePatchNum,
patchNum,
path
@@ -2305,6 +2309,7 @@
return this._getDiffComments(
changeNum,
'/drafts',
+ undefined,
basePatchNum,
patchNum,
path
@@ -2337,7 +2342,8 @@
_getDiffComments(
changeNum: NumericChangeId,
- endpoint: '/comments' | '/drafts'
+ endpoint: '/comments' | '/drafts',
+ params?: FetchParams
): Promise<PathToCommentsInfoMap | undefined>;
_getDiffComments(
@@ -2348,6 +2354,7 @@
_getDiffComments(
changeNum: NumericChangeId,
endpoint: '/comments' | '/drafts',
+ params?: FetchParams,
basePatchNum?: PatchSetNum,
patchNum?: PatchSetNum,
path?: string
@@ -2356,6 +2363,7 @@
_getDiffComments(
changeNum: NumericChangeId,
endpoint: '/robotcomments',
+ params?: FetchParams,
basePatchNum?: PatchSetNum,
patchNum?: PatchSetNum,
path?: string
@@ -2364,6 +2372,7 @@
_getDiffComments(
changeNum: NumericChangeId,
endpoint: string,
+ params?: FetchParams,
basePatchNum?: PatchSetNum,
patchNum?: PatchSetNum,
path?: string
@@ -2388,6 +2397,7 @@
endpoint,
revision: patchNum,
reportEndpointAsIs: true,
+ params,
},
noAcceptHeader
) as Promise<
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
index 6db2bff..fbd675f 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
@@ -75,7 +75,8 @@
},
],
}));
- return element._getDiffComments('42', '', 'PARENT', 1, 'sieve.go').then(
+ return element._getDiffComments('42', '', undefined, 'PARENT', 1,
+ 'sieve.go').then(
obj => {
assert.equal(obj.baseComments.length, 1);
assert.deepEqual(obj.baseComments[0], {
@@ -239,7 +240,7 @@
});
}
});
- return element._getDiffComments('42', '', 1, 2, 'sieve.go').then(
+ return element._getDiffComments('42', '', undefined, 1, 2, 'sieve.go').then(
obj => {
assert.equal(obj.baseComments.length, 1);
assert.deepEqual(obj.baseComments[0], {
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index f96afaa..878b8f7 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -31,4 +31,5 @@
NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
PORTING_COMMENTS = 'UiFeature__porting_comments',
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
+ COMMENT_CONTEXT = 'UiFeature__comment_context',
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 8f5518a..aeacd9a 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1173,11 +1173,21 @@
unresolved?: boolean;
change_message_id?: string;
commit_id?: string;
+ context_lines?: ContextLine[];
}
export type PathToCommentsInfoMap = {[path: string]: CommentInfo[]};
/**
+ * The ContextLine entity contains the line number and line text of a single line of the source file content..
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#context-line
+ */
+export interface ContextLine {
+ line_number: number;
+ context_line: string;
+}
+
+/**
* The ProjectInfo entity contains information about a project
* https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#project-info
*/
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 0c4a3c6..61456a2 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -24,12 +24,14 @@
CommentRange,
PatchRange,
ParentPatchSetNum,
+ ContextLine,
} from '../types/common';
import {CommentSide, Side} from '../constants/constants';
import {parseDate} from './date-util';
import {LineNumber} from '../elements/diff/gr-diff/gr-diff-line';
import {CommentIdToCommentThreadMap} from '../elements/diff/gr-comment-api/gr-comment-api';
import {isMergeParent, getParentIndex} from './patch-set-util';
+import {DiffInfo} from '../types/diff';
export interface DraftCommentProps {
__draft?: boolean;
@@ -285,3 +287,32 @@
};
}
}
+
+export function computeDiffFromContext(context: ContextLine[], path: string) {
+ const diff: DiffInfo = {
+ meta_a: {
+ name: '',
+ content_type: '',
+ lines: 0,
+ web_links: [],
+ },
+ meta_b: {
+ name: path,
+ content_type: '',
+ lines: context.length + context?.[0].line_number,
+ web_links: [],
+ },
+ change_type: 'MODIFIED',
+ intraline_status: 'OK',
+ diff_header: [],
+ content: [
+ {
+ skip: context[0].line_number - 1,
+ },
+ {
+ b: context.map(line => line.context_line),
+ },
+ ],
+ };
+ return diff;
+}
