Merge "Prefer using Splitter to String.split"
diff --git a/Documentation/config-plugins.txt b/Documentation/config-plugins.txt
index 16a3a93..185feb4 100644
--- a/Documentation/config-plugins.txt
+++ b/Documentation/config-plugins.txt
@@ -39,6 +39,14 @@
The core plugins are developed and maintained by the Gerrit maintainers
and the Gerrit community.
+[[codemirror-editor]]
+=== codemirror-editor
+
+CodeMirror plugin for polygerrit.
+
+link:https://gerrit-review.googlesource.com/#/admin/projects/plugins/codemirror-editor[
+Project] |
+
[[commit-message-length-validator]]
=== commit-message-length-validator
@@ -53,16 +61,6 @@
link:https://gerrit.googlesource.com/plugins/commit-message-length-validator/+doc/master/src/main/resources/Documentation/config.md[
Configuration]
-[[cookbook-plugin]]
-=== cookbook-plugin
-
-Sample plugin to demonstrate features of Gerrit's plugin API.
-
-link:https://gerrit-review.googlesource.com/#/admin/projects/plugins/cookbook-plugin[
-Project] |
-link:https://gerrit.googlesource.com/plugins/cookbook-plugin/+doc/master/src/main/resources/Documentation/about.md[
-Documentation]
-
[[download-commands]]
=== download-commands
@@ -347,6 +345,16 @@
link:https://gerrit.googlesource.com/plugins/its-jira/+doc/master/src/main/resources/Documentation/config.md[
Configuration]
+[[its-phabricator]]
+==== its-phabricator
+
+Plugin to integrate with Phabricator.
+
+link:https://gerrit-review.googlesource.com/#/admin/projects/plugins/its-phabricator[
+Project] |
+link:https://gerrit.googlesource.com/plugins/its-phabricator/+doc/master/src/main/resources/Documentation/config.md[
+Configuration]
+
[[its-rtc]]
==== its-rtc
diff --git a/Documentation/rest-api-access.txt b/Documentation/rest-api-access.txt
index 5a49f38..454de68 100644
--- a/Documentation/rest-api-access.txt
+++ b/Documentation/rest-api-access.txt
@@ -402,7 +402,7 @@
|`config_visible` |not set if `false`|
Whether the calling user can see the `refs/meta/config` branch of the
project.
-|`groups` |A map of group UUID to
+|`groups` ||A map of group UUID to
link:rest-api-groups.html#group-info[GroupInfo] objects, describing
the group UUIDs used in the `local` map. Groups that are not visible
are omitted from the `groups` map.
diff --git a/antlr3/BUILD b/antlr3/BUILD
index 08acf38..6d7102a 100644
--- a/antlr3/BUILD
+++ b/antlr3/BUILD
@@ -7,11 +7,11 @@
cmd = " && ".join([
"$(location //lib/antlr:antlr-tool) -o $$TMP $<",
"cd $$TMP",
- "zip -q $$ROOT/$@ $$(find . -type f )",
+ "find . -exec touch -t 198001010000 '{}' ';'",
+ "zip -q $$ROOT/$@ $$(find . -type f)",
]),
tools = [
"//lib/antlr:antlr-tool",
- "@bazel_tools//tools/zip:zipper",
],
visibility = ["//visibility:public"],
)
diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
index b21d3df..0a9f0ec 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
@@ -290,6 +290,7 @@
if (source.get(ChangeField.REVIEWER.getName()) != null) {
cd.setReviewers(
ChangeField.parseReviewerFieldValues(
+ cd.getId(),
FluentIterable.from(source.get(ChangeField.REVIEWER.getName()).getAsJsonArray())
.transform(JsonElement::getAsString)));
} else if (fields.contains(ChangeField.REVIEWER.getName())) {
@@ -300,6 +301,7 @@
if (source.get(ChangeField.REVIEWER_BY_EMAIL.getName()) != null) {
cd.setReviewersByEmail(
ChangeField.parseReviewerByEmailFieldValues(
+ cd.getId(),
FluentIterable.from(
source.get(ChangeField.REVIEWER_BY_EMAIL.getName()).getAsJsonArray())
.transform(JsonElement::getAsString)));
@@ -311,6 +313,7 @@
if (source.get(ChangeField.PENDING_REVIEWER.getName()) != null) {
cd.setPendingReviewers(
ChangeField.parseReviewerFieldValues(
+ cd.getId(),
FluentIterable.from(
source.get(ChangeField.PENDING_REVIEWER.getName()).getAsJsonArray())
.transform(JsonElement::getAsString)));
@@ -322,6 +325,7 @@
if (source.get(ChangeField.PENDING_REVIEWER_BY_EMAIL.getName()) != null) {
cd.setPendingReviewersByEmail(
ChangeField.parseReviewerByEmailFieldValues(
+ cd.getId(),
FluentIterable.from(
source.get(ChangeField.PENDING_REVIEWER_BY_EMAIL.getName()).getAsJsonArray())
.transform(JsonElement::getAsString)));
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index b30e66c..468aa67 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -586,12 +586,14 @@
private void decodeReviewers(ListMultimap<String, IndexableField> doc, ChangeData cd) {
cd.setReviewers(
ChangeField.parseReviewerFieldValues(
+ cd.getId(),
FluentIterable.from(doc.get(REVIEWER_FIELD)).transform(IndexableField::stringValue)));
}
private void decodeReviewersByEmail(ListMultimap<String, IndexableField> doc, ChangeData cd) {
cd.setReviewersByEmail(
ChangeField.parseReviewerByEmailFieldValues(
+ cd.getId(),
FluentIterable.from(doc.get(REVIEWER_BY_EMAIL_FIELD))
.transform(IndexableField::stringValue)));
}
@@ -599,6 +601,7 @@
private void decodePendingReviewers(ListMultimap<String, IndexableField> doc, ChangeData cd) {
cd.setPendingReviewers(
ChangeField.parseReviewerFieldValues(
+ cd.getId(),
FluentIterable.from(doc.get(PENDING_REVIEWER_FIELD))
.transform(IndexableField::stringValue)));
}
@@ -607,6 +610,7 @@
ListMultimap<String, IndexableField> doc, ChangeData cd) {
cd.setPendingReviewersByEmail(
ChangeField.parseReviewerByEmailFieldValues(
+ cd.getId(),
FluentIterable.from(doc.get(PENDING_REVIEWER_BY_EMAIL_FIELD))
.transform(IndexableField::stringValue)));
}
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index ee049c07..017bcaa 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -116,7 +116,7 @@
}
}
- if (nameOrEmail.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
+ if (ExternalId.isValidUsername(nameOrEmail)) {
Optional<AccountState> who = byId.getByUsername(nameOrEmail);
if (who.isPresent()) {
return ImmutableSet.of(who.map(a -> a.getAccount().getId()).get());
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalId.java b/java/com/google/gerrit/server/account/externalids/ExternalId.java
index 3b6ebd1..ffd413a 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -51,7 +51,7 @@
private static final String USER_NAME_PATTERN_LAST_REGEX = "[a-zA-Z0-9]";
/** Regular expression that a username must match. */
- public static final String USER_NAME_PATTERN_REGEX =
+ private static final String USER_NAME_PATTERN_REGEX =
"^"
+ //
"("
diff --git a/java/com/google/gerrit/server/args4j/AccountIdHandler.java b/java/com/google/gerrit/server/args4j/AccountIdHandler.java
index 01ebb1f..c7d3f73 100644
--- a/java/com/google/gerrit/server/args4j/AccountIdHandler.java
+++ b/java/com/google/gerrit/server/args4j/AccountIdHandler.java
@@ -91,7 +91,7 @@
}
private Account.Id createAccountByLdap(String user) throws CmdLineException, IOException {
- if (!user.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
+ if (!ExternalId.isValidUsername(user)) {
throw new CmdLineException(owner, "user \"" + user + "\" not found");
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 00aa5a4..c30edc1 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -256,39 +256,44 @@
return state.toString() + ',' + adr;
}
- public static ReviewerSet parseReviewerFieldValues(Iterable<String> values) {
+ public static ReviewerSet parseReviewerFieldValues(Change.Id changeId, Iterable<String> values) {
ImmutableTable.Builder<ReviewerStateInternal, Account.Id, Timestamp> b =
ImmutableTable.builder();
for (String v : values) {
int i = v.indexOf(',');
if (i < 0) {
- log.error("Invalid value for reviewer field: {}", v);
+ log.warn("Invalid value for reviewer field from change {}: {}", changeId.get(), v);
continue;
}
int i2 = v.lastIndexOf(',');
if (i2 == i) {
- log.error("Invalid value for reviewer field: {}", v);
+ log.warn("Invalid value for reviewer field from change {}: {}", changeId.get(), v);
continue;
}
com.google.common.base.Optional<ReviewerStateInternal> reviewerState =
Enums.getIfPresent(ReviewerStateInternal.class, v.substring(0, i));
if (!reviewerState.isPresent()) {
- log.error("Failed to parse reviewer state from reviewer field: {}", v);
+ log.warn(
+ "Failed to parse reviewer state of reviewer field from change {}: {}",
+ changeId.get(),
+ v);
continue;
}
Optional<Account.Id> accountId = Account.Id.tryParse(v.substring(i + 1, i2));
if (!accountId.isPresent()) {
- log.error("Failed to parse account ID from reviewer field: {}", v);
+ log.warn(
+ "Failed to parse account ID of reviewer field from change {}: {}", changeId.get(), v);
continue;
}
Long l = Longs.tryParse(v.substring(i2 + 1, v.length()));
if (l == null) {
- log.error("Failed to parse timestamp from reviewer field: {}", v);
+ log.warn(
+ "Failed to parse timestamp of reviewer field from change {}: {}", changeId.get(), v);
continue;
}
Timestamp timestamp = new Timestamp(l);
@@ -298,37 +303,47 @@
return ReviewerSet.fromTable(b.build());
}
- public static ReviewerByEmailSet parseReviewerByEmailFieldValues(Iterable<String> values) {
+ public static ReviewerByEmailSet parseReviewerByEmailFieldValues(
+ Change.Id changeId, Iterable<String> values) {
ImmutableTable.Builder<ReviewerStateInternal, Address, Timestamp> b = ImmutableTable.builder();
for (String v : values) {
int i = v.indexOf(',');
if (i < 0) {
- log.error("Invalid value for reviewer by email field: {}", v);
+ log.warn("Invalid value for reviewer by email field from change {}: {}", changeId.get(), v);
continue;
}
int i2 = v.lastIndexOf(',');
if (i2 == i) {
- log.error("Invalid value for reviewer by email field: {}", v);
+ log.warn("Invalid value for reviewer by email field from change {}: {}", changeId.get(), v);
continue;
}
com.google.common.base.Optional<ReviewerStateInternal> reviewerState =
Enums.getIfPresent(ReviewerStateInternal.class, v.substring(0, i));
if (!reviewerState.isPresent()) {
- log.error("Failed to parse reviewer state from reviewer by email field: {}", v);
+ log.warn(
+ "Failed to parse reviewer state of reviewer by email field from change {}: {}",
+ changeId.get(),
+ v);
continue;
}
Address address = Address.tryParse(v.substring(i + 1, i2));
if (address == null) {
- log.error("Failed to parse address from reviewer by email field: {}", v);
+ log.warn(
+ "Failed to parse address of reviewer by email field from change {}: {}",
+ changeId.get(),
+ v);
continue;
}
Long l = Longs.tryParse(v.substring(i2 + 1, v.length()));
if (l == null) {
- log.error("Failed to parse timestamp from reviewer by email field: {}", v);
+ log.warn(
+ "Failed to parse timestamp of reviewer by email field from change {}: {}",
+ changeId.get(),
+ v);
continue;
}
Timestamp timestamp = new Timestamp(l);
diff --git a/java/com/google/gerrit/server/restapi/account/CreateAccount.java b/java/com/google/gerrit/server/restapi/account/CreateAccount.java
index 29c331f..31bf93f 100644
--- a/java/com/google/gerrit/server/restapi/account/CreateAccount.java
+++ b/java/com/google/gerrit/server/restapi/account/CreateAccount.java
@@ -119,7 +119,7 @@
throw new BadRequestException("username must match URL");
}
- if (!username.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
+ if (!ExternalId.isValidUsername(username)) {
throw new BadRequestException(
"Username '" + username + "' must contain only letters, numbers, _, - or .");
}
diff --git a/java/com/google/gerrit/server/restapi/group/AddMembers.java b/java/com/google/gerrit/server/restapi/group/AddMembers.java
index 224a014..f65b29f 100644
--- a/java/com/google/gerrit/server/restapi/group/AddMembers.java
+++ b/java/com/google/gerrit/server/restapi/group/AddMembers.java
@@ -190,7 +190,7 @@
}
private Optional<Account> createAccountByLdap(String user) throws IOException {
- if (!user.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
+ if (!ExternalId.isValidUsername(user)) {
return Optional.empty();
}
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
index 5ecafd0..031284d 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
@@ -24,6 +24,7 @@
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.testing.GerritBaseTests;
@@ -60,7 +61,7 @@
.containsExactly(
"REVIEWER,1", "REVIEWER,1," + t1.getTime(), "CC,2", "CC,2," + t2.getTime());
- assertThat(ChangeField.parseReviewerFieldValues(values)).isEqualTo(reviewers);
+ assertThat(ChangeField.parseReviewerFieldValues(new Change.Id(1), values)).isEqualTo(reviewers);
}
@Test
diff --git a/javatests/com/google/gerrit/server/mail/receive/HtmlParserTest.java b/javatests/com/google/gerrit/server/mail/receive/HtmlParserTest.java
index b9548bd..d88e09f 100644
--- a/javatests/com/google/gerrit/server/mail/receive/HtmlParserTest.java
+++ b/javatests/com/google/gerrit/server/mail/receive/HtmlParserTest.java
@@ -83,6 +83,32 @@
}
@Test
+ public void simpleInlineCommentsWithLink() {
+ MailMessage.Builder b = newMailMessageBuilder();
+ b.htmlContent(
+ newHtmlBody(
+ "Looks good to me",
+ "How about [1]? This would help IMHO.</div><div>[1] "
+ + "<a href=\"http://gerritcodereview.com\">http://gerritcodereview.com</a>",
+ null,
+ "Also have a comment here.",
+ null,
+ null,
+ null));
+
+ List<Comment> comments = defaultComments();
+ List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, CHANGE_URL);
+
+ assertThat(parsedComments).hasSize(3);
+ assertChangeMessage("Looks good to me", parsedComments.get(0));
+ assertInlineComment(
+ "How about [1]? This would help IMHO.\n\n[1] http://gerritcodereview.com",
+ parsedComments.get(1),
+ comments.get(1));
+ assertInlineComment("Also have a comment here.", parsedComments.get(2), comments.get(4));
+ }
+
+ @Test
public void simpleFileComment() {
MailMessage.Builder b = newMailMessageBuilder();
b.htmlContent(
diff --git a/plugins/commit-message-length-validator b/plugins/commit-message-length-validator
index 3f96a82..315a115 160000
--- a/plugins/commit-message-length-validator
+++ b/plugins/commit-message-length-validator
@@ -1 +1 @@
-Subproject commit 3f96a82f15d9d592a86a989874b160567cd66f53
+Subproject commit 315a11558913fa8f9c6d3b1723e45583b25afa1c
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index 223730f..4672856 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit 223730f10cf16f6a8ed2c0a3867371ca336d9ae7
+Subproject commit 467285664ebf8eb6f1e03ff13ebc706eee6d8662
diff --git a/plugins/singleusergroup b/plugins/singleusergroup
index 2c42f31..9e65c40 160000
--- a/plugins/singleusergroup
+++ b/plugins/singleusergroup
@@ -1 +1 @@
-Subproject commit 2c42f316091726ba34a4f3809cb20197d8a46575
+Subproject commit 9e65c400d31b2f6e51c9d2a07dd4484011c5a713
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
index 2cfa0b2..8bceff1 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
@@ -35,19 +35,24 @@
:host {
display: inline-block;
}
- input {
- width: 25em;
+ input:not([type="checkbox"]),
+ gr-autocomplete,
+ iron-autogrow-textarea {
+ width: 100%;
+ }
+ .value {
+ width: 32em;
+ }
+ section {
+ align-items: center;
+ display: flex;
+ }
+ #description {
+ align-items: initial;
}
gr-autocomplete {
- border: none;
- float: right;
--gr-autocomplete: {
- border: 1px solid #d1d2d3;
- border-radius: 2px;
- font-size: var(--font-size-normal);
- height: 2em;
padding: 0 .15em;
- width: 20em;
}
}
</style>
@@ -65,40 +70,50 @@
</span>
</section>
<section>
- <span class="title">Enter topic for new change (optional)</span>
- <input
- is="iron-input"
- id="tagNameInput"
- maxlength="1024"
- bind-value="{{topic}}">
+ <span class="title">Enter topic for new change</span>
+ <span class="value">
+ <input
+ is="iron-input"
+ id="tagNameInput"
+ maxlength="1024"
+ placeholder="(optional)"
+ bind-value="{{topic}}">
+ </span>
</section>
- <section>
+ <section id="description">
<span class="title">Description</span>
- <iron-autogrow-textarea
- id="messageInput"
- class="message"
- autocomplete="on"
- rows="4"
- max-rows="15"
- bind-value="{{subject}}"
- placeholder="Insert the description of the change.">
- </iron-autogrow-textarea>
+ <span class="value">
+ <iron-autogrow-textarea
+ id="messageInput"
+ class="message"
+ autocomplete="on"
+ rows="4"
+ max-rows="15"
+ bind-value="{{subject}}"
+ placeholder="Insert the description of the change.">
+ </iron-autogrow-textarea>
+ </span>
</section>
<section>
- <span class="title">Options</span>
- <section>
- <label for="privateChangeCheckBox">Private Change</label>
+ <label
+ class="title"
+ for="privateChangeCheckBox">Private Change</label>
+ <span class="value">
<input
type="checkbox"
id="privateChangeCheckBox"
checked$="[[_repoConfig.private_by_default.inherited_value]]">
- </section>
- <section>
- <label for="wipChangeCheckBox">WIP Change</label>
+ </span>
+ </section>
+ <section>
+ <label
+ class="title"
+ for="wipChangeCheckBox">WIP Change</label>
+ <span class="value">
<input
type="checkbox"
id="wipChangeCheckBox">
- </section>
+ </span>
</section>
</div>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 6854f7a..a954559 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -52,7 +52,6 @@
ABANDON: 'abandon',
DELETE: '/',
DELETE_EDIT: 'deleteEdit',
- DONE_EDIT: 'doneEdit',
EDIT: 'edit',
IGNORE: 'ignore',
MOVE: 'move',
@@ -63,6 +62,7 @@
RESTORE: 'restore',
REVERT: 'revert',
REVIEWED: 'reviewed',
+ STOP_EDIT: 'stopEdit',
UNIGNORE: 'unignore',
UNREVIEWED: 'unreviewed',
WIP: 'wip',
@@ -159,11 +159,11 @@
__type: 'change',
};
- const DONE_EDIT = {
+ const STOP_EDIT = {
enabled: true,
- label: 'Done Editing',
+ label: 'Stop editing',
title: 'Stop editing this change',
- __key: 'doneEdit',
+ __key: 'stopEdit',
__primary: false,
__type: 'change',
};
@@ -569,14 +569,19 @@
delete this.actions.edit;
this.notifyPath('actions.edit');
}
- if (!changeActions.doneEdit) {
- this.set('actions.doneEdit', DONE_EDIT);
- }
} else {
if (!changeActions.edit) { this.set('actions.edit', EDIT); }
- if (changeActions.doneEdit) {
- delete this.actions.doneEdit;
- this.notifyPath('actions.doneEdit');
+ }
+ // Only show STOP_EDIT if edit mode is enabled, but no edit patch set
+ // is loaded.
+ if (editMode && !editPatchsetLoaded) {
+ if (!changeActions.stopEdit) {
+ this.set('actions.stopEdit', STOP_EDIT);
+ }
+ } else {
+ if (changeActions.stopEdit) {
+ delete this.actions.stopEdit;
+ this.notifyPath('actions.stopEdit');
}
}
}
@@ -831,8 +836,8 @@
case ChangeActions.EDIT:
this._handleEditTap();
break;
- case ChangeActions.DONE_EDIT:
- this._handleDoneEditTap();
+ case ChangeActions.STOP_EDIT:
+ this._handleStopEditTap();
break;
case ChangeActions.DELETE:
this._handleDeleteTap();
@@ -1316,8 +1321,8 @@
this.dispatchEvent(new CustomEvent('edit-tap', {bubbles: false}));
},
- _handleDoneEditTap() {
- this.dispatchEvent(new CustomEvent('done-edit-tap', {bubbles: false}));
+ _handleStopEditTap() {
+ this.dispatchEvent(new CustomEvent('stop-edit-tap', {bubbles: false}));
},
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index e76e494..a7a9636 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -437,7 +437,7 @@
assert.isOk(element.$$('gr-button[data-action-key="rebaseEdit"]'));
assert.isOk(element.$$('gr-button[data-action-key="deleteEdit"]'));
assert.isNotOk(element.$$('gr-button[data-action-key="edit"]'));
- assert.isOk(element.$$('gr-button[data-action-key="doneEdit"]'));
+ assert.isNotOk(element.$$('gr-button[data-action-key="stopEdit"]'));
});
test('edit patchset is loaded, does not need rebase', () => {
@@ -451,7 +451,7 @@
assert.isNotOk(element.$$('gr-button[data-action-key="rebaseEdit"]'));
assert.isOk(element.$$('gr-button[data-action-key="deleteEdit"]'));
assert.isNotOk(element.$$('gr-button[data-action-key="edit"]'));
- assert.isOk(element.$$('gr-button[data-action-key="doneEdit"]'));
+ assert.isNotOk(element.$$('gr-button[data-action-key="stopEdit"]'));
});
test('edit mode is loaded, no edit patchset', () => {
@@ -464,7 +464,7 @@
assert.isNotOk(element.$$('gr-button[data-action-key="rebaseEdit"]'));
assert.isNotOk(element.$$('gr-button[data-action-key="deleteEdit"]'));
assert.isNotOk(element.$$('gr-button[data-action-key="edit"]'));
- assert.isOk(element.$$('gr-button[data-action-key="doneEdit"]'));
+ assert.isOk(element.$$('gr-button[data-action-key="stopEdit"]'));
});
test('normal patch set', () => {
@@ -477,7 +477,7 @@
assert.isNotOk(element.$$('gr-button[data-action-key="rebaseEdit"]'));
assert.isNotOk(element.$$('gr-button[data-action-key="deleteEdit"]'));
assert.isOk(element.$$('gr-button[data-action-key="edit"]'));
- assert.isNotOk(element.$$('gr-button[data-action-key="doneEdit"]'));
+ assert.isNotOk(element.$$('gr-button[data-action-key="stopEdit"]'));
});
test('edit action', done => {
@@ -487,7 +487,7 @@
flushAsynchronousOperations();
assert.isNotOk(element.$$('gr-button[data-action-key="edit"]'));
- assert.isOk(element.$$('gr-button[data-action-key="doneEdit"]'));
+ assert.isOk(element.$$('gr-button[data-action-key="stopEdit"]'));
element.set('editMode', false);
flushAsynchronousOperations();
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 66e4331..268a6a2 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
@@ -367,7 +367,7 @@
edit-based-on-current-patch-set="[[hasEditBasedOnCurrentPatchSet(_allPatchSets)]]"
on-reload-change="_handleReloadChange"
on-edit-tap="_handleEditTap"
- on-done-edit-tap="_handleDoneEditTap"
+ on-stop-edit-tap="_handleStopEditTap"
on-download-tap="_handleOpenDownloadDialog"></gr-change-actions>
</div><!-- end commit actions -->
</div><!-- end header -->
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 5fdef79..2c49e6c 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
@@ -49,6 +49,11 @@
NEW_MESSAGE: 'There are new messages on this change',
};
+ const DiffViewMode = {
+ SIDE_BY_SIDE: 'SIDE_BY_SIDE',
+ UNIFIED: 'UNIFIED_DIFF',
+ };
+
Polymer({
is: 'gr-change-view',
@@ -251,6 +256,7 @@
'shift+r': '_handleCapitalRKey',
'a': '_handleAKey',
'd': '_handleDKey',
+ 'm': '_handleMKey',
's': '_handleSKey',
'u': '_handleUKey',
'x': '_handleXKey',
@@ -311,6 +317,18 @@
});
},
+ _handleMKey(e) {
+ if (this.shouldSuppressKeyboardShortcut(e) ||
+ this.modifierPressed(e)) { return; }
+
+ e.preventDefault();
+ if (this.viewState.diffMode === DiffViewMode.SIDE_BY_SIDE) {
+ this.set('viewState.diffMode', DiffViewMode.UNIFIED);
+ } else {
+ this.set('viewState.diffMode', DiffViewMode.SIDE_BY_SIDE);
+ }
+ },
+
_handleEditCommitMessage(e) {
this._editingCommitMessage = true;
this.$.commitMessageEditor.focusTextarea();
@@ -1451,15 +1469,8 @@
Gerrit.Nav.navigateToChange(this._change, patchNum, null, true);
},
- /**
- * Navigate to the latest non-edit patch set.
- */
- _handleDoneEditTap() {
- let patchNum = this._patchRange.patchNum;
- if (this.patchNumEquals(patchNum, this.EDIT_NAME)) {
- patchNum = this.computeLatestPatchNum(this._allPatchSets);
- }
- Gerrit.Nav.navigateToChange(this._change, patchNum);
+ _handleStopEditTap() {
+ Gerrit.Nav.navigateToChange(this._change, this._patchRange.patchNum);
},
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 347100e..b7ae295 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -257,6 +257,21 @@
MockInteractions.pressAndReleaseKeyOn(element, 188, null, ',');
assert.isTrue(stub.called);
});
+
+ test('m should toggle diff mode', () => {
+ sandbox.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
+ const e = {preventDefault: () => {}};
+ flushAsynchronousOperations();
+
+ // Initial state.
+ element.viewState.diffMode = 'SIDE_BY_SIDE';
+
+ element._handleMKey(e);
+ assert.equal(element.viewState.diffMode, 'UNIFIED_DIFF');
+
+ element._handleMKey(e);
+ assert.equal(element.viewState.diffMode, 'SIDE_BY_SIDE');
+ });
});
suite('reloading drafts', () => {
@@ -1535,42 +1550,20 @@
});
});
- suite('_handleDoneEditTap', () => {
- let fireDone;
-
- setup(() => {
- fireDone = () => {
- element.$.actions.dispatchEvent(new CustomEvent('done-edit-tap',
- {bubbles: false}));
- };
- sandbox.stub(element.$.metadata, '_computeShowLabelStatus');
- sandbox.stub(element.$.metadata, '_computeLabelNames');
- navigateToChangeStub.restore();
+ test('_handleStopEditTap', done => {
+ sandbox.stub(element.$.metadata, '_computeShowLabelStatus');
+ sandbox.stub(element.$.metadata, '_computeLabelNames');
+ navigateToChangeStub.restore();
+ sandbox.stub(element, 'computeLatestPatchNum').returns(1);
+ sandbox.stub(Gerrit.Nav, 'navigateToChange', (...args) => {
+ assert.equal(args.length, 2);
+ assert.equal(args[1], 1); // patchNum
+ done();
});
- test('edit patchset loaded', done => {
- sandbox.stub(element, 'computeLatestPatchNum').returns(1);
- sandbox.stub(Gerrit.Nav, 'navigateToChange', (...args) => {
- assert.equal(args.length, 2);
- assert.equal(args[1], 1); // patchNum
- done();
- });
-
- element._patchRange = {patchNum: element.EDIT_NAME};
- fireDone();
- });
-
- test('edit patchset loaded', done => {
- sandbox.stub(element, 'computeLatestPatchNum').returns(1);
- sandbox.stub(Gerrit.Nav, 'navigateToChange', (...args) => {
- assert.equal(args.length, 2);
- assert.equal(args[1], 1); // patchNum
- done();
- });
-
- element._patchRange = {patchNum: 1};
- fireDone();
- });
+ element._patchRange = {patchNum: 1};
+ element.$.actions.dispatchEvent(new CustomEvent('stop-edit-tap',
+ {bubbles: false}));
});
suite('plugin endpoints', () => {
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
index c30f726..ddec441 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
@@ -379,6 +379,12 @@
</tr>
<tr>
<td>
+ <span class="key">m</span>
+ </td>
+ <td>Toggle unified/side-by-side diff</td>
+ </tr>
+ <tr>
+ <td>
<span class="key">c</span>
</td>
<td>Draft new comment</td>
@@ -453,6 +459,12 @@
</tr>
<tr>
<td>
+ <span class="key">m</span>
+ </td>
+ <td>Toggle unified/side-by-side diff</td>
+ </tr>
+ <tr>
+ <td>
<span class="key">c</span>
</td>
<td>Draft new comment</td>
@@ -486,12 +498,6 @@
<td><span class="key">,</span></td>
<td>Show diff preferences</td>
</tr>
- <tr>
- <td>
- <span class="key">v</span>
- </td>
- <td>Toggle Unified/Side-by-side diff</td>
- </tr>
</tbody>
</table>
</main>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
index ab2077d..9b830cf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
@@ -76,7 +76,6 @@
margin-left: 1em;
}
</style>
-
<gr-overlay id="prefsOverlay" with-backdrop>
<div class="header">
Diff View Preferences
@@ -141,6 +140,14 @@
<input is="iron-input" type="checkbox" id="syntaxHighlightInput"
on-tap="_handleSyntaxHighlightTap">
</div>
+ <div class="pref">
+ <label for="automaticReviewInput">Automatically mark viewed files reviewed</label>
+ <input
+ is="iron-input"
+ id="automaticReviewInput"
+ type="checkbox"
+ on-tap="_handleAutomaticReviewTap">
+ </div>
</div>
<div class="actions">
<gr-button id="cancelButton" link on-tap="_handleCancel">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.js b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.js
index 185b047..1dcfc68 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.js
@@ -63,6 +63,7 @@
this.$.showTrailingWhitespaceInput.checked = prefs.show_whitespace_errors;
this.$.lineWrappingInput.checked = prefs.line_wrapping;
this.$.syntaxHighlightInput.checked = prefs.syntax_highlighting;
+ this.$.automaticReviewInput.checked = !prefs.manual_review;
},
_localPrefsChanged(changeRecord) {
@@ -93,6 +94,10 @@
this.set('_newPrefs.line_wrapping', Polymer.dom(e).rootTarget.checked);
},
+ _handleAutomaticReviewTap(e) {
+ this.set('_newPrefs.manual_review', !Polymer.dom(e).rootTarget.checked);
+ },
+
_handleSave(e) {
e.stopPropagation();
this.prefs = this._newPrefs;
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 da95c3d..f1cc83e 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
@@ -25,6 +25,11 @@
RIGHT: 'right',
};
+ const DiffViewMode = {
+ SIDE_BY_SIDE: 'SIDE_BY_SIDE',
+ UNIFIED: 'UNIFIED_DIFF',
+ };
+
Polymer({
is: 'gr-diff-view',
@@ -186,7 +191,7 @@
'a shift+a': '_handleAKey',
'u': '_handleUKey',
',': '_handleCommaKey',
- 'v': '_handleVKey',
+ 'm': '_handleMKey',
},
attached() {
@@ -426,15 +431,15 @@
this.$.diffPreferences.open();
},
- _handleVKey(e) {
+ _handleMKey(e) {
if (this.shouldSuppressKeyboardShortcut(e) ||
this.modifierPressed(e)) { return; }
e.preventDefault();
- if (this.changeViewState.diffMode=='SIDE_BY_SIDE') {
- this.set('changeViewState.diffMode', 'UNIFIED_DIFF');
+ if (this._getDiffViewMode() === DiffViewMode.SIDE_BY_SIDE) {
+ this.set('changeViewState.diffMode', DiffViewMode.UNIFIED);
} else {
- this.set('changeViewState.diffMode', 'SIDE_BY_SIDE');
+ this.set('changeViewState.diffMode', DiffViewMode.SIDE_BY_SIDE);
}
},
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 8b6d82f..ae98fe9 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
@@ -798,6 +798,19 @@
assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
});
+ test('_handleMKey', () => {
+ sandbox.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
+ const e = {preventDefault: () => {}};
+ // Initial state.
+ assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+
+ element._handleMKey(e);
+ assert.equal(element._getDiffViewMode(), 'UNIFIED_DIFF');
+
+ element._handleMKey(e);
+ assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+ });
+
suite('_loadComments', () => {
test('empty', done => {
element._loadComments().then(() => {
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
index 866a62b..4fedf73 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
@@ -141,11 +141,17 @@
_getFileData(changeNum, path, patchNum) {
return this.$.restAPI.getFileContent(changeNum, path, patchNum)
.then(res => {
- if (!res.ok) { return; }
-
- this._type = res.type || '';
this._newContent = res.content || '';
this._content = res.content || '';
+
+ // A non-ok response may result if the file does not yet exist.
+ // The `type` field of the response is only valid when the file
+ // already exists.
+ if (res.ok && res.type) {
+ this._type = res.type;
+ } else {
+ this._type = '';
+ }
});
},
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html
index 18ff1f5..ea87f56 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html
@@ -242,9 +242,9 @@
// Ensure no data is set with a bad response.
return element._getFileData('1', 'test/path', 'edit').then(() => {
- assert.equal(element._newContent, 'initial');
- assert.equal(element._content, 'initial');
- assert.equal(element._type, 'initial');
+ assert.equal(element._newContent, '');
+ assert.equal(element._content, '');
+ assert.equal(element._type, '');
});
});
diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html
index 68b9b59..46f4649 100644
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -78,13 +78,14 @@
footer {
color: var(--primary-text-color);
}
- gr-main-header.shadow {
- /* Make it obvious for shadow dom testing */
- border-bottom: 1px solid pink;
- }
gr-main-header {
background-color: var(--header-background-color);
padding: 0 var(--default-horizontal-margin);
+ border-bottom: 1px solid #ddd;
+ }
+ gr-main-header.shadow {
+ /* Make it obvious for shadow dom testing */
+ border-bottom: 1px solid pink;
}
footer {
background-color: var(--footer-background-color);
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 cbb2287..6749437 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
@@ -1418,9 +1418,15 @@
* @param {number|string} patchNum
*/
getFileContent(changeNum, path, patchNum) {
+ // 404s indicate the file does not exist yet in the revision, so suppress
+ // them.
+ const suppress404s = res => {
+ if (res && res.status !== 404) { this.fire('server-error', {res}); }
+ return res;
+ };
const promise = this.patchNumEquals(patchNum, this.EDIT_NAME) ?
this._getFileInChangeEdit(changeNum, path) :
- this._getFileInRevision(changeNum, path, patchNum);
+ this._getFileInRevision(changeNum, path, patchNum, suppress404s);
return promise.then(res => {
if (!res.ok) { return res; }
@@ -1439,12 +1445,13 @@
* @param {number|string} changeNum
* @param {string} path
* @param {number|string} patchNum
+ * @param {?function(?Response, string=)=} opt_errFn
*/
- _getFileInRevision(changeNum, path, patchNum) {
+ _getFileInRevision(changeNum, path, patchNum, opt_errFn) {
const e = `/files/${encodeURIComponent(path)}/content`;
const headers = {Accept: 'application/json'};
- return this.getChangeURLAndSend(changeNum, 'GET', patchNum, e, null, null,
- null, null, headers);
+ return this.getChangeURLAndSend(changeNum, 'GET', patchNum, e, null,
+ opt_errFn, null, null, headers);
},
/**
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index efe70a9..352f759 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -1277,6 +1277,23 @@
return Promise.all([edit, normal]);
});
+ test('getFileContent suppresses 404s', done => {
+ const res = {status: 404};
+ const handler = e => {
+ assert.isFalse(e.detail.res.status === 404);
+ done();
+ };
+ element.addEventListener('server-error', handler);
+ sandbox.stub(Gerrit.Auth, 'fetch').returns(Promise.resolve(res));
+ sandbox.stub(element, '_changeBaseURL').returns(Promise.resolve(''));
+ element.getFileContent('1', 'tst/path', '1').then(() => {
+ flushAsynchronousOperations();
+
+ res.status = 500;
+ element.getFileContent('1', 'tst/path', '1');
+ });
+ });
+
test('getChangeFilesAsSpeciallySortedArray is edit-sensitive', () => {
const fn = element.getChangeFilesAsSpeciallySortedArray.bind(element);
const getChangeFilesStub = sandbox.stub(element, 'getChangeFiles')
