Merge "Fix Shift-A shortcut for hiding the left side of the diff"
diff --git a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
index 30514a6..681d0bd 100644
--- a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
+++ b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
@@ -31,7 +31,9 @@
PULL,
CHECKOUT,
CHERRY_PICK,
- FORMAT_PATCH
+ FORMAT_PATCH,
+ BRANCH,
+ RESET,
}
public enum DateFormat {
diff --git a/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java b/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java
index d92da18..e3e96df 100644
--- a/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java
+++ b/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java
@@ -43,8 +43,8 @@
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException {
CacheHeaders.setNotCacheable(res);
- res.setContentLength(0);
if (user.get().isIdentifiedUser()) {
+ res.setContentLength(0);
res.setStatus(HttpServletResponse.SC_NO_CONTENT);
} else {
res.setStatus(HttpServletResponse.SC_FORBIDDEN);
diff --git a/java/com/google/gerrit/server/change/ReviewerAdder.java b/java/com/google/gerrit/server/change/ReviewerAdder.java
index 3d986d2..5d55b4d 100644
--- a/java/com/google/gerrit/server/change/ReviewerAdder.java
+++ b/java/com/google/gerrit/server/change/ReviewerAdder.java
@@ -19,7 +19,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.extensions.client.ReviewerState.CC;
-import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import static java.util.Comparator.comparing;
import static java.util.Objects.requireNonNull;
@@ -140,12 +139,12 @@
}
logger.atFine().log(
- "Adding account %d from author/committer identity of commit %s as reviewer to change %d",
+ "Adding account %d from author/committer identity of commit %s as cc to change %d",
accountId.get(), commitId.name(), change.getChangeId());
InternalAddReviewerInput in = new InternalAddReviewerInput();
in.reviewer = accountId.toString();
- in.state = REVIEWER;
+ in.state = CC;
in.notify = notify;
in.otherFailureBehavior = FailureBehavior.IGNORE;
return Optional.of(in);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 496a2b4..ccfa60e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1499,19 +1499,19 @@
assertThat(commit.author.email).isEqualTo(user.email());
assertThat(commit.committer.email).isEqualTo(user.email());
- // check that the author/committer was added as reviewer
- Collection<AccountInfo> reviewers = change.reviewers.get(REVIEWER);
+ // check that the author/committer was added as cc
+ Collection<AccountInfo> reviewers = change.reviewers.get(CC);
assertThat(reviewers).isNotNull();
assertThat(reviewers).hasSize(1);
assertThat(reviewers.iterator().next()._accountId).isEqualTo(user.id().get());
- assertThat(change.reviewers.get(CC)).isNull();
+ assertThat(change.reviewers.get(REVIEWER)).isNull();
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.from().name()).isEqualTo("Administrator (Code Review)");
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
- assertThat(m.body()).contains("I'd like you to do a code review");
+ assertThat(m.body()).contains("has uploaded this change for review");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertMailReplyTo(m, admin.email());
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 4350072..39366bd 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -931,6 +931,37 @@
}
@Test
+ public void intralineEditsAreIdentified() throws Exception {
+ // TODO(ghareeb): This test asserts the wrong behavior due to the following issue
+ // bugs.chromium.org/p/gerrit/issues/detail?id=13563
+ // Please remove this comment and assert the correct behavior when the bug is fixed.
+
+ assume().that(intraline).isTrue();
+
+ String orig = "[-9999,9999]";
+ String replace = "[-999,999]";
+
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat(orig));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.replace(orig, replace));
+
+ // TODO(ghareeb): remove this comment when the issue is fixed.
+ // The returned diff incorrectly contains:
+ // replace [-9999{,99}99] with [-999{,}999].
+ // If this replace edit is done, the resulting string incorrectly becomes [-9999,99].
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
+
+ List<List<Integer>> editsA = diffInfo.content.get(1).editA;
+ List<List<Integer>> editsB = diffInfo.content.get(1).editB;
+ String reconstructed = transformStringUsingEditList(orig, replace, editsA, editsB);
+
+ // TODO(ghareeb): assert equals when the issue is fixed.
+ assertThat(reconstructed).isNotEqualTo(replace);
+ }
+
+ @Test
public void intralineEditsForModifiedLastLineArePreservedWhenNewlineIsAlsoAddedAtEnd()
throws Exception {
assume().that(intraline).isTrue();
@@ -2896,4 +2927,29 @@
.diffRequest()
.withIntraline(intraline);
}
+
+ /**
+ * This method transforms the {@code orig} input String using the list of replace edits {@code
+ * editsA}, {@code editsB} and the resulting {@code replace} String. This method currently assumes
+ * that all input edits are replace edits, and that the edits are sorted according to their
+ * indices.
+ *
+ * @return The transformed String after applying the list of replace edits to the original String.
+ */
+ private String transformStringUsingEditList(
+ String orig, String replace, List<List<Integer>> editsA, List<List<Integer>> editsB) {
+ assertThat(editsA).hasSize(editsB.size());
+ StringBuilder process = new StringBuilder(orig);
+ // The edits are processed right to left to avoid recomputation of indices when characters
+ // are removed.
+ for (int i = editsA.size() - 1; i >= 0; i--) {
+ List<Integer> leftEdit = editsA.get(i);
+ List<Integer> rightEdit = editsB.get(i);
+ process.replace(
+ leftEdit.get(0),
+ leftEdit.get(0) + leftEdit.get(1),
+ replace.substring(rightEdit.get(0), rightEdit.get(0) + rightEdit.get(1)));
+ }
+ return process.toString();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 2f9530c..839b051 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -872,7 +872,7 @@
String changeId = project.get() + "~master~" + result.getChangeId();
// 'user' cherry-picks the change to a new branch, the source change's author/committer('admin')
- // will be added as a reviewer of the newly created change.
+ // will be added as cc of the newly created change.
requestScopeOperations.setApiUser(user.id());
CherryPickInput input = new CherryPickInput();
input.message = "it goes to a new branch";
@@ -882,7 +882,7 @@
input.notify = NotifyHandling.ALL;
sender.clear();
gApi.changes().id(changeId).current().cherryPick(input);
- assertNotifyTo(admin);
+ assertNotifyCc(admin);
// Disable the notification. 'admin' as a reviewer should not be notified any more.
input.destination = "branch-2";
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index c526e31..763e7b1 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -87,6 +87,7 @@
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -1123,7 +1124,7 @@
String changeId = GitUtil.getChangeId(testRepo, c).get();
assertThat(getOwnerEmail(changeId)).isEqualTo(admin.email());
- assertThat(getReviewerEmails(changeId, ReviewerState.REVIEWER))
+ assertThat(getReviewerEmails(changeId, ReviewerState.CC))
.containsExactly(user.email(), user2.email());
assertThat(sender.getMessages()).hasSize(1);
@@ -1148,7 +1149,7 @@
pushHead(testRepo, "refs/for/master");
assertThat(getOwnerEmail(r.getChangeId())).isEqualTo(admin.email());
- assertThat(getReviewerEmails(r.getChangeId(), ReviewerState.REVIEWER))
+ assertThat(getReviewerEmails(r.getChangeId(), ReviewerState.CC))
.containsExactly(user.email(), user2.email());
assertThat(sender.getMessages()).hasSize(1);
@@ -1180,27 +1181,20 @@
// Push this commit as "Administrator" (requires Forge Committer Identity)
pushHead(testRepo, "refs/for/master%l=Code-Review+1", false);
- // Expected Code-Review votes:
- // 1. 0 from User (committer):
- // When the committer is forged, the committer is automatically added as
- // reviewer, hence we expect a dummy 0 vote for the committer.
- // 2. +1 from Administrator (uploader):
- // On push Code-Review+1 was specified, hence we expect a +1 vote from
- // the uploader.
+ // Expected Code-Review vote:
+ // +1 from Administrator (uploader):
+ // On push Code-Review+1 was specified, hence we expect a +1 vote from the uploader. When the
+ // committer is forged, the committer is automatically added as cc, but that doesn't add votes
+ // (as opposted to being added as reviewer that adds a dummy +0 vote). We ensure there are no
+ // votes from the committer.
ChangeInfo ci =
get(GitUtil.getChangeId(testRepo, c).get(), DETAILED_LABELS, MESSAGES, DETAILED_ACCOUNTS);
LabelInfo cr = ci.labels.get("Code-Review");
- assertThat(cr.all).hasSize(2);
- int indexAdmin = admin.fullName().equals(cr.all.get(0).name) ? 0 : 1;
- int indexUser = indexAdmin == 0 ? 1 : 0;
- assertThat(cr.all.get(indexAdmin).name).isEqualTo(admin.fullName());
- assertThat(cr.all.get(indexAdmin).value.intValue()).isEqualTo(1);
- assertThat(cr.all.get(indexUser).name).isEqualTo(user.fullName());
- assertThat(cr.all.get(indexUser).value.intValue()).isEqualTo(0);
+ ApprovalInfo approvalInfo = Iterables.getOnlyElement(cr.all);
+ assertThat(approvalInfo.name).isEqualTo(admin.fullName());
+ assertThat(approvalInfo.value.intValue()).isEqualTo(1);
assertThat(Iterables.getLast(ci.messages).message)
.isEqualTo("Uploaded patch set 1: Code-Review+1.");
- // Check that the user who pushed the change was added as a reviewer since they added a vote
- assertThatUserIsOnlyReviewer(ci, admin);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java b/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java
index 9298b43..00b1c55 100644
--- a/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java
@@ -14,8 +14,6 @@
package com.google.gerrit.acceptance.rest.auth;
-import static com.google.common.truth.Truth.assertThat;
-
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.RestSession;
@@ -34,6 +32,5 @@
RestSession anonymous = new RestSession(server, null);
RestResponse r = anonymous.get("/auth-check");
r.assertForbidden();
- assertThat(r.getHeader("Content-Length")).isEqualTo("0");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index 9b12f29..b2a349e 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -1714,7 +1714,7 @@
.sent("newpatchset", sc)
.notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
.to(sc.reviewer)
- .to(other)
+ .cc(other)
.cc(sc.ccer)
.cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
@@ -1730,7 +1730,7 @@
.sent("newpatchset", sc)
.notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
.to(sc.reviewer)
- .to(other)
+ .cc(other)
.cc(sc.ccer)
.cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
diff --git a/polygerrit-ui/app/constants/constants.ts b/polygerrit-ui/app/constants/constants.ts
index b64a7d1..6029dc1 100644
--- a/polygerrit-ui/app/constants/constants.ts
+++ b/polygerrit-ui/app/constants/constants.ts
@@ -327,17 +327,6 @@
}
/**
- * Whether whitespace changes should be ignored and if yes, which whitespace changes should be ignored
- * https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#diff-preferences-input
- */
-export enum IgnoreWhitespaceType {
- IGNORE_NONE = 'IGNORE_NONE',
- IGNORE_TRAILING = 'IGNORE_TRAILING',
- IGNORE_LEADING_AND_TRAILING = 'IGNORE_LEADING_AND_TRAILING',
- IGNORE_ALL = 'IGNORE_ALL',
-}
-
-/**
* how draft comments are handled
*/
export enum DraftsAction {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 1119a20..2c9b31c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -98,7 +98,6 @@
ConfigInfo,
PreferencesInfo,
CommitInfo,
- DiffPreferencesInfo,
RevisionInfo,
EditInfo,
LabelNameToInfoMap,
@@ -107,6 +106,7 @@
ApprovalInfo,
ElementPropertyDeepChange,
} from '../../../types/common';
+import {DiffPreferencesInfo} from '../../../types/diff';
import {GrReplyDialog, FocusTarget} from '../gr-reply-dialog/gr-reply-dialog';
import {GrIncludedInDialog} from '../gr-included-in-dialog/gr-included-in-dialog';
import {GrDownloadDialog} from '../gr-download-dialog/gr-download-dialog';
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index b86dd90..c9a7058 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -46,10 +46,10 @@
PatchSetNum,
CommitInfo,
ServerInfo,
- DiffPreferencesInfo,
RevisionInfo,
NumericChangeId,
} from '../../../types/common';
+import {DiffPreferencesInfo} from '../../../types/diff';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {GrDiffModeSelector} from '../../diff/gr-diff-mode-selector/gr-diff-mode-selector';
import {DiffViewMode} from '../../../constants/constants';
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index e188254..e5107f1 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -57,7 +57,6 @@
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {
ConfigInfo,
- DiffPreferencesInfo,
ElementPropertyDeepChange,
FileInfo,
FileNameToFileInfoMap,
@@ -67,6 +66,7 @@
RevisionInfo,
UrlEncodedCommentId,
} from '../../../types/common';
+import {DiffPreferencesInfo} from '../../../types/diff';
import {GrDiffHost} from '../../diff/gr-diff-host/gr-diff-host';
import {GrDiffPreferencesDialog} from '../../diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog';
import {hasOwnProperty} from '../../../utils/common-util';
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index 0e73516..66f3b81 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -29,14 +29,13 @@
import {ParsedChangeInfo} from '../../shared/gr-rest-api-interface/gr-reviewer-updates-parser';
import {
NumericChangeId,
- DiffInfo,
- DiffPreferencesInfo,
EditPatchSetNum,
FixId,
FixSuggestionInfo,
PatchSetNum,
RobotId,
} from '../../../types/common';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {isRobot} from '../../../utils/comment-util';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.ts
index 7a26e77..763a524 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.ts
@@ -16,7 +16,7 @@
*/
import {GrDiffBuilderUnified} from './gr-diff-builder-unified';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/common';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
export class GrDiffBuilderBinary extends GrDiffBuilderUnified {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
index 6d81e9b..9e64ee6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -31,12 +31,8 @@
import {GrDiffBuilderBinary} from './gr-diff-builder-binary';
import {CancelablePromise, util} from '../../../scripts/util';
import {customElement, property, observe} from '@polymer/decorators';
-import {
- BlameInfo,
- DiffInfo,
- DiffPreferencesInfo,
- ImageInfo,
-} from '../../../types/common';
+import {BlameInfo, ImageInfo} from '../../../types/common';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {CoverageRange, DiffLayer} from '../../../types/types';
import {
GrDiffProcessor,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
index 15264ea..5b3f225 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
@@ -16,7 +16,8 @@
*/
import {GrDiffBuilderSideBySide} from './gr-diff-builder-side-by-side';
-import {DiffInfo, DiffPreferencesInfo, ImageInfo} from '../../../types/common';
+import {ImageInfo} from '../../../types/common';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {GrEndpointParam} from '../../plugins/gr-endpoint-param/gr-endpoint-param';
// MIME types for images we allow showing. Do not include SVG, it can contain
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
index 657dfa2..c203b33 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
@@ -17,7 +17,7 @@
import {GrDiffBuilder} from './gr-diff-builder';
import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/common';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {GrDiffLine, LineNumber} from '../gr-diff/gr-diff-line';
import {DiffViewMode, Side} from '../../../constants/constants';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
index 2028b0c..9709e74 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
@@ -17,7 +17,7 @@
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
import {GrDiffBuilder} from './gr-diff-builder';
import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/common';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {DiffViewMode, Side} from '../../../constants/constants';
export class GrDiffBuilderUnified extends GrDiffBuilder {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index a3778ef..119bc15 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -23,7 +23,8 @@
hideInContextControl,
rangeBySide,
} from '../gr-diff/gr-diff-group';
-import {BlameInfo, DiffInfo, DiffPreferencesInfo} from '../../../types/common';
+import {BlameInfo} from '../../../types/common';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {DiffViewMode, Side} from '../../../constants/constants';
import {DiffLayer} from '../../../types/types';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index e42eb84..fc7c870 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -64,14 +64,6 @@
return htmlTemplate;
}
- private _boundHandleWindowScroll: () => void;
-
- private _boundHandleDiffRenderStart: () => void;
-
- private _boundHandleDiffRenderContent: () => void;
-
- private _boundHandleDiffLineSelected: (e: Event) => void;
-
private _preventAutoScrollOnManualScroll = false;
private lastDisplayedNavigateToNextFileToast: number | null = null;
@@ -110,15 +102,6 @@
@property({type: Boolean})
_listeningForScroll = false;
- constructor() {
- super();
- this._boundHandleWindowScroll = () => this._handleWindowScroll();
- this._boundHandleDiffRenderStart = () => this._handleDiffRenderStart();
- this._boundHandleDiffRenderContent = () => this._handleDiffRenderContent();
- this._boundHandleDiffLineSelected = (e: Event) =>
- this._handleDiffLineSelected(e);
- }
-
/** @override */
ready() {
super.ready();
@@ -340,13 +323,13 @@
this._scrollMode = ScrollMode.KEEP_VISIBLE;
}
- _handleWindowScroll() {
+ private _boundHandleWindowScroll = () => {
if (this._preventAutoScrollOnManualScroll) {
this._scrollMode = ScrollMode.NEVER;
this._focusOnMove = false;
this._preventAutoScrollOnManualScroll = false;
}
- }
+ };
reInitAndUpdateStops() {
this.reInit();
@@ -358,25 +341,29 @@
this.reInitCursor();
}
- _handleDiffRenderStart() {
- this._preventAutoScrollOnManualScroll = true;
- }
+ private boundHandleDiffLoadingChanged = () => {
+ this._updateStops();
+ };
- _handleDiffRenderContent() {
+ private _boundHandleDiffRenderStart = () => {
+ this._preventAutoScrollOnManualScroll = true;
+ };
+
+ private _boundHandleDiffRenderContent = () => {
this._updateStops();
// When done rendering, turn focus on move and automatic scrolling back on
this._focusOnMove = true;
this._preventAutoScrollOnManualScroll = false;
- }
+ };
- _handleDiffLineSelected(event: Event) {
+ private _boundHandleDiffLineSelected = (event: Event) => {
const customEvent = event as CustomEvent;
this.moveToLineNumber(
customEvent.detail.number,
customEvent.detail.side,
customEvent.detail.path
);
- }
+ };
createCommentInPlace() {
const diffWithRangeSelected = this.diffs.find(diff =>
@@ -399,7 +386,6 @@
* {leftSide: true, number: 321} for line 321 of the base patch.
* Returns null if an address is not available.
*
- * @return
*/
getAddress() {
if (!this.diffRow) {
@@ -556,6 +542,10 @@
// might be the same.
for (i = 0; i < splice?.removed.length; i++) {
splice.removed[i].removeEventListener(
+ 'loading-changed',
+ this.boundHandleDiffLoadingChanged
+ );
+ splice.removed[i].removeEventListener(
'render-start',
this._boundHandleDiffRenderStart
);
@@ -571,6 +561,10 @@
for (i = splice.index; i < splice.index + splice.addedCount; i++) {
this.diffs[i].addEventListener(
+ 'loading-changed',
+ this.boundHandleDiffLoadingChanged
+ );
+ this.diffs[i].addEventListener(
'render-start',
this._boundHandleDiffRenderStart
);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
index 5619acc..f01de5f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
@@ -101,14 +101,14 @@
test('cursor scroll behavior', () => {
assert.equal(cursorElement._scrollMode, 'keep-visible');
- cursorElement._handleDiffRenderStart();
+ diffElement.dispatchEvent(new Event('render-start'));
assert.isTrue(cursorElement._focusOnMove);
- cursorElement._handleWindowScroll();
+ window.dispatchEvent(new Event('scroll'));
assert.equal(cursorElement._scrollMode, 'never');
assert.isFalse(cursorElement._focusOnMove);
- cursorElement._handleDiffRenderContent();
+ diffElement.dispatchEvent(new Event('render-content'));
assert.isTrue(cursorElement._focusOnMove);
cursorElement.reInitCursor();
@@ -118,7 +118,7 @@
test('moves to selected line', () => {
const moveToNumStub = sinon.stub(cursorElement, 'moveToLineNumber');
- cursorElement._handleDiffLineSelected(
+ diffElement.dispatchEvent(
new CustomEvent('line-selected', {
detail: {number: '123', side: 'right', path: 'some/file'},
}));
@@ -474,6 +474,12 @@
});
});
+ test('updates stops when loading changes', () => {
+ sinon.spy(cursorElement, '_updateStops');
+ diffElement.dispatchEvent(new Event('loading-changed'));
+ assert.isTrue(cursorElement._updateStops.called);
+ });
+
suite('gr-diff-cursor event tests', () => {
let someEmptyDiv;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index da7d900..875b8c8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -44,33 +44,32 @@
CoverageRange,
DiffLayer,
DiffLayerListener,
+ PatchSetFile,
} from '../../../types/types';
import {
Base64ImageFile,
BlameInfo,
ChangeInfo,
CommentRange,
- DiffInfo,
- DiffPreferencesInfo,
NumericChangeId,
PatchRange,
PatchSetNum,
RepoName,
} from '../../../types/common';
+import {
+ DiffInfo,
+ DiffPreferencesInfo,
+ IgnoreWhitespaceType,
+} from '../../../types/diff';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {JsApiService} from '../../shared/gr-js-api-interface/gr-js-api-types';
import {GrDiff, LineOfInterest} from '../gr-diff/gr-diff';
import {GrSyntaxLayer} from '../gr-syntax-layer/gr-syntax-layer';
-import {
- DiffViewMode,
- IgnoreWhitespaceType,
- Side,
-} from '../../../constants/constants';
+import {DiffViewMode, Side} from '../../../constants/constants';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select';
import {LineNumber} from '../gr-diff/gr-diff-line';
import {GrCommentThread} from '../../shared/gr-comment-thread/gr-comment-thread';
-import {PatchSetFile} from '../../../types/types';
import {KnownExperimentId} from '../../../services/flags/flags';
const MSG_EMPTY_BLAME = 'No blame information for this diff.';
@@ -329,7 +328,6 @@
/**
* @param shouldReportMetric indicate a new Diff Page. This is a
* signal to report metrics event that started on location change.
- * @return
*/
async reload(shouldReportMetric?: boolean) {
this.clear();
@@ -938,7 +936,7 @@
_getIgnoreWhitespace(): IgnoreWhitespaceType {
if (!this.prefs || !this.prefs.ignore_whitespace) {
- return IgnoreWhitespaceType.IGNORE_NONE;
+ return 'IGNORE_NONE';
}
return this.prefs.ignore_whitespace;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.ts b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.ts
index c66af58..8829afc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.ts
@@ -26,7 +26,7 @@
import {GrDiffPreferences} from '../../shared/gr-diff-preferences/gr-diff-preferences';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
-import {DiffPreferencesInfo} from '../../../types/common';
+import {DiffPreferencesInfo} from '../../../types/diff';
export interface GrDiffPreferencesDialog {
$: {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
index ab7ab8a..850e3ef 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
@@ -30,7 +30,7 @@
} from '../gr-diff/gr-diff-group';
import {CancelablePromise, util} from '../../../scripts/util';
import {customElement, property} from '@polymer/decorators';
-import {DiffContent} from '../../../types/common';
+import {DiffContent} from '../../../types/diff';
import {Side} from '../../../constants/constants';
const WHOLE_FILE = -1;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
index b75ba8f..03740ba 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
@@ -27,7 +27,7 @@
} from '../gr-diff-highlight/gr-range-normalizer';
import {descendedFromClass, querySelectorAll} from '../../../utils/dom-util';
import {customElement, property, observe} from '@polymer/decorators';
-import {DiffInfo} from '../../../types/common';
+import {DiffInfo} from '../../../types/diff';
import {Side} from '../../../constants/constants';
import {GrDiffBuilderElement} from '../gr-diff-builder/gr-diff-builder-element';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 0b2757f..fce8c30 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -75,8 +75,6 @@
ChangeInfo,
CommitId,
ConfigInfo,
- DiffInfo,
- DiffPreferencesInfo,
EditInfo,
EditPatchSetNum,
ElementPropertyDeepChange,
@@ -89,6 +87,7 @@
RepoName,
RevisionInfo,
} from '../../../types/common';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {ChangeViewState, CommitRange, FileRange} from '../../../types/types';
import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
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 14b8666..d079639 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -34,14 +34,16 @@
import {
BlameInfo,
CommentRange,
- DiffInfo,
- DiffPreferencesInfo,
- DiffPreferencesInfoKey,
EditPatchSetNum,
ImageInfo,
ParentPatchSetNum,
PatchRange,
} from '../../../types/common';
+import {
+ DiffInfo,
+ DiffPreferencesInfo,
+ DiffPreferencesInfoKey,
+} from '../../../types/diff';
import {GrDiffHighlight} from '../gr-diff-highlight/gr-diff-highlight';
import {GrDiffBuilderElement} from '../gr-diff-builder/gr-diff-builder-element';
import {
@@ -192,9 +194,18 @@
@property({type: Object})
lineOfInterest?: LineOfInterest;
- /** True when diff is changed, until the content is done rendering. */
- @property({type: Boolean})
- _loading = true;
+ /**
+ * True when diff is changed, until the content is done rendering.
+ *
+ * This is readOnly, meaning one can listen for the loading-changed event, but
+ * not write to it from the outside. Code in this class should use the
+ * "private" _setLoading method.
+ */
+ @property({type: Boolean, notify: true, readOnly: true})
+ loading!: boolean;
+
+ // Polymer generated when setting readOnly above.
+ _setLoading!: (loading: boolean) => void;
@property({type: Boolean})
loggedIn = false;
@@ -276,6 +287,7 @@
/** @override */
created() {
super.created();
+ this._setLoading(true);
this.addEventListener('create-range-comment', (e: Event) =>
this._handleCreateRangeComment(e as CustomEvent)
);
@@ -406,7 +418,6 @@
* The key locations based on the comments and line of interests,
* where lines should not be collapsed.
*
- * @return
*/
_computeKeyLocations() {
const keyLocations: KeyLocations = {left: {}, right: {}};
@@ -463,7 +474,7 @@
getCursorStops(): Array<HTMLElement | AbortStop> {
if (this.hidden && this.noAutoRender) return [];
- if (this._loading) {
+ if (this.loading) {
return [new AbortStop()];
}
@@ -833,7 +844,7 @@
}
_diffChanged(newValue?: DiffInfo) {
- this._loading = true;
+ this._setLoading(true);
this._cleanup();
if (newValue) {
this._diffLength = this.getDiffLength(newValue);
@@ -893,7 +904,7 @@
}
_handleRenderContent() {
- this._loading = false;
+ this._setLoading(false);
this._unobserveIncrementalNodes();
this._incrementalNodeObserver = (dom(
this
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 48a4596..b587d8a 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
@@ -27,7 +27,10 @@
:host {
font-family: var(--monospace-font-family, ''), 'Roboto Mono';
font-size: var(--font-size, var(--font-size-code, 12px));
- line-height: var(--line-height-code, 1.334);
+ /* usually 16px = 12px + 4px */
+ line-height: calc(
+ var(--font-size, var(--font-size-code, 12px)) + var(--spacing-s, 4px)
+ );
}
.thread-group {
@@ -142,7 +145,7 @@
width: 100%;
}
.full-width .contentText {
- white-space: pre-wrap;
+ white-space: break-spaces;
word-wrap: break-word;
}
.lineNumButton,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
index 5e95d83..3c53697 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
@@ -605,7 +605,7 @@
};
element._renderDiffTable();
- element._loading = false;
+ element._setLoading(false);
flush();
}
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
index 334c8f4..f5e5e04 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
@@ -23,8 +23,8 @@
import {FILE, GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
import {CancelablePromise, util} from '../../../scripts/util';
import {customElement, property} from '@polymer/decorators';
+import {DiffFileMetaInfo, DiffInfo} from '../../../types/diff';
import {DiffLayer, DiffLayerListener, HighlightJS} from '../../../types/types';
-import {DiffFileMetaInfo, DiffInfo} from '../../../types/common';
import {GrLibLoader} from '../../shared/gr-lib-loader/gr-lib-loader';
import {Side} from '../../../constants/constants';
diff --git a/polygerrit-ui/app/elements/edit/gr-default-editor/gr-default-editor_html.ts b/polygerrit-ui/app/elements/edit/gr-default-editor/gr-default-editor_html.ts
index ba8af3c..77b4bc6 100644
--- a/polygerrit-ui/app/elements/edit/gr-default-editor/gr-default-editor_html.ts
+++ b/polygerrit-ui/app/elements/edit/gr-default-editor/gr-default-editor_html.ts
@@ -23,7 +23,8 @@
box-sizing: border-box;
font-family: var(--monospace-font-family);
font-size: var(--font-size-code);
- line-height: var(--line-height-code);
+ /* usually 16px = 12px + 4px */
+ line-height: calc(var(--font-size-code) + var(--spacing-s));
min-height: 60vh;
resize: none;
white-space: pre;
diff --git a/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.ts b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.ts
index 419c8db..5c57208 100644
--- a/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.ts
@@ -22,13 +22,6 @@
* 2. we have css variables which are more recommended way to custom styling
*/
-/**
- * // import { useShadow } from '@polymer/polymer/lib/utils/settings';
- * TODO(TS): polymer/lib/utils/settings.d.ts is not exporting useShadow
- * while the js is, to avoid the error, re-define it here
- */
-const useShadow = !window.ShadyDOM || !window.ShadyDOM.inUse;
-
let styleObjectCount = 0;
interface PgElement extends Element {
@@ -50,10 +43,9 @@
* if it hasn't been added yet. A root node is an document or is the
* associated shadowRoot. This class can be added to any element with the same
* root node.
- *
*/
getClassName(element: Element) {
- let rootNodeEl = useShadow ? element.getRootNode() : document.body;
+ let rootNodeEl = element.getRootNode();
if (rootNodeEl === document) {
rootNodeEl = document.head;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
index 9fdbb34..352bc58 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
@@ -419,9 +419,6 @@
return top;
}
- /**
- * @return
- */
_targetIsVisible(top: number) {
const dims = this._getWindowDims();
return (
diff --git a/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.ts b/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.ts
index 02d039a..e781820c 100644
--- a/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.ts
+++ b/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.ts
@@ -25,7 +25,7 @@
import {htmlTemplate} from './gr-diff-preferences_html';
import {customElement, property} from '@polymer/decorators';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
-import {DiffPreferencesInfo} from '../../../types/common';
+import {DiffPreferencesInfo} from '../../../types/diff';
import {GrSelect} from '../gr-select/gr-select';
export interface GrDiffPreferences {
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_html.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_html.ts
index bc1dfe0..1688a0d 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_html.ts
@@ -59,7 +59,8 @@
gr-linked-text.pre {
font-family: var(--monospace-font-family);
font-size: var(--font-size-code);
- line-height: var(--line-height-code);
+ /* usually 16px = 12px + 4px */
+ line-height: calc(var(--font-size-code) + var(--spacing-s));
}
</style>
<div id="container"></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 1a89008..0629457 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
@@ -72,9 +72,7 @@
DashboardId,
DashboardInfo,
DeleteDraftCommentsInput,
- DiffInfo,
DiffPreferenceInput,
- DiffPreferencesInfo,
EditPatchSetNum,
EditPreferencesInfo,
EncodedGroupId,
@@ -142,6 +140,11 @@
MergeableInfo,
} from '../../../types/common';
import {
+ DiffInfo,
+ DiffPreferencesInfo,
+ IgnoreWhitespaceType,
+} from '../../../types/diff';
+import {
CancelConditionCallback,
ErrorCallback,
RestApiService,
@@ -152,7 +155,6 @@
CommentSide,
DiffViewMode,
HttpMethod,
- IgnoreWhitespaceType,
ReviewerState,
} from '../../../constants/constants';
@@ -828,7 +830,7 @@
context: 10,
cursor_blink_rate: 0,
font_size: 12,
- ignore_whitespace: IgnoreWhitespaceType.IGNORE_NONE,
+ ignore_whitespace: 'IGNORE_NONE',
intraline_difference: true,
line_length: 100,
line_wrapping: false,
@@ -2525,7 +2527,7 @@
const params: GetDiffParams = {
context: 'ALL',
intraline: null,
- whitespace: whitespace || IgnoreWhitespaceType.IGNORE_NONE,
+ whitespace: whitespace || 'IGNORE_NONE',
};
if (isMergeParent(basePatchNum)) {
params.parent = getParentIndex(basePatchNum);
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_html.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_html.ts
index 1f777aa..d55481b 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_html.ts
@@ -31,7 +31,8 @@
:host(.code) {
font-family: var(--monospace-font-family);
font-size: var(--font-size-code);
- line-height: var(--line-height-code);
+ /* usually 16px = 12px + 4px */
+ line-height: calc(var(--font-size-code) + var(--spacing-s));
font-weight: var(--font-weight-normal);
}
#emojiSuggestions {
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 047e9e0..d6b2237 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -25,6 +25,5 @@
*/
export enum KnownExperimentId {
PATCHSET_COMMENTS = 'UiFeature__patchset_comments',
- PATCHSET_CHOICE_FOR_COMMENT_LINKS = 'UiFeature__patchset_choice_for_comment_links',
NEW_CONTEXT_CONTROLS = 'UiFeature__new_context_controls',
}
diff --git a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
index 08dbb16..19a8dc5 100644
--- a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+import {HttpMethod} from '../../../constants/constants';
import {
AccountDetailInfo,
AccountExternalIdInfo,
@@ -29,7 +30,6 @@
PatchSetNum,
RequestPayload,
PreferencesInput,
- DiffPreferencesInfo,
EditPreferencesInfo,
DiffPreferenceInput,
SshKeyInfo,
@@ -86,7 +86,6 @@
EmailAddress,
FixId,
FilePathToDiffInfoMap,
- DiffInfo,
BlameInfo,
PatchRange,
ImagesForDiff,
@@ -101,8 +100,12 @@
MergeableInfo,
CommitInfo,
} from '../../../types/common';
+import {
+ DiffInfo,
+ DiffPreferencesInfo,
+ IgnoreWhitespaceType,
+} from '../../../types/diff';
import {ParsedChangeInfo} from '../../../elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser';
-import {HttpMethod, IgnoreWhitespaceType} from '../../../constants/constants';
export type ErrorCallback = (response?: Response | null, err?: Error) => void;
export type CancelConditionCallback = () => boolean;
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 9586c09..d7b96c8 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -122,7 +122,6 @@
--font-size-h3: 1.143rem; /* 16px */
--font-size-h2: 1.429rem; /* 20px */
--font-size-h1: 1.714rem; /* 24px */
- --line-height-code: 1.143rem; /* 16px */
--line-height-mono: 1.286rem; /* 18px */
--line-height-small: 1.143rem; /* 16px */
--line-height-normal: 1.429rem; /* 20px */
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 96cb02a..b454b6e 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -37,7 +37,6 @@
TimeFormat,
EmailStrategy,
DefaultBase,
- IgnoreWhitespaceType,
UserPriority,
DiffViewMode,
DraftsAction,
@@ -50,6 +49,8 @@
} from '../constants/constants';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
+import {DiffInfo, IgnoreWhitespaceType, WebLinkInfo} from './diff';
+
export type BrandType<T, BrandName extends string> = T &
{[__brand in BrandName]: never};
@@ -716,16 +717,6 @@
}
/**
- * The WebLinkInfo entity describes a link to an external site.
- * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#web-link-info
- */
-export interface WebLinkInfo {
- name: string;
- url: string;
- image_url: string;
-}
-
-/**
* The VotingRangeInfo entity describes the continuous voting range from minto
* max values.
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#voting-range-info
@@ -1240,103 +1231,9 @@
export type LabelTypeInfoValues = {[value: string]: string};
-/**
- * The DiffContent entity contains information about the content differences in a file.
- * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-content
- */
-export interface DiffContent {
- a?: string[];
- b?: string[];
- ab?: string[];
- // The inner array is always of length two. The first entry is the 'skip'
- // length. The second entry is the 'edit' length.
- edit_a?: number[][];
- edit_b?: number[][];
- due_to_rebase?: boolean;
- due_to_move?: boolean;
- skip?: number;
- common?: string;
- keyLocation?: boolean;
-}
-
-/**
- * The DiffFileMetaInfo entity contains meta information about a file diff.
- * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-file-meta-info
- */
-export interface DiffFileMetaInfo {
- name: string;
- content_type: string;
- lines: string;
- web_links?: WebLinkInfo[];
- language?: string;
-}
-
-/**
- * The DiffInfo entity contains information about the diff of a file in a revision.
- * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-info
- */
-export interface DiffInfo {
- meta_a: DiffFileMetaInfo;
- meta_b: DiffFileMetaInfo;
- change_type: string;
- intraline_status: string;
- diff_header: string[];
- content: DiffContent[];
- web_links?: DiffWebLinkInfo[];
- binary: boolean;
-}
-
export type FilePathToDiffInfoMap = {[path: string]: DiffInfo};
/**
- * The DiffWebLinkInfo entity describes a link on a diff screen to an external site.
- * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-web-link-info
- */
-export interface DiffWebLinkInfo {
- name: string;
- url: string;
- image_url: string;
- show_on_side_by_side_diff_view: string;
- show_on_unified_diff_view: string;
-}
-
-/**
- * The DiffPreferencesInfo entity contains information about the diff preferences of a user.
- * https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#diff-preferences-info
- */
-export interface DiffPreferencesInfo {
- context: number;
- expand_all_comments?: boolean;
- ignore_whitespace: IgnoreWhitespaceType;
- intraline_difference?: boolean;
- line_length: number;
- cursor_blink_rate: number;
- manual_review?: boolean;
- retain_header?: boolean;
- show_line_endings?: boolean;
- show_tabs?: boolean;
- show_whitespace_errors?: boolean;
- skip_deleted?: boolean;
- skip_uncommented?: boolean;
- syntax_highlighting?: boolean;
- hide_top_menu?: boolean;
- auto_hide_diff_table_header?: boolean;
- hide_line_numbers?: boolean;
- tab_size: number;
- font_size: number;
- hide_empty_pane?: boolean;
- match_brackets?: boolean;
- line_wrapping?: boolean;
- // TODO(TS): show_file_comment_button exists in JS code, but doesn't exist in the doc.
- // Either remove or update doc
- show_file_comment_button?: boolean;
- // TODO(TS): theme exists in JS code, but doesn't exist in the doc.
- // Either remove or update doc
- theme?: string;
-}
-export type DiffPreferencesInfoKey = keyof DiffPreferencesInfo;
-
-/**
* The RangeInfo entity stores the coordinates of a range.
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#range-info
*/
diff --git a/polygerrit-ui/app/types/diff.d.ts b/polygerrit-ui/app/types/diff.d.ts
new file mode 100644
index 0000000..417798a
--- /dev/null
+++ b/polygerrit-ui/app/types/diff.d.ts
@@ -0,0 +1,222 @@
+/**
+ * @fileoverview The Gerrit diff API.
+ *
+ * This API is used by other apps embedding gr-diff and any breaking changes
+ * should be discussed with the Gerrit core team and properly versioned.
+ *
+ * Should only contain types, no values, so that other apps using gr-diff can
+ * use this solely to type check and generate externs for their separate ts
+ * bundles.
+ *
+ * Should declare all types, to avoid renaming breaking multi-bundle setups.
+ *
+ * Enums should be converted to union types to avoid values in this file.
+ *
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * The DiffInfo entity contains information about the diff of a file in a
+ * revision.
+ *
+ * If the weblinks-only parameter is specified, only the web_links field is set.
+ */
+export declare interface DiffInfo {
+ /** Meta information about the file on side A as a DiffFileMetaInfo entity. */
+ meta_a: DiffFileMetaInfo;
+ /** Meta information about the file on side B as a DiffFileMetaInfo entity. */
+ meta_b: DiffFileMetaInfo;
+ /** The type of change (ADDED, MODIFIED, DELETED, RENAMED COPIED, REWRITE). */
+ change_type: ChangeType;
+ /** Intraline status (OK, ERROR, TIMEOUT). */
+ intraline_status: 'OK' | 'Error' | 'Timeout';
+ /** A list of strings representing the patch set diff header. */
+ diff_header: string[];
+ /** The content differences in the file as a list of DiffContent entities. */
+ content: DiffContent[];
+ /**
+ * Links to the file diff in external sites as a list of DiffWebLinkInfo
+ * entries.
+ */
+ web_links?: DiffWebLinkInfo[];
+ /** Whether the file is binary. */
+ binary?: boolean;
+}
+
+/**
+ * The DiffFileMetaInfo entity contains meta information about a file diff.
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-file-meta-info
+ */
+export declare interface DiffFileMetaInfo {
+ /** The name of the file. */
+ name: string;
+ /** The content type of the file. */
+ content_type: string;
+ /** The total number of lines in the file. */
+ lines: number;
+ /** Links to the file in external sites as a list of WebLinkInfo entries. */
+ web_links: WebLinkInfo[];
+ // TODO: Not documented.
+ language?: string;
+}
+
+export declare type ChangeType =
+ | 'ADDED'
+ | 'MODIFIED'
+ | 'DELETED'
+ | 'RENAMED'
+ | 'COPIED'
+ | 'REWRITE';
+
+/**
+ * The DiffContent entity contains information about the content differences in
+ * a file.
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-content
+ */
+export declare interface DiffContent {
+ /** Content only in the file on side A (deleted in B). */
+ a?: string[];
+ /** Content only in the file on side B (added in B). */
+ b?: string[];
+ /** Content in the file on both sides (unchanged). */
+ ab?: string[];
+ /**
+ * Text sections deleted from side A as a DiffIntralineInfo entity.
+ *
+ * Only present during a replace, i.e. both a and b are present.
+ */
+ edit_a?: DiffIntralineInfo[];
+ /**
+ * Text sections inserted in side B as a DiffIntralineInfo entity.
+ *
+ * Only present during a replace, i.e. both a and b are present.
+ */
+ edit_b?: DiffIntralineInfo[];
+ /** Indicates whether this entry was introduced by a rebase. */
+ due_to_rebase?: boolean;
+ /** Indicates whether this entry was introduced by a move. */
+ due_to_move?: boolean;
+ /**
+ * Count of lines skipped on both sides when the file is too large to include
+ * all common lines.
+ */
+ skip?: number;
+ /**
+ * Set to true if the region is common according to the requested
+ * ignore-whitespace parameter, but a and b contain differing amounts of
+ * whitespace. When present and true a and b are used instead of ab.
+ */
+ common?: boolean;
+ // TODO: Undocumented, but used in code.
+ keyLocation?: boolean;
+}
+
+/**
+ * The DiffWebLinkInfo entity describes a link on a diff screen to an external
+ * site.
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-web-link-info
+ */
+export declare interface DiffWebLinkInfo {
+ /** The link name. */
+ name: string;
+ /** The link URL. */
+ url: string;
+ /** URL to the icon of the link. */
+ image_url: string;
+ // TODO: Are these really of type string? Not able to trigger them, but the
+ // docs sound more like boolean.
+ show_on_side_by_side_diff_view: string;
+ show_on_unified_diff_view: string;
+}
+/**
+ * The DiffIntralineInfo entity contains information about intraline edits in a
+ * file.
+ *
+ * The information consists of a list of <skip length, mark length> pairs, where
+ * the skip length is the number of characters between the end of the previous
+ * edit and the start of this edit, and the mark length is the number of edited
+ * characters following the skip. The start of the edits is from the beginning
+ * of the related diff content lines.
+ *
+ * Note that the implied newline character at the end of each line is included
+ * in the length calculation, and thus it is possible for the edits to span
+ * newlines.
+ */
+export declare type SkipLength = number;
+export declare type MarkLength = number;
+export declare type DiffIntralineInfo = [SkipLength, MarkLength];
+
+/**
+ * The WebLinkInfo entity describes a link to an external site.
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#web-link-info
+ */
+export declare interface WebLinkInfo {
+ /** The link name. */
+ name: string;
+ /** The link URL. */
+ url: string;
+ /** URL to the icon of the link. */
+ image_url: string;
+}
+
+/**
+ * The DiffPreferencesInfo entity contains information about the diff
+ * preferences of a user.
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#diff-preferences-info
+ */
+export declare interface DiffPreferencesInfo {
+ context: number;
+ expand_all_comments?: boolean;
+ ignore_whitespace: IgnoreWhitespaceType;
+ intraline_difference?: boolean;
+ line_length: number;
+ cursor_blink_rate: number;
+ manual_review?: boolean;
+ retain_header?: boolean;
+ show_line_endings?: boolean;
+ show_tabs?: boolean;
+ show_whitespace_errors?: boolean;
+ skip_deleted?: boolean;
+ skip_uncommented?: boolean;
+ syntax_highlighting?: boolean;
+ hide_top_menu?: boolean;
+ auto_hide_diff_table_header?: boolean;
+ hide_line_numbers?: boolean;
+ tab_size: number;
+ font_size: number;
+ hide_empty_pane?: boolean;
+ match_brackets?: boolean;
+ line_wrapping?: boolean;
+ // TODO(TS): show_file_comment_button exists in JS code, but doesn't exist in
+ // the doc. Either remove or update doc
+ show_file_comment_button?: boolean;
+ // TODO(TS): theme exists in JS code, but doesn't exist in the doc.
+ // Either remove or update doc
+ theme?: string;
+}
+
+export declare type DiffPreferencesInfoKey = keyof DiffPreferencesInfo;
+
+/**
+ * Whether whitespace changes should be ignored and if yes, which whitespace
+ * changes should be ignored
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#diff-preferences-input
+ */
+export declare type IgnoreWhitespaceType =
+ | 'IGNORE_NONE'
+ | 'IGNORE_TRAILING'
+ | 'IGNORE_LEADING_AND_TRAILING'
+ | 'IGNORE_ALL';
diff --git a/polygerrit-ui/app/types/globals.ts b/polygerrit-ui/app/types/globals.ts
index 2962158..28cac87 100644
--- a/polygerrit-ui/app/types/globals.ts
+++ b/polygerrit-ui/app/types/globals.ts
@@ -23,12 +23,6 @@
interface Window {
CANONICAL_PATH?: string;
INITIAL_DATA?: {[key: string]: ParsedJSON};
- ShadyCSS?: {
- getComputedStyleValue(el: Element, name: string): string;
- };
- ShadyDOM?: {
- inUse?: boolean;
- };
HTMLImports?: {whenReady: (cb: () => void) => void};
linkify(
text: string,
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index 76db40b..364112b 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -15,7 +15,6 @@
* limitations under the License.
*/
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
import {JsApiService} from '../elements/shared/gr-js-api-interface/gr-js-api-types';
@@ -70,27 +69,9 @@
/**
* Get computed style value.
- *
- * If ShadyCSS is provided, use ShadyCSS api.
- * If `getComputedStyleValue` is provided on the element, use it.
- * Otherwise fallback to native method (in polymer 2).
- *
*/
-export function getComputedStyleValue(
- name: string,
- el: Element | LegacyElementMixin
-) {
- let style;
- if (window.ShadyCSS) {
- style = window.ShadyCSS.getComputedStyleValue(el as Element, name);
- // `getComputedStyleValue` defined through LegacyElementMixin
- // TODO: It should be safe to just use `getComputedStyle`, but just to be safe
- } else if ('getComputedStyleValue' in el) {
- style = el.getComputedStyleValue(name);
- } else {
- style = getComputedStyle(el).getPropertyValue(name);
- }
- return style;
+export function getComputedStyleValue(name: string, el: Element) {
+ return getComputedStyle(el).getPropertyValue(name).trim();
}
/**
diff --git a/tools/bzl/asciidoc.bzl b/tools/bzl/asciidoc.bzl
index 1e7ec96..7977cf0 100644
--- a/tools/bzl/asciidoc.bzl
+++ b/tools/bzl/asciidoc.bzl
@@ -18,8 +18,7 @@
]
def _replace_macros_impl(ctx):
- cmd = [
- ctx.file._exe.path,
+ args = [
"--suffix",
ctx.attr.suffix,
"-s",
@@ -28,13 +27,14 @@
ctx.outputs.out.path,
]
if ctx.attr.searchbox:
- cmd.append("--searchbox")
+ args.append("--searchbox")
else:
- cmd.append("--no-searchbox")
- ctx.actions.run_shell(
+ args.append("--no-searchbox")
+ ctx.actions.run(
inputs = [ctx.file._exe, ctx.file.src],
outputs = [ctx.outputs.out],
- command = cmd,
+ executable = ctx.file._exe.path,
+ arguments = args,
use_default_shell_env = True,
progress_message = "Replacing macros in %s" % ctx.file.src.short_path,
)
