Make messages more conversational
Attempts to consume a change message to create a shorter and more
legible format. If the function encounters unexpected characters at any
point, it sets the _successfulParse flag to false and terminates,
causing the UI to fall back to displaying the entirety of the change
message.
A successful parse results in a one-liner that reads:
`${AVATAR} voted ${VOTES} and left ${NUM} comment(s) on ${PATCHSET}`
The ultimate goal/motivation for this change is to conserve vertical
space in the message list.
Change-Id: I857c17aa47a3ac1a0d17ad3dbddd563fecedbad3
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index 00157ab..5608dec 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -178,20 +178,34 @@
<gr-account-label
account="[[author]]"
hide-avatar></gr-account-label>
- <template is="dom-repeat" items="[[_getScores(message)]]" as="score">
- <span class$="score [[_computeScoreClass(score, labelExtremes)]]">
- [[score.label]] [[score.value]]
- </span>
+ <template is="dom-if" if="[[_successfulParse]]">
+ <template is="dom-if" if="[[_parsedVotes.length]]">voted</template>
+ <template is="dom-repeat" items="[[_parsedVotes]]" as="score">
+ <span class$="score [[_computeScoreClass(score, labelExtremes)]]">
+ [[score.label]] [[score.value]]
+ </span>
+ </template>
+ [[_computeConversationalString(_parsedVotes, _parsedPatchNum, _parsedCommentCount)]]
</template>
</div>
<template is="dom-if" if="[[message.message]]">
<div class="content">
- <div class="message hideOnOpen">[[message.message]]</div>
- <gr-formatted-text
- no-trailing-margin
- class="message hideOnCollapsed"
- content="[[message.message]]"
- config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
+ <template is="dom-if" if="[[_successfulParse]]">
+ <div class="message hideOnOpen">[[_parsedChangeMessage]]</div>
+ <gr-formatted-text
+ no-trailing-margin
+ class="message hideOnCollapsed"
+ content="[[_parsedChangeMessage]]"
+ config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
+ </template>
+ <template is="dom-if" if="[[!_successfulParse]]">
+ <div class="message hideOnOpen">[[message.message]]</div>
+ <gr-formatted-text
+ no-trailing-margin
+ class="message hideOnCollapsed"
+ content="[[message.message]]"
+ config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
+ </template>
<div class="replyContainer" hidden$="[[!showReplyButton]]" hidden>
<gr-button link small on-tap="_handleReplyTap">Reply</gr-button>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 0590c73..addd660 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -17,7 +17,8 @@
(function() {
'use strict';
- const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+: /;
+ const PATCH_SET_PREFIX_PATTERN = /^Patch Set (\d)+:[ ]?/;
+ const COMMENTS_COUNT_PATTERN = /^\((\d+)( inline)? comments?\)$/;
const LABEL_TITLE_SCORE_PATTERN = /^([A-Za-z0-9-]+)([+-]\d+)$/;
Polymer({
@@ -101,10 +102,17 @@
type: Boolean,
value: false,
},
+
+ _parsedPatchNum: String,
+ _parsedCommentCount: String,
+ _parsedVotes: Array,
+ _parsedChangeMessage: String,
+ _successfulParse: Boolean,
},
observers: [
'_updateExpandedClass(message.expanded)',
+ '_consumeMessage(message.message)',
],
ready() {
@@ -184,19 +192,6 @@
return event.type === 'REVIEWER_UPDATE';
},
- _getScores(message) {
- if (!message.message) { return []; }
- const line = message.message.split('\n', 1)[0];
- const patchSetPrefix = PATCH_SET_PREFIX_PATTERN;
- if (!line.match(patchSetPrefix)) { return []; }
- const scoresRaw = line.split(patchSetPrefix)[1];
- if (!scoresRaw) { return []; }
- return scoresRaw.split(' ')
- .map(s => s.match(LABEL_TITLE_SCORE_PATTERN))
- .filter(ms => ms && ms.length === 3)
- .map(ms => ({label: ms[1], value: ms[2]}));
- },
-
_computeScoreClass(score, labelExtremes) {
const classes = [];
if (score.value > 0) {
@@ -260,5 +255,73 @@
e.stopPropagation();
this.set('message.expanded', !this.message.expanded);
},
+
+ /**
+ * Attempts to consume a change message to create a shorter and more legible
+ * format. If the function encounters unexpected characters at any point, it
+ * sets the _successfulParse flag to false and terminates, causing the UI to
+ * fall back to displaying the entirety of the change message.
+ *
+ * A successful parse results in a one-liner that reads:
+ * `${AVATAR} voted ${VOTES} and left ${NUM} comment(s) on ${PATCHSET}`
+ *
+ * @param {string} text
+ */
+ _consumeMessage(text) {
+ this._parsedPatchNum = '';
+ this._parsedCommentCount = '';
+ this._parsedChangeMessage = '';
+ this._parsedVotes = [];
+ if (!text) {
+ // No message body means nothing to parse.
+ this._successfulParse = false;
+ return;
+ }
+ const lines = text.split('\n');
+ const messageLines = lines.shift().split(PATCH_SET_PREFIX_PATTERN);
+ if (!messageLines[1]) {
+ // Message is in an unexpected format.
+ this._successfulParse = false;
+ return;
+ }
+ this._parsedPatchNum = messageLines[1];
+ if (messageLines[2]) {
+ // Content after the colon is always vote information. If it is in the
+ // most up to date schema, parse it. Otherwise, cancel the parsing
+ // completely.
+ let match;
+ for (const score of messageLines[2].split(' ')) {
+ match = score.match(LABEL_TITLE_SCORE_PATTERN);
+ if (!match || match.length !== 3) {
+ this._successfulParse = false;
+ return;
+ }
+ this._parsedVotes.push({label: match[1], value: match[2]});
+ }
+ }
+ // Remove empty line.
+ lines.shift();
+ if (lines.length) {
+ const commentMatch = lines[0].match(COMMENTS_COUNT_PATTERN);
+ if (commentMatch) {
+ this._parsedCommentCount = commentMatch[1];
+ // Remove comment line and the following empty line.
+ lines.splice(0, 2);
+ }
+ this._parsedChangeMessage = lines.join('\n');
+ }
+ this._successfulParse = true;
+ },
+
+ _computeConversationalString(votes, patchNum, commentCount) {
+ let clause = ' on Patch Set ' + patchNum;
+ if (commentCount) {
+ let commentStr = ' comment';
+ if (parseInt(commentCount, 10) > 1) { commentStr += 's'; }
+ clause = ' left ' + commentCount + commentStr + clause;
+ if (votes.length) { clause = ' and' + clause; }
+ }
+ return clause;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
index 870f366..6bb4618 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
@@ -169,7 +169,9 @@
author: {},
expanded: false,
message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Ready+1',
+ _revision_number: 1,
};
+ element.comments = {};
element.labelExtremes = {
'Verified': {max: 1, min: -1},
'Code-Review': {max: 2, min: -2},
@@ -199,5 +201,145 @@
const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
assert.equal(scoreChips.length, 0);
});
+
+ suite('_consumeMessage', () => {
+ const assertConsumeFailed = str => {
+ element._consumeMessage(str);
+ assert.isFalse(element._successfulParse);
+ };
+
+ test('no message body', () => {
+ assertConsumeFailed('');
+ });
+
+ test('known old schema', () => {
+ const str = 'Patch Set 1: Looks good to me, approved; Verified';
+ assertConsumeFailed(str);
+ });
+
+ test('known old schema 2', () => {
+ const str = [
+ 'Patch Set 2: Looks good to me',
+ '',
+ 'Patch set 2 compiles, and runs as expected.',
+ ].join('\n');
+ assertConsumeFailed(str);
+ });
+
+ test('known old schema 3', () => {
+ const str = 'Patch Set 2: (1 inline comment)';
+ assertConsumeFailed(str);
+ });
+
+ test('known old schema 4', () => {
+ const str = [
+ 'Patch Set 2: Looks good to me',
+ '',
+ '(1 inline comment)',
+ ].join('\n');
+ assertConsumeFailed(str);
+ });
+
+ test('just change message', () => {
+ const str = [
+ 'Patch Set 2:',
+ '',
+ 'I think you should reconsider this approach.',
+ 'It really makes no sense.',
+ ].join('\n');
+
+ element._consumeMessage(str);
+
+ assert.isTrue(element._successfulParse);
+ assert.equal(element._parsedPatchNum, '2');
+ assert.equal(element._parsedCommentCount, '');
+ assert.deepEqual(element._parsedVotes, []);
+ assert.equal(element._parsedChangeMessage, [
+ 'I think you should reconsider this approach.',
+ 'It really makes no sense.',
+ ].join('\n'));
+ });
+
+ test('just votes', () => {
+ element._consumeMessage('Patch Set 2: Code-Review-Label+1 Verified+1');
+
+ assert.isTrue(element._successfulParse);
+ assert.equal(element._parsedPatchNum, '2');
+ assert.equal(element._parsedCommentCount, '');
+ assert.deepEqual(element._parsedVotes, [
+ {label: 'Code-Review-Label', value: '+1'},
+ {label: 'Verified', value: '+1'},
+ ]);
+ assert.equal(element._parsedChangeMessage, '');
+ });
+
+ test('just comments', () => {
+ const str = [
+ 'Patch Set 2:',
+ '',
+ '(8 comments)',
+ ].join('\n');
+
+ element._consumeMessage(str);
+
+ assert.isTrue(element._successfulParse);
+ assert.equal(element._parsedPatchNum, '2');
+ assert.equal(element._parsedCommentCount, '8');
+ assert.deepEqual(element._parsedVotes, []);
+ assert.equal(element._parsedChangeMessage, '');
+ });
+
+ test('vote with comments and change message', () => {
+ const str = [
+ 'Patch Set 2: Code-Review-Label+1 Verified+1',
+ '',
+ '(1 comment)',
+ '',
+ 'LGTM',
+ '',
+ 'Nice work, just one nit.',
+ ].join('\n');
+
+ element._consumeMessage(str);
+
+ assert.isTrue(element._successfulParse);
+ assert.equal(element._parsedPatchNum, '2');
+ assert.equal(element._parsedCommentCount, '1');
+ assert.deepEqual(element._parsedVotes, [
+ {label: 'Code-Review-Label', value: '+1'},
+ {label: 'Verified', value: '+1'},
+ ]);
+ assert.equal(element._parsedChangeMessage, [
+ 'LGTM',
+ '',
+ 'Nice work, just one nit.',
+ ].join('\n'));
+ });
+
+ test('vote with change message', () => {
+ const str = [
+ 'Patch Set 2: Code-Review-Label+2 Verified+1',
+ '',
+ 'LGTM',
+ '',
+ 'Nice work.',
+ ].join('\n');
+
+ element._consumeMessage(str);
+
+ assert.isTrue(element._successfulParse);
+ assert.equal(element._parsedPatchNum, '2');
+ assert.equal(element._parsedCommentCount, '');
+ assert.deepEqual(element._parsedVotes, [
+ {label: 'Code-Review-Label', value: '+2'},
+ {label: 'Verified', value: '+1'},
+ ]);
+ assert.equal(element._parsedChangeMessage, [
+ 'LGTM',
+ '',
+ 'Nice work.',
+ ].join('\n'));
+ });
+ });
});
</script>
