Merge changes I93022cb7,I4b32d082
* changes:
Rearrange code to avoid dependency on GerritClassLoader internally.
Revert "Revert "Use custom gerrit class loader to avoid usage of reflection""
diff --git a/Documentation/backend_licenses.txt b/Documentation/backend_licenses.txt
index 586406f..45da2ba 100755
--- a/Documentation/backend_licenses.txt
+++ b/Documentation/backend_licenses.txt
@@ -1123,7 +1123,6 @@
* flexmark-ext-footnotes
* flexmark-ext-gfm-issues
* flexmark-ext-gfm-strikethrough
-* flexmark-ext-gfm-tables
* flexmark-ext-gfm-tasklist
* flexmark-ext-gfm-users
* flexmark-ext-ins
@@ -1134,8 +1133,6 @@
* flexmark-ext-typographic
* flexmark-ext-wikilink
* flexmark-ext-yaml-front-matter
-* flexmark-formatter
-* flexmark-html-parser
* flexmark-profile-pegdown
* flexmark-util
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index cea9400..7864858 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -886,6 +886,7 @@
+
If set to 0 the cache is disabled; entries are loaded but not stored
in-memory.
+
+
**NOTE**: When the cache is disabled, there is no locking when accessing
the same key/value, and therefore multiple threads may
@@ -2439,7 +2440,7 @@
[gerrit]
installIndexModule = com.google.gerrit.elasticsearch.ElasticIndexModule
----
-+
+
[[gerrit.installModule]]gerrit.installModule::
+
Repeatable list of class name of additional Guice modules to load at
diff --git a/Documentation/intro-project-owner.txt b/Documentation/intro-project-owner.txt
index f13bc22..97b58af 100644
--- a/Documentation/intro-project-owner.txt
+++ b/Documentation/intro-project-owner.txt
@@ -125,7 +125,7 @@
and link:access-control.html#references_magic[magic refs].
Gerrit only supports tags that are reachable by any ref not owned by
-Gerrit. This includes branches (refs/heads/*) or custom ref namespaces
+Gerrit. This includes branches (refs/heads/\*) or custom ref namespaces
(refs/my-company/*). Tagging a change ref is not supported.
When filtering tags by visibility, Gerrit performs a reachability check
and will present the user ony with tags that are reachable by any ref
diff --git a/Documentation/licenses.txt b/Documentation/licenses.txt
index ab5c255..2499a15 100644
--- a/Documentation/licenses.txt
+++ b/Documentation/licenses.txt
@@ -1127,7 +1127,6 @@
* flexmark-ext-footnotes
* flexmark-ext-gfm-issues
* flexmark-ext-gfm-strikethrough
-* flexmark-ext-gfm-tables
* flexmark-ext-gfm-tasklist
* flexmark-ext-gfm-users
* flexmark-ext-ins
@@ -1138,8 +1137,6 @@
* flexmark-ext-typographic
* flexmark-ext-wikilink
* flexmark-ext-yaml-front-matter
-* flexmark-formatter
-* flexmark-html-parser
* flexmark-profile-pegdown
* flexmark-util
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index af922f1..e8f9a6a 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -7060,11 +7060,8 @@
The user who submitted the change, as an
link:rest-api-accounts.html#account-info[ AccountInfo] entity.
|`starred` |not set if `false`|
-Whether the calling user has starred this change with the default label.
+Whether the calling user has starred this change.
Only set if link:#star[requested].
-|`stars` |optional|
-A list of star labels that are applied by the calling user to this
-change. The labels are lexicographically sorted.
|`reviewed` |not set if `false`|
Whether the change was reviewed by the calling user.
Only set if link:#reviewed[reviewed] is requested.
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 53b6c95..6e7ac6b 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -81,7 +81,6 @@
"//lib:flexmark-ext-footnotes",
"//lib:flexmark-ext-gfm-issues",
"//lib:flexmark-ext-gfm-strikethrough",
- "//lib:flexmark-ext-gfm-tables",
"//lib:flexmark-ext-gfm-tasklist",
"//lib:flexmark-ext-gfm-users",
"//lib:flexmark-ext-ins",
@@ -92,8 +91,6 @@
"//lib:flexmark-ext-typographic",
"//lib:flexmark-ext-wikilink",
"//lib:flexmark-ext-yaml-front-matter",
- "//lib:flexmark-formatter",
- "//lib:flexmark-html-parser",
"//lib:flexmark-profile-pegdown",
"//lib:flexmark-util",
"//lib:gson",
diff --git a/java/com/google/gerrit/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java
index 8b41356..df470ec 100644
--- a/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -14,232 +14,68 @@
package com.google.gerrit.server;
-import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
-import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Objects.requireNonNull;
-import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toSet;
-
import com.google.auto.value.AutoValue;
-import com.google.common.base.CharMatcher;
-import com.google.common.base.Joiner;
-import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSortedSet;
-import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.git.GitUpdateFailureException;
-import com.google.gerrit.git.LockFailureException;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.logging.Metadata;
-import com.google.gerrit.server.logging.TraceContext;
-import com.google.gerrit.server.logging.TraceContext.TraceTimer;
-import com.google.gerrit.server.update.context.RefUpdateContext;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Collection;
-import java.util.Collections;
import java.util.List;
-import java.util.NavigableSet;
-import java.util.Objects;
import java.util.Set;
-import java.util.TreeSet;
-import java.util.stream.Collectors;
-import org.eclipse.jgit.lib.BatchRefUpdate;
-import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.NullProgressMonitor;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.ObjectLoader;
-import org.eclipse.jgit.lib.ObjectReader;
-import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefDatabase;
-import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.transport.ReceiveCommand;
-@Singleton
-public class StarredChangesUtil {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
+public interface StarredChangesUtil {
@AutoValue
- public abstract static class StarField {
+ abstract class StarField {
private static final String SEPARATOR = ":";
@Nullable
public static StarField parse(String s) {
+ Integer id;
int p = s.indexOf(SEPARATOR);
if (p >= 0) {
- Integer id = Ints.tryParse(s.substring(0, p));
- if (id == null) {
- return null;
- }
- Account.Id accountId = Account.id(id);
- String label = s.substring(p + 1);
- return create(accountId, label);
+ id = Ints.tryParse(s.substring(0, p));
+ } else {
+ id = Ints.tryParse(s);
}
- return null;
+ if (id == null) {
+ return null;
+ }
+ return create(Account.id(id));
}
- public static StarField create(Account.Id accountId, String label) {
- return new AutoValue_StarredChangesUtil_StarField(accountId, label);
+ public static StarField create(Account.Id accountId) {
+ return new AutoValue_StarredChangesUtil_StarField(accountId);
}
public abstract Account.Id accountId();
- public abstract String label();
-
@Override
public final String toString() {
- return accountId() + SEPARATOR + label();
+ return accountId().toString();
}
}
- public enum Operation {
- ADD,
- REMOVE
- }
-
- @AutoValue
- public abstract static class StarRef {
- private static final StarRef MISSING =
- new AutoValue_StarredChangesUtil_StarRef(null, Collections.emptyNavigableSet());
-
- private static StarRef create(Ref ref, Iterable<String> labels) {
- return new AutoValue_StarredChangesUtil_StarRef(
- requireNonNull(ref), ImmutableSortedSet.copyOf(labels));
- }
-
- @Nullable
- public abstract Ref ref();
-
- public abstract NavigableSet<String> labels();
-
- public ObjectId objectId() {
- return ref() != null ? ref().getObjectId() : ObjectId.zeroId();
- }
- }
-
- public static class IllegalLabelException extends Exception {
- private static final long serialVersionUID = 1L;
-
- IllegalLabelException(String message) {
- super(message);
- }
- }
-
- public static class InvalidLabelsException extends IllegalLabelException {
- private static final long serialVersionUID = 1L;
-
- InvalidLabelsException(Set<String> invalidLabels) {
- super(String.format("invalid labels: %s", Joiner.on(", ").join(invalidLabels)));
- }
- }
-
- public static class MutuallyExclusiveLabelsException extends IllegalLabelException {
- private static final long serialVersionUID = 1L;
-
- MutuallyExclusiveLabelsException(String label1, String label2) {
- super(
- String.format(
- "The labels %s and %s are mutually exclusive. Only one of them can be set.",
- label1, label2));
- }
- }
-
- public static final String DEFAULT_LABEL = "star";
-
- private final GitRepositoryManager repoManager;
- private final GitReferenceUpdated gitRefUpdated;
- private final AllUsersName allUsers;
- private final Provider<PersonIdent> serverIdent;
-
- @Inject
- StarredChangesUtil(
- GitRepositoryManager repoManager,
- GitReferenceUpdated gitRefUpdated,
- AllUsersName allUsers,
- @GerritPersonIdent Provider<PersonIdent> serverIdent) {
- this.repoManager = repoManager;
- this.gitRefUpdated = gitRefUpdated;
- this.allUsers = allUsers;
- this.serverIdent = serverIdent;
- }
-
- public NavigableSet<String> getLabels(Account.Id accountId, Change.Id changeId) {
- try (Repository repo = repoManager.openRepository(allUsers)) {
- return readLabels(repo, RefNames.refsStarredChanges(changeId, accountId)).labels();
- } catch (IOException e) {
- throw new StorageException(
- String.format(
- "Reading stars from change %d for account %d failed",
- changeId.get(), accountId.get()),
- e);
- }
- }
-
- public void star(Account.Id accountId, Change.Id changeId, Operation op)
- throws IllegalLabelException {
- try (Repository repo = repoManager.openRepository(allUsers)) {
- String refName = RefNames.refsStarredChanges(changeId, accountId);
- StarRef old = readLabels(repo, refName);
-
- NavigableSet<String> labels = new TreeSet<>(old.labels());
- switch (op) {
- case ADD:
- labels.add(DEFAULT_LABEL);
- break;
- case REMOVE:
- labels.remove(DEFAULT_LABEL);
- break;
- }
-
- if (labels.isEmpty()) {
- deleteRef(repo, refName, old.objectId());
- } else {
- updateLabels(repo, refName, old.objectId(), labels);
- }
- } catch (IOException e) {
- throw new StorageException(
- String.format("Star change %d for account %d failed", changeId.get(), accountId.get()),
- e);
- }
- }
+ boolean isStarred(Account.Id accountId, Change.Id changeId);
/**
* Returns a subset of change IDs among the input {@code changeIds} list that are starred by the
* {@code caller} user.
*/
- public Set<Change.Id> areStarred(
- Repository allUsersRepo, List<Change.Id> changeIds, Account.Id caller) {
- List<String> starRefs =
- changeIds.stream()
- .map(c -> RefNames.refsStarredChanges(c, caller))
- .collect(Collectors.toList());
- try {
- return allUsersRepo.getRefDatabase().exactRef(starRefs.toArray(new String[0])).keySet()
- .stream()
- .map(r -> Change.Id.fromAllUsersRef(r))
- .collect(Collectors.toSet());
- } catch (IOException e) {
- logger.atWarning().withCause(e).log(
- "Failed getting starred changes for account %d within changes: %s",
- caller.get(), Joiner.on(", ").join(changeIds));
- return ImmutableSet.of();
- }
- }
+ Set<Change.Id> areStarred(Repository allUsersRepo, List<Change.Id> changeIds, Account.Id caller);
+
+ ImmutableMap<Account.Id, Ref> byChange(Change.Id changeId);
+
+ ImmutableSet<Change.Id> byAccountId(Account.Id accountId);
+
+ ImmutableSet<Change.Id> byAccountId(Account.Id accountId, boolean skipInvalidChanges);
+
+ void star(Account.Id accountId, Change.Id changeId);
+
+ void unstar(Account.Id accountId, Change.Id changeId);
/**
* Unstar the given change for all users.
@@ -250,239 +86,5 @@
* @param changeId change ID.
* @throws IOException if an error occurred.
*/
- public void unstarAllForChangeDeletion(Change.Id changeId) throws IOException {
- try (Repository repo = repoManager.openRepository(allUsers);
- RevWalk rw = new RevWalk(repo)) {
- BatchRefUpdate batchUpdate = repo.getRefDatabase().newBatchUpdate();
- batchUpdate.setAllowNonFastForwards(true);
- batchUpdate.setRefLogIdent(serverIdent.get());
- batchUpdate.setRefLogMessage("Unstar change " + changeId.get(), true);
- for (Account.Id accountId : getStars(repo, changeId)) {
- String refName = RefNames.refsStarredChanges(changeId, accountId);
- Ref ref = repo.getRefDatabase().exactRef(refName);
- if (ref != null) {
- batchUpdate.addCommand(new ReceiveCommand(ref.getObjectId(), ObjectId.zeroId(), refName));
- }
- }
- batchUpdate.execute(rw, NullProgressMonitor.INSTANCE);
- for (ReceiveCommand command : batchUpdate.getCommands()) {
- if (command.getResult() != ReceiveCommand.Result.OK) {
- String message =
- String.format(
- "Unstar change %d failed, ref %s could not be deleted: %s",
- changeId.get(), command.getRefName(), command.getResult());
- if (command.getResult() == ReceiveCommand.Result.LOCK_FAILURE) {
- throw new LockFailureException(message, batchUpdate);
- }
- throw new GitUpdateFailureException(message, batchUpdate);
- }
- }
- }
- }
-
- public ImmutableMap<Account.Id, StarRef> byChange(Change.Id changeId) {
- try (Repository repo = repoManager.openRepository(allUsers)) {
- ImmutableMap.Builder<Account.Id, StarRef> builder = ImmutableMap.builder();
- for (Account.Id accountId : getStars(repo, changeId)) {
- builder.put(accountId, readLabels(repo, RefNames.refsStarredChanges(changeId, accountId)));
- }
- return builder.build();
- } catch (IOException e) {
- throw new StorageException(
- String.format("Get accounts that starred change %d failed", changeId.get()), e);
- }
- }
-
- public ImmutableSet<Change.Id> byAccountId(Account.Id accountId) {
- return byAccountId(accountId, true);
- }
-
- public ImmutableSet<Change.Id> byAccountId(Account.Id accountId, boolean skipInvalidChanges) {
- try (Repository repo = repoManager.openRepository(allUsers)) {
- ImmutableSet.Builder<Change.Id> builder = ImmutableSet.builder();
- for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_STARRED_CHANGES)) {
- Account.Id currentAccountId = Account.Id.fromRef(ref.getName());
- // Skip all refs that don't correspond with accountId.
- if (currentAccountId == null || !currentAccountId.equals(accountId)) {
- continue;
- }
- // Skip all refs that don't contain the required label.
- StarRef starRef = readLabels(repo, ref.getName());
- if (!starRef.labels().contains(DEFAULT_LABEL)) {
- continue;
- }
-
- // Skip invalid change ids.
- Change.Id changeId = Change.Id.fromAllUsersRef(ref.getName());
- if (skipInvalidChanges && changeId == null) {
- continue;
- }
- builder.add(changeId);
- }
- return builder.build();
- } catch (IOException e) {
- throw new StorageException(
- String.format("Get starred changes for account %d failed", accountId.get()), e);
- }
- }
-
- private static Set<Account.Id> getStars(Repository allUsers, Change.Id changeId)
- throws IOException {
- String prefix = RefNames.refsStarredChangesPrefix(changeId);
- RefDatabase refDb = allUsers.getRefDatabase();
- return refDb.getRefsByPrefix(prefix).stream()
- .map(r -> r.getName().substring(prefix.length()))
- .map(refPart -> Ints.tryParse(refPart))
- .filter(Objects::nonNull)
- .map(id -> Account.id(id))
- .collect(toSet());
- }
-
- public ObjectId getObjectId(Account.Id accountId, Change.Id changeId) {
- try (Repository repo = repoManager.openRepository(allUsers)) {
- Ref ref = repo.exactRef(RefNames.refsStarredChanges(changeId, accountId));
- return ref != null ? ref.getObjectId() : ObjectId.zeroId();
- } catch (IOException e) {
- logger.atSevere().withCause(e).log(
- "Getting star object ID for account %d on change %d failed",
- accountId.get(), changeId.get());
- return ObjectId.zeroId();
- }
- }
-
- public static StarRef readLabels(Repository repo, String refName) throws IOException {
- try (TraceTimer traceTimer =
- TraceContext.newTimer(
- "Read star labels", Metadata.builder().noteDbRefName(refName).build())) {
- Ref ref = repo.exactRef(refName);
- return readLabels(repo, ref);
- }
- }
-
- public static StarRef readLabels(Repository repo, Ref ref) throws IOException {
- if (ref == null) {
- return StarRef.MISSING;
- }
- try (TraceTimer traceTimer =
- TraceContext.newTimer(
- String.format("Read star labels from %s (without ref lookup)", ref.getName()));
- ObjectReader reader = repo.newObjectReader()) {
- ObjectLoader obj = reader.open(ref.getObjectId(), Constants.OBJ_BLOB);
- return StarRef.create(
- ref,
- Splitter.on(CharMatcher.whitespace())
- .omitEmptyStrings()
- .split(new String(obj.getCachedBytes(Integer.MAX_VALUE), UTF_8)));
- }
- }
-
- public static ObjectId writeLabels(Repository repo, Collection<String> labels)
- throws IOException, InvalidLabelsException {
- validateLabels(labels);
- try (ObjectInserter oi = repo.newObjectInserter()) {
- ObjectId id =
- oi.insert(
- Constants.OBJ_BLOB,
- labels.stream().sorted().distinct().collect(joining("\n")).getBytes(UTF_8));
- oi.flush();
- return id;
- }
- }
-
- private static void validateLabels(Collection<String> labels) throws InvalidLabelsException {
- if (labels == null) {
- return;
- }
-
- NavigableSet<String> invalidLabels = new TreeSet<>();
- for (String label : labels) {
- if (CharMatcher.whitespace().matchesAnyOf(label)) {
- invalidLabels.add(label);
- }
- }
- if (!invalidLabels.isEmpty()) {
- throw new InvalidLabelsException(invalidLabels);
- }
- }
-
- private void updateLabels(
- Repository repo, String refName, ObjectId oldObjectId, Collection<String> labels)
- throws IOException, InvalidLabelsException {
- try (TraceTimer traceTimer =
- TraceContext.newTimer(
- "Update star labels",
- Metadata.builder().noteDbRefName(refName).resourceCount(labels.size()).build());
- RevWalk rw = new RevWalk(repo)) {
- RefUpdate u = repo.updateRef(refName);
- u.setExpectedOldObjectId(oldObjectId);
- u.setForceUpdate(true);
- u.setNewObjectId(writeLabels(repo, labels));
- u.setRefLogIdent(serverIdent.get());
- u.setRefLogMessage("Update star labels", true);
- try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
- RefUpdate.Result result = u.update(rw);
- switch (result) {
- case NEW:
- case FORCED:
- case NO_CHANGE:
- case FAST_FORWARD:
- gitRefUpdated.fire(allUsers, u, null);
- return;
- case LOCK_FAILURE:
- throw new LockFailureException(
- String.format("Update star labels on ref %s failed", refName), u);
- case IO_FAILURE:
- case NOT_ATTEMPTED:
- case REJECTED:
- case REJECTED_CURRENT_BRANCH:
- case RENAMED:
- case REJECTED_MISSING_OBJECT:
- case REJECTED_OTHER_REASON:
- default:
- throw new StorageException(
- String.format("Update star labels on ref %s failed: %s", refName, result.name()));
- }
- }
- }
- }
-
- private void deleteRef(Repository repo, String refName, ObjectId oldObjectId) throws IOException {
- if (ObjectId.zeroId().equals(oldObjectId)) {
- // ref doesn't exist
- return;
- }
-
- try (TraceTimer traceTimer =
- TraceContext.newTimer(
- "Delete star labels", Metadata.builder().noteDbRefName(refName).build())) {
- RefUpdate u = repo.updateRef(refName);
- u.setForceUpdate(true);
- u.setExpectedOldObjectId(oldObjectId);
- u.setRefLogIdent(serverIdent.get());
- u.setRefLogMessage("Unstar change", true);
- try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
- RefUpdate.Result result = u.delete();
- switch (result) {
- case FORCED:
- gitRefUpdated.fire(allUsers, u, null);
- return;
- case LOCK_FAILURE:
- throw new LockFailureException(String.format("Delete star ref %s failed", refName), u);
- case NEW:
- case NO_CHANGE:
- case FAST_FORWARD:
- case IO_FAILURE:
- case NOT_ATTEMPTED:
- case REJECTED:
- case REJECTED_CURRENT_BRANCH:
- case RENAMED:
- case REJECTED_MISSING_OBJECT:
- case REJECTED_OTHER_REASON:
- default:
- throw new StorageException(
- String.format("Delete star ref %s failed: %s", refName, result.name()));
- }
- }
- }
- }
+ void unstarAllForChangeDeletion(Change.Id changeId) throws IOException;
}
diff --git a/java/com/google/gerrit/server/change/ActionJson.java b/java/com/google/gerrit/server/change/ActionJson.java
index dc8a7dc..52230ba 100644
--- a/java/com/google/gerrit/server/change/ActionJson.java
+++ b/java/com/google/gerrit/server/change/ActionJson.java
@@ -145,7 +145,6 @@
copy.revertOf = changeInfo.revertOf;
copy.submissionId = changeInfo.submissionId;
copy.starred = changeInfo.starred;
- copy.stars = changeInfo.stars;
copy.submitted = changeInfo.submitted;
copy.submitter = changeInfo.submitter;
copy.unresolvedCommentCount = changeInfo.unresolvedCommentCount;
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 8d42fcc..e41566c 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -693,10 +693,8 @@
}
if (user.isIdentifiedUser()) {
- Collection<String> stars = cd.stars(user.getAccountId());
- out.starred = stars.contains(StarredChangesUtil.DEFAULT_LABEL) ? true : null;
- if (!stars.isEmpty()) {
- out.stars = stars;
+ if (cd.isStarred(user.getAccountId())) {
+ out.starred = true;
}
}
diff --git a/java/com/google/gerrit/server/change/ChangeResource.java b/java/com/google/gerrit/server/change/ChangeResource.java
index c5c0be0..234cd2e 100644
--- a/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/java/com/google/gerrit/server/change/ChangeResource.java
@@ -237,7 +237,7 @@
.build())) {
Hasher h = Hashing.murmur3_128().newHasher();
if (user.isIdentifiedUser()) {
- h.putString(starredChangesUtil.getObjectId(user.getAccountId(), getId()).name(), UTF_8);
+ h.putBoolean(starredChangesUtil.isStarred(user.getAccountId(), getId()));
}
prepareETag(h, user);
return h.hash().toString();
diff --git a/java/com/google/gerrit/server/documentation/MarkdownFormatter.java b/java/com/google/gerrit/server/documentation/MarkdownFormatter.java
index 0e911b9..22c6995 100644
--- a/java/com/google/gerrit/server/documentation/MarkdownFormatter.java
+++ b/java/com/google/gerrit/server/documentation/MarkdownFormatter.java
@@ -14,9 +14,9 @@
package com.google.gerrit.server.documentation;
-import static com.vladsch.flexmark.profiles.pegdown.Extensions.ALL;
-import static com.vladsch.flexmark.profiles.pegdown.Extensions.HARDWRAPS;
-import static com.vladsch.flexmark.profiles.pegdown.Extensions.SUPPRESS_ALL_HTML;
+import static com.vladsch.flexmark.parser.PegdownExtensions.ALL;
+import static com.vladsch.flexmark.parser.PegdownExtensions.HARDWRAPS;
+import static com.vladsch.flexmark.parser.PegdownExtensions.SUPPRESS_ALL_HTML;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Strings;
@@ -26,7 +26,7 @@
import com.vladsch.flexmark.ast.util.TextCollectingVisitor;
import com.vladsch.flexmark.html.HtmlRenderer;
import com.vladsch.flexmark.parser.Parser;
-import com.vladsch.flexmark.profiles.pegdown.PegdownOptionsAdapter;
+import com.vladsch.flexmark.profile.pegdown.PegdownOptionsAdapter;
import com.vladsch.flexmark.util.ast.Block;
import com.vladsch.flexmark.util.ast.Node;
import com.vladsch.flexmark.util.data.MutableDataHolder;
diff --git a/java/com/google/gerrit/server/documentation/MarkdownFormatterHeader.java b/java/com/google/gerrit/server/documentation/MarkdownFormatterHeader.java
index 1875b64..04d2e5b 100644
--- a/java/com/google/gerrit/server/documentation/MarkdownFormatterHeader.java
+++ b/java/com/google/gerrit/server/documentation/MarkdownFormatterHeader.java
@@ -23,10 +23,9 @@
import com.vladsch.flexmark.html.renderer.DelegatingNodeRendererFactory;
import com.vladsch.flexmark.html.renderer.NodeRenderer;
import com.vladsch.flexmark.html.renderer.NodeRendererContext;
-import com.vladsch.flexmark.html.renderer.NodeRendererFactory;
import com.vladsch.flexmark.html.renderer.NodeRenderingHandler;
-import com.vladsch.flexmark.profiles.pegdown.Extensions;
-import com.vladsch.flexmark.profiles.pegdown.PegdownOptionsAdapter;
+import com.vladsch.flexmark.profile.pegdown.Extensions;
+import com.vladsch.flexmark.profile.pegdown.PegdownOptionsAdapter;
import com.vladsch.flexmark.util.ast.Node;
import com.vladsch.flexmark.util.data.DataHolder;
import com.vladsch.flexmark.util.data.MutableDataHolder;
@@ -124,8 +123,8 @@
}
@Override
- public Set<Class<? extends NodeRendererFactory>> getDelegates() {
- Set<Class<? extends NodeRendererFactory>> delegates = new HashSet<>();
+ public Set<Class<?>> getDelegates() {
+ Set<Class<?>> delegates = new HashSet<>();
delegates.add(AnchorLinkNodeRenderer.Factory.class);
return delegates;
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 7057ff7..eccd199 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -16,7 +16,6 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.util.AttentionSetUtil.additionsOnly;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -1267,21 +1266,20 @@
public static final IndexedField<ChangeData, Iterable<Integer>>.SearchSpec COMMENTBY_SPEC =
COMMENTBY_FIELD.integer(ChangeQueryBuilder.FIELD_COMMENTBY);
- /** Star labels on this change in the format: <account-id>:<label> */
+ /** Star labels on this change in the format: <account-id> */
public static final IndexedField<ChangeData, Iterable<String>> STAR_FIELD =
IndexedField.<ChangeData>iterableStringBuilder("Star")
.stored()
.build(
cd ->
Iterables.transform(
- cd.stars().entries(),
- e ->
- StarredChangesUtil.StarField.create(e.getKey(), e.getValue()).toString()),
+ cd.stars(),
+ accountId -> StarredChangesUtil.StarField.create(accountId).toString()),
(cd, field) ->
cd.setStars(
StreamSupport.stream(field.spliterator(), false)
- .map(f -> StarredChangesUtil.StarField.parse(f))
- .collect(toImmutableListMultimap(e -> e.accountId(), e -> e.label()))));
+ .map(f -> StarredChangesUtil.StarField.parse(f).accountId())
+ .collect(toImmutableList())));
public static final IndexedField<ChangeData, Iterable<String>>.SearchSpec STAR_SPEC =
STAR_FIELD.exact(ChangeQueryBuilder.FIELD_STAR);
@@ -1289,7 +1287,7 @@
/** Users that have starred the change with any label. */
public static final IndexedField<ChangeData, Iterable<Integer>> STARBY_FIELD =
IndexedField.<ChangeData>iterableIntegerBuilder("StarBy")
- .build(cd -> Iterables.transform(cd.stars().keySet(), Account.Id::get));
+ .build(cd -> Iterables.transform(cd.stars(), Account.Id::get));
public static final IndexedField<ChangeData, Iterable<Integer>>.SearchSpec STARBY_SPEC =
STARBY_FIELD.integer(ChangeQueryBuilder.FIELD_STARBY);
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmailImpl.java b/java/com/google/gerrit/server/mail/send/ChangeEmailImpl.java
index bf6ed0c..d4b9b08 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmailImpl.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmailImpl.java
@@ -19,7 +19,6 @@
import com.google.auto.factory.AutoFactory;
import com.google.auto.factory.Provided;
-import com.google.common.collect.ListMultimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
@@ -40,7 +39,6 @@
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.mail.MailHeader;
-import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.mail.send.ProjectWatch.Watchers;
import com.google.gerrit.server.mail.send.ProjectWatch.Watchers.WatcherList;
@@ -58,9 +56,9 @@
import java.io.IOException;
import java.text.MessageFormat;
import java.time.Instant;
-import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -90,7 +88,7 @@
// Available after init or after being explicitly set.
private OutgoingEmail email;
- private ListMultimap<Account.Id, String> stars;
+ private List<Account.Id> stars;
private PatchSet patchSet;
private PatchSetInfo patchSetInfo;
private String changeMessage;
@@ -414,11 +412,7 @@
return;
}
- for (Map.Entry<Account.Id, Collection<String>> e : stars.asMap().entrySet()) {
- if (e.getValue().contains(StarredChangesUtil.DEFAULT_LABEL)) {
- email.addByAccountId(RecipientType.BCC, e.getKey());
- }
- }
+ stars.forEach(accountId -> email.addByAccountId(RecipientType.BCC, accountId));
}
/** Include users and groups that want notification of events. */
diff --git a/java/com/google/gerrit/server/notedb/NoteDbModule.java b/java/com/google/gerrit/server/notedb/NoteDbModule.java
index d8a5fd5..9323178 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbModule.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbModule.java
@@ -17,6 +17,8 @@
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.server.StarredChangesUtil;
+import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Names;
@@ -44,6 +46,7 @@
factory(NoteDbUpdateManager.Factory.class);
factory(RobotCommentNotes.Factory.class);
factory(RobotCommentUpdate.Factory.class);
+ bind(StarredChangesUtil.class).to(StarredChangesUtilNoteDbImpl.class).in(Singleton.class);
if (!useTestBindings) {
install(ChangeNotesCache.module());
diff --git a/java/com/google/gerrit/server/notedb/StarredChangesUtilNoteDbImpl.java b/java/com/google/gerrit/server/notedb/StarredChangesUtilNoteDbImpl.java
new file mode 100644
index 0000000..7dc8f59
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/StarredChangesUtilNoteDbImpl.java
@@ -0,0 +1,332 @@
+// Copyright (C) 2015 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.
+
+package com.google.gerrit.server.notedb;
+
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.toSet;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.flogger.FluentLogger;
+import com.google.common.primitives.Ints;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.git.GitUpdateFailureException;
+import com.google.gerrit.git.LockFailureException;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.StarredChangesUtil;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.logging.Metadata;
+import com.google.gerrit.server.logging.TraceContext;
+import com.google.gerrit.server.logging.TraceContext.TraceTimer;
+import com.google.gerrit.server.update.context.RefUpdateContext;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.NullProgressMonitor;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+
+@Singleton
+public class StarredChangesUtilNoteDbImpl implements StarredChangesUtil {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private static final String DEFAULT_STAR_LABEL = "star";
+
+ private final GitRepositoryManager repoManager;
+ private final GitReferenceUpdated gitRefUpdated;
+ private final AllUsersName allUsers;
+ private final Provider<PersonIdent> serverIdent;
+
+ @Inject
+ StarredChangesUtilNoteDbImpl(
+ GitRepositoryManager repoManager,
+ GitReferenceUpdated gitRefUpdated,
+ AllUsersName allUsers,
+ @GerritPersonIdent Provider<PersonIdent> serverIdent) {
+ this.repoManager = repoManager;
+ this.gitRefUpdated = gitRefUpdated;
+ this.allUsers = allUsers;
+ this.serverIdent = serverIdent;
+ }
+
+ @Override
+ public boolean isStarred(Account.Id accountId, Change.Id changeId) {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ return getStarRef(repo, RefNames.refsStarredChanges(changeId, accountId)).isPresent();
+ } catch (IOException e) {
+ throw new StorageException(
+ String.format(
+ "Reading stars from change %d for account %d failed",
+ changeId.get(), accountId.get()),
+ e);
+ }
+ }
+
+ @Override
+ public void star(Account.Id accountId, Change.Id changeId) {
+ updateStar(accountId, changeId, true);
+ }
+
+ @Override
+ public void unstar(Account.Id accountId, Change.Id changeId) {
+ updateStar(accountId, changeId, false);
+ }
+
+ private void updateStar(Account.Id accountId, Change.Id changeId, boolean shouldAdd) {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ String refName = RefNames.refsStarredChanges(changeId, accountId);
+ if (shouldAdd) {
+ addRef(repo, refName, null);
+ } else {
+ Optional<Ref> ref = getStarRef(repo, refName);
+ if (ref.isPresent()) {
+ deleteRef(repo, refName, ref.get().getObjectId());
+ }
+ }
+ } catch (IOException e) {
+ throw new StorageException(
+ String.format("Star change %d for account %d failed", changeId.get(), accountId.get()),
+ e);
+ }
+ }
+
+ @Override
+ public Set<Change.Id> areStarred(
+ Repository allUsersRepo, List<Change.Id> changeIds, Account.Id caller) {
+ List<String> starRefs =
+ changeIds.stream()
+ .map(c -> RefNames.refsStarredChanges(c, caller))
+ .collect(Collectors.toList());
+ try {
+ return allUsersRepo.getRefDatabase().exactRef(starRefs.toArray(new String[0])).keySet()
+ .stream()
+ .map(r -> Change.Id.fromAllUsersRef(r))
+ .collect(Collectors.toSet());
+ } catch (IOException e) {
+ logger.atWarning().withCause(e).log(
+ "Failed getting starred changes for account %d within changes: %s",
+ caller.get(), Joiner.on(", ").join(changeIds));
+ return ImmutableSet.of();
+ }
+ }
+
+ @Override
+ public void unstarAllForChangeDeletion(Change.Id changeId) throws IOException {
+ try (Repository repo = repoManager.openRepository(allUsers);
+ RevWalk rw = new RevWalk(repo)) {
+ BatchRefUpdate batchUpdate = repo.getRefDatabase().newBatchUpdate();
+ batchUpdate.setAllowNonFastForwards(true);
+ batchUpdate.setRefLogIdent(serverIdent.get());
+ batchUpdate.setRefLogMessage("Unstar change " + changeId.get(), true);
+ for (Account.Id accountId : getStars(repo, changeId)) {
+ String refName = RefNames.refsStarredChanges(changeId, accountId);
+ Ref ref = repo.getRefDatabase().exactRef(refName);
+ if (ref != null) {
+ batchUpdate.addCommand(new ReceiveCommand(ref.getObjectId(), ObjectId.zeroId(), refName));
+ }
+ }
+ batchUpdate.execute(rw, NullProgressMonitor.INSTANCE);
+ for (ReceiveCommand command : batchUpdate.getCommands()) {
+ if (command.getResult() != ReceiveCommand.Result.OK) {
+ String message =
+ String.format(
+ "Unstar change %d failed, ref %s could not be deleted: %s",
+ changeId.get(), command.getRefName(), command.getResult());
+ if (command.getResult() == ReceiveCommand.Result.LOCK_FAILURE) {
+ throw new LockFailureException(message, batchUpdate);
+ }
+ throw new GitUpdateFailureException(message, batchUpdate);
+ }
+ }
+ }
+ }
+
+ @Override
+ public ImmutableMap<Account.Id, Ref> byChange(Change.Id changeId) {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ ImmutableMap.Builder<Account.Id, Ref> builder = ImmutableMap.builder();
+ for (Account.Id accountId : getStars(repo, changeId)) {
+ Optional<Ref> starRef = getStarRef(repo, RefNames.refsStarredChanges(changeId, accountId));
+ if (starRef.isPresent()) {
+ builder.put(accountId, starRef.get());
+ }
+ }
+ return builder.build();
+ } catch (IOException e) {
+ throw new StorageException(
+ String.format("Get accounts that starred change %d failed", changeId.get()), e);
+ }
+ }
+
+ @Override
+ public ImmutableSet<Change.Id> byAccountId(Account.Id accountId) {
+ return byAccountId(accountId, true);
+ }
+
+ @Override
+ public ImmutableSet<Change.Id> byAccountId(Account.Id accountId, boolean skipInvalidChanges) {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ ImmutableSet.Builder<Change.Id> builder = ImmutableSet.builder();
+ for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_STARRED_CHANGES)) {
+ Account.Id currentAccountId = Account.Id.fromRef(ref.getName());
+ // Skip all refs that don't correspond with accountId.
+ if (currentAccountId == null || !currentAccountId.equals(accountId)) {
+ continue;
+ }
+
+ // Skip invalid change ids.
+ Change.Id changeId = Change.Id.fromAllUsersRef(ref.getName());
+ if (skipInvalidChanges && changeId == null) {
+ continue;
+ }
+ builder.add(changeId);
+ }
+ return builder.build();
+ } catch (IOException e) {
+ throw new StorageException(
+ String.format("Get starred changes for account %d failed", accountId.get()), e);
+ }
+ }
+
+ private static Set<Account.Id> getStars(Repository allUsers, Change.Id changeId)
+ throws IOException {
+ String prefix = RefNames.refsStarredChangesPrefix(changeId);
+ RefDatabase refDb = allUsers.getRefDatabase();
+ return refDb.getRefsByPrefix(prefix).stream()
+ .map(r -> r.getName().substring(prefix.length()))
+ .map(refPart -> Ints.tryParse(refPart))
+ .filter(Objects::nonNull)
+ .map(id -> Account.id(id))
+ .collect(toSet());
+ }
+
+ private static Optional<Ref> getStarRef(Repository repo, @Nullable String refName)
+ throws IOException {
+ if (refName == null) {
+ return Optional.empty();
+ }
+ Ref ref = repo.exactRef(refName);
+ return Optional.ofNullable(ref);
+ }
+
+ private static ObjectId writeStarredRefContent(Repository repo) throws IOException {
+ try (ObjectInserter oi = repo.newObjectInserter()) {
+ ObjectId id = oi.insert(Constants.OBJ_BLOB, DEFAULT_STAR_LABEL.getBytes(UTF_8));
+ oi.flush();
+ return id;
+ }
+ }
+
+ private void addRef(Repository repo, String refName, ObjectId oldObjectId) throws IOException {
+ try (TraceTimer traceTimer =
+ TraceContext.newTimer(
+ "Add star ref",
+ Metadata.builder().noteDbRefName(refName).resourceCount(1).build());
+ RevWalk rw = new RevWalk(repo)) {
+ RefUpdate u = repo.updateRef(refName);
+ u.setExpectedOldObjectId(oldObjectId);
+ u.setForceUpdate(true);
+ u.setNewObjectId(writeStarredRefContent(repo));
+ u.setRefLogIdent(serverIdent.get());
+ u.setRefLogMessage("Add star ref", true);
+ try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ RefUpdate.Result result = u.update(rw);
+ switch (result) {
+ case NEW:
+ case FORCED:
+ case NO_CHANGE:
+ case FAST_FORWARD:
+ gitRefUpdated.fire(allUsers, u, null);
+ return;
+ case LOCK_FAILURE:
+ throw new LockFailureException(
+ String.format("Add star ref on ref %s failed", refName), u);
+ case IO_FAILURE:
+ case NOT_ATTEMPTED:
+ case REJECTED:
+ case REJECTED_CURRENT_BRANCH:
+ case RENAMED:
+ case REJECTED_MISSING_OBJECT:
+ case REJECTED_OTHER_REASON:
+ default:
+ throw new StorageException(
+ String.format("Add star ref on ref %s failed: %s", refName, result.name()));
+ }
+ }
+ }
+ }
+
+ private void deleteRef(Repository repo, String refName, ObjectId oldObjectId) throws IOException {
+ if (ObjectId.zeroId().equals(oldObjectId)) {
+ // ref doesn't exist
+ return;
+ }
+
+ try (TraceTimer traceTimer =
+ TraceContext.newTimer(
+ "Delete star ref", Metadata.builder().noteDbRefName(refName).build())) {
+ RefUpdate u = repo.updateRef(refName);
+ u.setForceUpdate(true);
+ u.setExpectedOldObjectId(oldObjectId);
+ u.setRefLogIdent(serverIdent.get());
+ u.setRefLogMessage("Unstar change", true);
+ try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ RefUpdate.Result result = u.delete();
+ switch (result) {
+ case FORCED:
+ gitRefUpdated.fire(allUsers, u, null);
+ return;
+ case LOCK_FAILURE:
+ throw new LockFailureException(String.format("Delete star ref %s failed", refName), u);
+ case NEW:
+ case NO_CHANGE:
+ case FAST_FORWARD:
+ case IO_FAILURE:
+ case NOT_ATTEMPTED:
+ case REJECTED:
+ case REJECTED_CURRENT_BRANCH:
+ case RENAMED:
+ case REJECTED_MISSING_OBJECT:
+ case REJECTED_OTHER_REASON:
+ default:
+ throw new StorageException(
+ String.format("Delete star ref %s failed: %s", refName, result.name()));
+ }
+ }
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 854897f6..638fea9 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -28,7 +28,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
-import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
@@ -68,7 +67,6 @@
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.StarredChangesUtil;
-import com.google.gerrit.server.StarredChangesUtil.StarRef;
import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.CommentThread;
import com.google.gerrit.server.change.CommentThreads;
@@ -397,9 +395,9 @@
*/
private Map<Account.Id, ObjectId> draftsByUser;
- private ImmutableListMultimap<Account.Id, String> stars;
- private StarsOf starsOf;
- private ImmutableMap<Account.Id, StarRef> starRefs;
+ private ImmutableList<Account.Id> stars;
+ private Account.Id starredBy;
+ private ImmutableMap<Account.Id, Ref> starRefs;
private ReviewerSet reviewers;
private ReviewerByEmailSet reviewersByEmail;
private ReviewerSet pendingReviewers;
@@ -1265,25 +1263,21 @@
return customKeyedValues;
}
- public ImmutableListMultimap<Account.Id, String> stars() {
+ public ImmutableList<Account.Id> stars() {
if (stars == null) {
if (!lazyload()) {
- return ImmutableListMultimap.of();
+ return ImmutableList.of();
}
- ImmutableListMultimap.Builder<Account.Id, String> b = ImmutableListMultimap.builder();
- for (Map.Entry<Account.Id, StarRef> e : starRefs().entrySet()) {
- b.putAll(e.getKey(), e.getValue().labels());
- }
- return b.build();
+ return starRefs().keySet().asList();
}
return stars;
}
- public void setStars(ListMultimap<Account.Id, String> stars) {
- this.stars = ImmutableListMultimap.copyOf(stars);
+ public void setStars(List<Account.Id> accountIds) {
+ this.stars = ImmutableList.copyOf(accountIds);
}
- private ImmutableMap<Account.Id, StarRef> starRefs() {
+ private ImmutableMap<Account.Id, Ref> starRefs() {
if (starRefs == null) {
if (!lazyload()) {
return ImmutableMap.of();
@@ -1293,23 +1287,25 @@
return starRefs;
}
- public Set<String> stars(Account.Id accountId) {
- if (starsOf != null) {
- if (!starsOf.accountId().equals(accountId)) {
- starsOf = null;
+ public boolean isStarred(Account.Id accountId) {
+ if (starredBy != null) {
+ if (!starredBy.equals(accountId)) {
+ starredBy = null;
}
}
- if (starsOf == null) {
- if (stars != null) {
- starsOf = StarsOf.create(accountId, stars.get(accountId));
+ if (starredBy == null) {
+ if (stars != null && stars.contains(accountId)) {
+ starredBy = accountId;
} else {
if (!lazyload()) {
- return ImmutableSet.of();
+ return false;
}
- starsOf = StarsOf.create(accountId, starredChangesUtil.getLabels(accountId, legacyId));
+ if (starredChangesUtil.isStarred(accountId, legacyId)) {
+ starredBy = accountId;
+ }
}
}
- return starsOf.stars();
+ return starredBy != null;
}
/**
@@ -1413,17 +1409,6 @@
public abstract Instant ts();
}
- @AutoValue
- abstract static class StarsOf {
- private static StarsOf create(Account.Id accountId, Iterable<String> stars) {
- return new AutoValue_ChangeData_StarsOf(accountId, ImmutableSortedSet.copyOf(stars));
- }
-
- public abstract Account.Id accountId();
-
- public abstract ImmutableSortedSet<String> stars();
- }
-
private Map<Account.Id, ObjectId> draftRefs() {
if (draftsByUser == null) {
if (!lazyload()) {
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteAccount.java b/java/com/google/gerrit/server/restapi/account/DeleteAccount.java
index c5cbd62..9b4c0a6 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteAccount.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteAccount.java
@@ -159,12 +159,10 @@
user.getUserName().ifPresent(sshKeyCache::evict);
}
- private void deleteStarredChanges(Account.Id accountId)
- throws StarredChangesUtil.IllegalLabelException {
+ private void deleteStarredChanges(Account.Id accountId) {
ImmutableSet<Change.Id> staredChanges = starredChangesUtil.byAccountId(accountId, false);
for (Change.Id change : staredChanges) {
- starredChangesUtil.star(
- self.get().getAccountId(), change, StarredChangesUtil.Operation.REMOVE);
+ starredChangesUtil.unstar(self.get().getAccountId(), change);
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/StarredChanges.java b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
index 173f24b..08b9bac 100644
--- a/java/com/google/gerrit/server/restapi/account/StarredChanges.java
+++ b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
@@ -20,10 +20,8 @@
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -36,8 +34,6 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil;
-import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException;
-import com.google.gerrit.server.StarredChangesUtil.MutuallyExclusiveLabelsException;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -72,9 +68,7 @@
throws RestApiException, PermissionBackendException, IOException {
IdentifiedUser user = parent.getUser();
ChangeResource change = changes.parse(TopLevelResource.INSTANCE, id);
- if (starredChangesUtil
- .getLabels(user.getAccountId(), change.getId())
- .contains(StarredChangesUtil.DEFAULT_LABEL)) {
+ if (starredChangesUtil.isStarred(user.getAccountId(), change.getId())) {
return new AccountResource.StarredChange(user, change);
}
throw new ResourceNotFoundException(id);
@@ -130,12 +124,7 @@
}
try {
- starredChangesUtil.star(
- self.get().getAccountId(), change.getId(), StarredChangesUtil.Operation.ADD);
- } catch (MutuallyExclusiveLabelsException e) {
- throw new ResourceConflictException(e.getMessage());
- } catch (IllegalLabelException e) {
- throw new BadRequestException(e.getMessage());
+ starredChangesUtil.star(self.get().getAccountId(), change.getId());
} catch (DuplicateKeyException e) {
return Response.none();
}
@@ -174,12 +163,11 @@
@Override
public Response<?> apply(AccountResource.StarredChange rsrc, Input in)
- throws AuthException, IOException, IllegalLabelException {
+ throws AuthException, IOException {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed remove starred change");
}
- starredChangesUtil.star(
- self.get().getAccountId(), rsrc.getChange().getId(), StarredChangesUtil.Operation.REMOVE);
+ starredChangesUtil.unstar(self.get().getAccountId(), rsrc.getChange().getId());
return Response.none();
}
}
diff --git a/java/com/google/gerrit/server/update/context/RefUpdateContext.java b/java/com/google/gerrit/server/update/context/RefUpdateContext.java
index 5d2cb35..a8c11df 100644
--- a/java/com/google/gerrit/server/update/context/RefUpdateContext.java
+++ b/java/com/google/gerrit/server/update/context/RefUpdateContext.java
@@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkState;
import com.google.common.collect.ImmutableList;
-import com.google.gerrit.common.UsedAt;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Optional;
@@ -131,11 +130,12 @@
checkArgument(
updateType != RefUpdateType.DIRECT_PUSH,
"openDirectPush method with justification must be used to open DIRECT_PUSH context.");
- return open(new RefUpdateContext(updateType));
+ return open(new RefUpdateContext(updateType, Optional.empty()));
}
- public static DirectPushRefUpdateContext openDirectPush(Optional<String> justification) {
- return open(new DirectPushRefUpdateContext(justification));
+ /** Opens a direct push context with an optional justification. */
+ public static RefUpdateContext openDirectPush(Optional<String> justification) {
+ return open(new RefUpdateContext(RefUpdateType.DIRECT_PUSH, justification));
}
/** Returns the list of opened contexts; the first element is the outermost context. */
@@ -149,13 +149,15 @@
}
private final RefUpdateType updateType;
+ private final Optional<String> justification;
- private RefUpdateContext(RefUpdateType updateType) {
+ private RefUpdateContext(RefUpdateType updateType, Optional<String> justification) {
this.updateType = updateType;
+ this.justification = justification;
}
- protected RefUpdateContext() {
- this(RefUpdateType.OTHER);
+ protected RefUpdateContext(Optional<String> justification) {
+ this(RefUpdateType.OTHER, justification);
}
protected static final Deque<RefUpdateContext> getCurrent() {
@@ -177,6 +179,11 @@
return updateType;
}
+ /** Returns the justification for the operation. */
+ public final Optional<String> getJustification() {
+ return justification;
+ }
+
/** Closes the current context. */
@Override
public void close() {
@@ -185,18 +192,4 @@
openedContexts.peekLast() == this, "The current context is different from this context.");
openedContexts.removeLast();
}
-
- public static class DirectPushRefUpdateContext extends RefUpdateContext {
- private final Optional<String> justification;
-
- private DirectPushRefUpdateContext(Optional<String> justification) {
- super(RefUpdateType.DIRECT_PUSH);
- this.justification = justification;
- }
-
- @UsedAt(UsedAt.Project.GOOGLE)
- public Optional<String> getJustification() {
- return justification;
- }
- }
}
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index 81a6443..7466c67 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -8,10 +8,12 @@
exclude = [
"AssertableExecutorService.java",
"TestActionRefUpdateContext.java",
+ "GerritJUnit.java",
],
),
visibility = ["//visibility:public"],
exports = [
+ ":gerrit-junit",
"//lib:junit",
"//lib/mockito",
],
@@ -61,6 +63,12 @@
)
java_library(
+ name = "gerrit-junit",
+ srcs = ["GerritJUnit.java"],
+ visibility = ["//visibility:public"],
+)
+
+java_library(
# This can't be part of gerrit-test-util because of https://github.com/google/guava/issues/2837
name = "assertable-executor",
testonly = True,
diff --git a/java/com/google/gerrit/testing/TestActionRefUpdateContext.java b/java/com/google/gerrit/testing/TestActionRefUpdateContext.java
index 23ec9aa..e3f6dcf 100644
--- a/java/com/google/gerrit/testing/TestActionRefUpdateContext.java
+++ b/java/com/google/gerrit/testing/TestActionRefUpdateContext.java
@@ -16,6 +16,7 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.server.update.context.RefUpdateContext;
+import java.util.Optional;
/**
* Marks ref updates as a test actions.
@@ -62,6 +63,10 @@
}
}
+ public TestActionRefUpdateContext() {
+ super(Optional.empty());
+ }
+
public interface CallableWithException<V, E extends Exception> {
V call() throws E;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 6360642..0fb7f00 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -32,7 +32,6 @@
import static com.google.gerrit.gpg.testing.TestKeys.validKeyWithExpiration;
import static com.google.gerrit.gpg.testing.TestKeys.validKeyWithSecondUserId;
import static com.google.gerrit.gpg.testing.TestKeys.validKeyWithoutExpiration;
-import static com.google.gerrit.server.StarredChangesUtil.DEFAULT_LABEL;
import static com.google.gerrit.server.account.AccountProperties.ACCOUNT;
import static com.google.gerrit.server.account.AccountProperties.ACCOUNT_CONFIG;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPGKEY;
@@ -798,7 +797,6 @@
gApi.accounts().self().starChange(triplet);
ChangeInfo change = info(triplet);
assertThat(change.starred).isTrue();
- assertThat(change.stars).contains(DEFAULT_LABEL);
refUpdateCounter.assertRefUpdateFor(
RefUpdateCounter.projectRef(
allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
@@ -806,7 +804,6 @@
gApi.accounts().self().unstarChange(triplet);
change = info(triplet);
assertThat(change.starred).isNull();
- assertThat(change.stars).isNull();
refUpdateCounter.assertRefUpdateFor(
RefUpdateCounter.projectRef(
allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index ec474f1..7fe7635 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -48,7 +48,6 @@
import static com.google.gerrit.extensions.client.ReviewerState.CC;
import static com.google.gerrit.extensions.client.ReviewerState.REMOVED;
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
-import static com.google.gerrit.server.StarredChangesUtil.DEFAULT_LABEL;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
import static com.google.gerrit.server.group.SystemGroupBackend.PROJECT_OWNERS;
@@ -4301,14 +4300,12 @@
gApi.accounts().self().starChange(triplet);
ChangeInfo change = info(triplet);
assertThat(change.starred).isTrue();
- assertThat(change.stars).contains(DEFAULT_LABEL);
// change was not re-indexed
changeIndexedCounter.assertReindexOf(change, 0);
gApi.accounts().self().unstarChange(triplet);
change = info(triplet);
assertThat(change.starred).isNull();
- assertThat(change.stars).isNull();
// change was not re-indexed
changeIndexedCounter.assertReindexOf(change, 0);
}
diff --git a/javatests/com/google/gerrit/acceptance/git/DirectPushRefUpdateContextIT.java b/javatests/com/google/gerrit/acceptance/git/DirectPushRefUpdateContextIT.java
index 5124e2b..e802604 100644
--- a/javatests/com/google/gerrit/acceptance/git/DirectPushRefUpdateContextIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/DirectPushRefUpdateContextIT.java
@@ -21,7 +21,6 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.server.update.context.RefUpdateContext;
-import com.google.gerrit.server.update.context.RefUpdateContext.DirectPushRefUpdateContext;
import com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType;
import com.google.gerrit.testing.RefUpdateContextCollector;
import java.util.Map.Entry;
@@ -49,7 +48,7 @@
assertThat(ctxts).hasSize(1);
RefUpdateContext ctx = ctxts.get(0);
assertThat(ctx.getUpdateType()).isEqualTo(RefUpdateType.DIRECT_PUSH);
- assertThat(((DirectPushRefUpdateContext) ctx).getJustification()).isEmpty();
+ assertThat(ctx.getJustification()).isEmpty();
}
@Test
@@ -69,7 +68,6 @@
assertThat(ctxts).hasSize(1);
RefUpdateContext ctx = ctxts.get(0);
assertThat(ctx.getUpdateType()).isEqualTo(RefUpdateType.DIRECT_PUSH);
- assertThat(((DirectPushRefUpdateContext) ctx).getJustification())
- .hasValue("test justification");
+ assertThat(ctx.getJustification()).hasValue("test justification");
}
}
diff --git a/javatests/com/google/gerrit/proto/BUILD b/javatests/com/google/gerrit/proto/BUILD
index 6b98b72..216ef94 100644
--- a/javatests/com/google/gerrit/proto/BUILD
+++ b/javatests/com/google/gerrit/proto/BUILD
@@ -5,7 +5,7 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/proto",
- "//java/com/google/gerrit/testing:gerrit-test-util",
+ "//java/com/google/gerrit/testing:gerrit-junit",
"//lib:protobuf",
"//lib/truth",
"//lib/truth:truth-proto-extension",
diff --git a/javatests/com/google/gerrit/server/update/context/RefUpdateContextTest.java b/javatests/com/google/gerrit/server/update/context/RefUpdateContextTest.java
index 02ab0e4..0646669 100644
--- a/javatests/com/google/gerrit/server/update/context/RefUpdateContextTest.java
+++ b/javatests/com/google/gerrit/server/update/context/RefUpdateContextTest.java
@@ -24,7 +24,6 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableList;
-import com.google.gerrit.server.update.context.RefUpdateContext.DirectPushRefUpdateContext;
import java.util.Optional;
import org.junit.After;
import org.junit.Test;
@@ -107,8 +106,7 @@
ImmutableList<RefUpdateContext> openedContexts = RefUpdateContext.getOpenedContexts();
assertThat(openedContexts).hasSize(1);
assertThat(openedContexts.get(0).getUpdateType()).isEqualTo(DIRECT_PUSH);
- assertThat(((DirectPushRefUpdateContext) openedContexts.get(0)).getJustification())
- .hasValue("Open in test");
+ assertThat(openedContexts.get(0).getJustification()).hasValue("Open in test");
assertThat(RefUpdateContext.hasOpen(DIRECT_PUSH)).isTrue();
assertThat(RefUpdateContext.hasOpen(INIT_REPO)).isFalse();
}
@@ -123,7 +121,7 @@
ImmutableList<RefUpdateContext> openedContexts = RefUpdateContext.getOpenedContexts();
assertThat(openedContexts).hasSize(1);
assertThat(openedContexts.get(0).getUpdateType()).isEqualTo(DIRECT_PUSH);
- assertThat(((DirectPushRefUpdateContext) openedContexts.get(0)).getJustification()).isEmpty();
+ assertThat(openedContexts.get(0).getJustification()).isEmpty();
assertThat(RefUpdateContext.hasOpen(DIRECT_PUSH)).isTrue();
assertThat(RefUpdateContext.hasOpen(INIT_REPO)).isFalse();
}
diff --git a/lib/BUILD b/lib/BUILD
index 7aa9a45..96a5325 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -273,19 +273,6 @@
data = ["//lib:LICENSE-flexmark"],
visibility = ["//visibility:public"],
exports = ["@flexmark-ext-gfm-strikethrough//jar"],
- runtime_deps = [
- ":flexmark-ext-gfm-tables",
- ],
-)
-
-java_library(
- name = "flexmark-ext-gfm-tables",
- data = ["//lib:LICENSE-flexmark"],
- visibility = ["//visibility:public"],
- exports = ["@flexmark-ext-gfm-tables//jar"],
- runtime_deps = [
- ":flexmark-ext-gfm-tasklist",
- ],
)
java_library(
@@ -383,29 +370,6 @@
data = ["//lib:LICENSE-flexmark"],
visibility = ["//visibility:public"],
exports = ["@flexmark-ext-yaml-front-matter//jar"],
- runtime_deps = [
- ":flexmark-formatter",
- ],
-)
-
-java_library(
- name = "flexmark-formatter",
- data = ["//lib:LICENSE-flexmark"],
- visibility = ["//visibility:public"],
- exports = ["@flexmark-formatter//jar"],
- runtime_deps = [
- ":flexmark-html-parser",
- ],
-)
-
-java_library(
- name = "flexmark-html-parser",
- data = ["//lib:LICENSE-flexmark"],
- visibility = ["//visibility:public"],
- exports = ["@flexmark-html-parser//jar"],
- runtime_deps = [
- ":flexmark-profile-pegdown",
- ],
)
java_library(
@@ -422,7 +386,15 @@
name = "flexmark-util",
data = ["//lib:LICENSE-flexmark"],
visibility = ["//visibility:public"],
- exports = ["@flexmark-util//jar"],
+ exports = [
+ "@flexmark-util-ast//jar",
+ "@flexmark-util-builder//jar",
+ "@flexmark-util-data//jar",
+ "@flexmark-util-html//jar",
+ "@flexmark-util-misc//jar",
+ "@flexmark-util-sequence//jar",
+ "@flexmark-util-visitor//jar",
+ ],
)
java_library(
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index 5b6019e..db69fcc 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -376,7 +376,6 @@
submitted?: Timestamp;
submitter?: AccountInfo;
starred?: boolean; // not set if false
- stars?: StarLabel[];
submit_type?: SubmitType;
mergeable?: boolean;
submittable?: boolean;
@@ -1033,7 +1032,6 @@
*/
export type SshdInfo = {};
-export type StarLabel = BrandType<string, '_startLabel'>;
// Timestamps are given in UTC and have the format
// "'yyyy-mm-dd hh:mm:ss.fffffffff'"
// where "'ffffffffff'" represents nanoseconds.
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 0b14f4f..93cb124 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
@@ -1259,12 +1259,9 @@
private goToEditFile() {
assertIsDefined(this.path, 'path');
- // TODO(taoalpha): add a shortcut for editing
- const cursorAddress = this.cursor?.getAddress();
- this.getChangeModel().navigateToEdit({
- path: this.path,
- lineNum: cursorAddress?.number,
- });
+ const lineNumber = this.cursor?.getTargetLineNumber();
+ const lineNum = typeof lineNumber === 'number' ? lineNumber : undefined;
+ this.getChangeModel().navigateToEdit({path: this.path, lineNum});
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index 6896ca8..c085953 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -806,9 +806,7 @@
element.patchNum = 1 as RevisionPatchSetNum;
element.basePatchNum = PARENT;
assertIsDefined(element.cursor);
- sinon
- .stub(element.cursor, 'getAddress')
- .returns({number: lineNumber, leftSide: false});
+ sinon.stub(element.cursor, 'getTargetLineNumber').returns(lineNumber);
await element.updateComplete;
const editBtn = queryAndAssert<GrButton>(
element,
@@ -818,7 +816,7 @@
editBtn.click();
assert.equal(navToEditStub.callCount, 1);
assert.deepEqual(navToEditStub.lastCall.args, [
- {path: 't.txt', lineNum: 42},
+ {path: 't.txt', lineNum: lineNumber},
]);
});
@@ -1428,9 +1426,6 @@
test('onLineSelected', () => {
const replaceStateStub = sinon.stub(history, 'replaceState');
assertIsDefined(element.cursor);
- sinon
- .stub(element.cursor, 'getAddress')
- .returns({number: 123, leftSide: false});
element.changeNum = 321 as NumericChangeId;
element.change = {
@@ -1450,9 +1445,6 @@
test('line selected on left side', () => {
const replaceStateStub = sinon.stub(history, 'replaceState');
assertIsDefined(element.cursor);
- sinon
- .stub(element.cursor, 'getAddress')
- .returns({number: 123, leftSide: true});
element.changeNum = 321 as NumericChangeId;
element.change = {
diff --git a/polygerrit-ui/app/embed/diff-old/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/embed/diff-old/gr-diff-cursor/gr-diff-cursor.ts
index 6a32afb..3d0e507 100644
--- a/polygerrit-ui/app/embed/diff-old/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/embed/diff-old/gr-diff-cursor/gr-diff-cursor.ts
@@ -375,6 +375,10 @@
}
}
+ getTargetLineNumber(): LineNumber | undefined {
+ return this.getAddress()?.number;
+ }
+
/**
* Get an object describing the location of the cursor. Such as
* {leftSide: false, number: 123} for line 123 of the revision, or
diff --git a/polygerrit-ui/app/embed/diff-old/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff-old/gr-diff/gr-diff.ts
index 0da3522..45a8784 100644
--- a/polygerrit-ui/app/embed/diff-old/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff-old/gr-diff/gr-diff.ts
@@ -25,6 +25,8 @@
getResponsiveMode,
isResponsive,
isNewDiff,
+ getDataFromCommentThreadEl,
+ GrDiffCommentThread,
} from '../../diff/gr-diff/gr-diff-utils';
import {BlameInfo, CommentRange, ImageInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
@@ -564,10 +566,12 @@
threadEl: GrDiffThreadElement
) {
hoverEl.addEventListener('mouseenter', () => {
- fire(threadEl, 'comment-thread-mouseenter', {});
+ const data = getDataFromCommentThreadEl(threadEl);
+ if (data) fire(threadEl, 'comment-thread-mouseenter', data);
});
hoverEl.addEventListener('mouseleave', () => {
- fire(threadEl, 'comment-thread-mouseleave', {});
+ const data = getDataFromCommentThreadEl(threadEl);
+ if (data) fire(threadEl, 'comment-thread-mouseleave', data);
});
}
@@ -1119,8 +1123,8 @@
'gr-diff': LitElement;
}
interface HTMLElementEventMap {
- 'comment-thread-mouseenter': CustomEvent<{}>;
- 'comment-thread-mouseleave': CustomEvent<{}>;
+ 'comment-thread-mouseenter': CustomEvent<GrDiffCommentThread>;
+ 'comment-thread-mouseleave': CustomEvent<GrDiffCommentThread>;
'loading-changed': ValueChangedEvent<boolean>;
'render-required': CustomEvent<{}>;
}
diff --git a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
index d936126..0962de7 100644
--- a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
@@ -17,7 +17,7 @@
let element: GrContextControlsSection;
setup(async () => {
- const diffModel = new DiffModel();
+ const diffModel = new DiffModel(document);
element = (
await fixture<DIProviderElement>(
wrapInProvider(
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
index 0ac7516..8ef2f82 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
@@ -3,7 +3,7 @@
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {html, LitElement, nothing, TemplateResult} from 'lit';
+import {html, LitElement, nothing, PropertyValues, TemplateResult} from 'lit';
import {property, state} from 'lit/decorators.js';
import {ifDefined} from 'lit/directives/if-defined.js';
import {createRef, Ref, ref} from 'lit/directives/ref.js';
@@ -20,10 +20,25 @@
import {assertIsDefined} from '../../../utils/common-util';
import {fire} from '../../../utils/event-util';
import {getBaseUrl} from '../../../utils/url-util';
+import {otherSide} from '../../../utils/diff-util';
import './gr-diff-text';
+import {
+ diffClasses,
+ GrDiffCommentThread,
+ isLongCommentRange,
+ isNewDiff,
+ isResponsive,
+} from '../gr-diff/gr-diff-utils';
+import {resolve} from '../../../models/dependency';
+import {diffModelToken} from '../gr-diff-model/gr-diff-model';
+import {when} from 'lit/directives/when.js';
+import {isDefined} from '../../../types/types';
+import {BehaviorSubject, combineLatest} from 'rxjs';
import '../../../elements/shared/gr-hovercard/gr-hovercard';
import {GrDiffLine} from '../gr-diff/gr-diff-line';
-import {diffClasses, isNewDiff, isResponsive} from '../gr-diff/gr-diff-utils';
+import {distinctUntilChanged, map} from 'rxjs/operators';
+import {deepEqual} from '../../../utils/deep-util';
+import {subscribe} from '../../../elements/lit/subscription-controller';
export class GrDiffRow extends LitElement {
contentLeftRef: Ref<LitElement> = createRef();
@@ -45,9 +60,13 @@
@property({type: Object})
left?: GrDiffLine;
+ private left$ = new BehaviorSubject<GrDiffLine | undefined>(undefined);
+
@property({type: Object})
right?: GrDiffLine;
+ private right$ = new BehaviorSubject<GrDiffLine | undefined>(undefined);
+
@property({type: Object})
blameInfo?: BlameInfo;
@@ -78,8 +97,11 @@
* running such tests the render() method has to wrap the DOM in a proper
* <table> element.
*/
- @state()
- addTableWrapperForTesting = false;
+ @state() addTableWrapperForTesting = false;
+
+ @state() leftComments: GrDiffCommentThread[] = [];
+
+ @state() rightComments: GrDiffCommentThread[] = [];
/**
* Keeps track of whether diff layers have already been applied to the diff
@@ -93,6 +115,46 @@
*/
private layersApplied = false;
+ private readonly getDiffModel = resolve(this, diffModelToken);
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () =>
+ combineLatest([this.left$, this.getDiffModel().comments$]).pipe(
+ map(([left, comments]) =>
+ comments.filter(
+ c =>
+ c.line === left?.lineNumber(Side.LEFT) && c.side === Side.LEFT
+ )
+ ),
+ distinctUntilChanged(deepEqual)
+ ),
+ leftComments => (this.leftComments = leftComments)
+ );
+ subscribe(
+ this,
+ () =>
+ combineLatest([this.right$, this.getDiffModel().comments$]).pipe(
+ map(([right, comments]) =>
+ comments.filter(
+ c =>
+ c.line === right?.lineNumber(Side.RIGHT) &&
+ c.side === Side.RIGHT
+ )
+ ),
+ distinctUntilChanged(deepEqual)
+ ),
+ rightComments => (this.rightComments = rightComments)
+ );
+ }
+
+ override willUpdate(changedProperties: PropertyValues) {
+ if (changedProperties.has('left')) this.left$.next(this.left);
+ if (changedProperties.has('right')) this.right$.next(this.right);
+ }
+
/**
* The browser API for handling selection does not (yet) work for selection
* across multiple shadow DOM elements. So we are rendering gr-diff components
@@ -296,12 +358,13 @@
data-value=${lineNumber}
aria-label=${ifDefined(
this.computeLineNumberAriaLabel(line, lineNumber)
- )}
+ )}
+ @click=${() => this.getDiffModel().createCommentOnLine(lineNumber, side)}
@mouseenter=${() =>
fire(this, 'line-mouse-enter', {lineNum: lineNumber, side})}
@mouseleave=${() =>
fire(this, 'line-mouse-leave', {lineNum: lineNumber, side})}
- >${lineNumber === FILE ? 'File' : lineNumber.toString()}</button>
+ >${lineNumber === FILE ? 'FILE' : lineNumber.toString()}</button>
`;
}
@@ -352,6 +415,11 @@
<td
${ref(this.contentCellRef(side))}
class=${diffClasses(...extras)}
+ @click=${() => {
+ if (lineNumber) {
+ this.getDiffModel().selectLine(lineNumber, side);
+ }
+ }}
@mouseenter=${() => {
if (lineNumber)
fire(this, 'line-mouse-enter', {lineNum: lineNumber, side});
@@ -360,7 +428,7 @@
if (lineNumber)
fire(this, 'line-mouse-leave', {lineNum: lineNumber, side});
}}
- >${this.renderText(side)}${this.renderThreadGroup(side)}</td>
+ >${this.renderText(side)}${this.renderLostMessage(side)}${this.renderThreadGroup(side)}</td>
`;
}
@@ -381,21 +449,53 @@
return html`<td class=${diffClasses(...extras)}>${sign}</td>`;
}
+ private renderLostMessage(side: Side) {
+ if (this.lineNumber(side) !== LOST) return nothing;
+ if (this.getComments(side).length === 0) return nothing;
+ // .content has `white-space: pre`, so prettier must not add spaces.
+ // prettier-ignore
+ return html`<div class="lost-message"
+ ><gr-icon icon="info"></gr-icon
+ ><span>Original comment position not found in this patchset</span
+ ></div>`;
+ }
+
private renderThreadGroup(side: Side) {
- const lineNumber = this.lineNumber(side);
- if (!lineNumber) return nothing;
+ if (!this.lineNumber(side)) return nothing;
+
+ if (
+ this.getComments(side).length === 0 &&
+ (!this.unifiedDiff || this.getComments(otherSide(side)).length === 0)
+ ) {
+ return nothing;
+ }
return html`<div class="thread-group" data-side=${side}>
- <slot name="${side}-${lineNumber}"></slot>
- ${this.renderSecondSlot()}
+ ${this.renderSlot(side)}
+ ${when(this.unifiedDiff, () => this.renderSlot(otherSide(side)))}
</div>`;
}
- private renderSecondSlot() {
- if (!this.unifiedDiff) return nothing;
- if (this.line(Side.LEFT)?.type !== GrDiffLineType.BOTH) return nothing;
- return html`<slot
- name="${Side.LEFT}-${this.lineNumber(Side.LEFT)}"
- ></slot>`;
+ private renderSlot(side: Side) {
+ const line = this.lineNumber(side);
+ if (!line) return nothing;
+ if (this.getComments(side).length === 0) return nothing;
+ return html`
+ ${this.renderRangedCommentHints(side)}
+ <slot name="${side}-${line}"></slot>
+ `;
+ }
+
+ private renderRangedCommentHints(side: Side) {
+ const ranges = this.getComments(side)
+ .map(c => c.range)
+ .filter(isDefined)
+ .filter(isLongCommentRange);
+ return ranges.map(
+ range =>
+ html`
+ <gr-ranged-comment-hint .range=${range}></gr-ranged-comment-hint>
+ `
+ );
}
private contentRef(side: Side) {
@@ -414,14 +514,18 @@
: this.lineNumberRightRef;
}
- private lineNumber(side: Side) {
+ lineNumber(side: Side) {
return this.line(side)?.lineNumber(side);
}
- private line(side: Side) {
+ line(side: Side) {
return side === Side.LEFT ? this.left : this.right;
}
+ private getComments(side: Side) {
+ return side === Side.LEFT ? this.leftComments : this.rightComments;
+ }
+
private getType(side?: Side): string | undefined {
if (this.unifiedDiff) return undefined;
if (side === Side.LEFT) return this.left?.type;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
index 42d30aa..61ea055 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
@@ -58,8 +58,6 @@
>
<gr-diff-text> lorem ipsum </gr-diff-text>
</div>
- <div class="thread-group" data-side="left">
- <slot name="left-1"> </slot>
</div>
</td>
<td class="gr-diff lineNum right" data-value="1">
@@ -82,9 +80,6 @@
>
<gr-diff-text> lorem ipsum </gr-diff-text>
</div>
- <div class="thread-group" data-side="right">
- <slot name="right-1"> </slot>
- </div>
</td>
</tr>
<slot name="post-left-line-1"></slot>
@@ -143,10 +138,6 @@
>
<gr-diff-text> lorem ipsum </gr-diff-text>
</div>
- <div class="thread-group" data-side="right">
- <slot name="right-1"> </slot>
- <slot name="left-1"> </slot>
- </div>
</td>
</tr>
<slot name="post-left-line-1"></slot>
@@ -201,9 +192,6 @@
>
<gr-diff-text> lorem ipsum </gr-diff-text>
</div>
- <div class="thread-group" data-side="right">
- <slot name="right-1"> </slot>
- </div>
</td>
<slot name="post-right-line-1"></slot>
</tr>
@@ -252,9 +240,6 @@
>
<gr-diff-text> lorem ipsum </gr-diff-text>
</div>
- <div class="thread-group" data-side="left">
- <slot name="left-1"> </slot>
- </div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
index d23c9c5..b184be3 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
@@ -161,9 +161,6 @@
>
<gr-diff-text>asdf</gr-diff-text>
</div>
- <div class="thread-group" data-side="left">
- <slot name="left-1"> </slot>
- </div>
</td>
<td class="gr-diff lineNum right" data-value="1">
<button
@@ -185,9 +182,6 @@
>
<gr-diff-text>asdf </gr-diff-text>
</div>
- <div class="thread-group" data-side="right">
- <slot name="right-1"> </slot>
- </div>
</td>
</tr>
<tr
@@ -218,9 +212,6 @@
>
<gr-diff-text> qwer</gr-diff-text>
</div>
- <div class="thread-group" data-side="left">
- <slot name="left-1"> </slot>
- </div>
</td>
<td class="gr-diff lineNum right" data-value="1">
<button
@@ -242,9 +233,6 @@
>
<gr-diff-text>qwer </gr-diff-text>
</div>
- <div class="thread-group" data-side="right">
- <slot name="right-1"> </slot>
- </div>
</td>
</tr>
<tr
@@ -275,9 +263,6 @@
>
<gr-diff-text>zxcv </gr-diff-text>
</div>
- <div class="thread-group" data-side="left">
- <slot name="left-1"> </slot>
- </div>
</td>
<td class="gr-diff lineNum right" data-value="1">
<button
@@ -299,9 +284,6 @@
>
<gr-diff-text>zxcv </gr-diff-text>
</div>
- <div class="thread-group" data-side="right">
- <slot name="right-1"> </slot>
- </div>
</td>
</tr>
</tbody>
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
index 6a32afb..ed6e9da 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -18,20 +18,13 @@
GrCursorManager,
isTargetable,
} from '../../../elements/shared/gr-cursor-manager/gr-cursor-manager';
-import {GrDiffGroupType} from '../gr-diff/gr-diff-group';
import {GrDiff} from '../gr-diff/gr-diff';
import {fire} from '../../../utils/event-util';
-
-type GrDiffRowType = GrDiffLineType | GrDiffGroupType;
+import {GrDiffRow} from '../gr-diff-builder/gr-diff-row';
const LEFT_SIDE_CLASS = 'target-side-left';
const RIGHT_SIDE_CLASS = 'target-side-right';
-interface Address {
- leftSide: boolean;
- number: number;
-}
-
/**
* From <tr> diff row go up to <tbody> diff chunk.
*
@@ -70,17 +63,13 @@
if (this.sideInternal === side) {
return;
}
- if (this.sideInternal && this.diffRow) {
- this.fireCursorMoved(
- 'line-cursor-moved-out',
- this.diffRow,
- this.sideInternal
- );
+ if (this.diffRowTR) {
+ this.fireCursorMoved('line-cursor-moved-out');
}
this.sideInternal = side;
this.updateSideClass();
- if (this.diffRow) {
- this.fireCursorMoved('line-cursor-moved-in', this.diffRow, this.side);
+ if (this.diffRowTR) {
+ this.fireCursorMoved('line-cursor-moved-in');
}
}
@@ -90,28 +79,30 @@
private sideInternal = Side.RIGHT;
- set diffRow(diffRow: HTMLElement | undefined) {
- if (this.diffRowInternal) {
- this.diffRowInternal.classList.remove(LEFT_SIDE_CLASS, RIGHT_SIDE_CLASS);
- this.fireCursorMoved(
- 'line-cursor-moved-out',
- this.diffRowInternal,
- this.side
+ set diffRowTR(diffRowTR: HTMLTableRowElement | undefined) {
+ if (this.diffRowTRInternal) {
+ this.diffRowTRInternal.classList.remove(
+ LEFT_SIDE_CLASS,
+ RIGHT_SIDE_CLASS
);
+ this.fireCursorMoved('line-cursor-moved-out');
}
- this.diffRowInternal = diffRow;
+ this.diffRowTRInternal = diffRowTR;
this.updateSideClass();
- if (this.diffRow) {
- this.fireCursorMoved('line-cursor-moved-in', this.diffRow, this.side);
+ if (this.diffRowTR) {
+ this.fireCursorMoved('line-cursor-moved-in');
}
}
- get diffRow(): HTMLElement | undefined {
- return this.diffRowInternal;
+ /**
+ * This is the current target of the diff cursor.
+ */
+ get diffRowTR(): HTMLTableRowElement | undefined {
+ return this.diffRowTRInternal;
}
- private diffRowInternal?: HTMLElement;
+ private diffRowTRInternal?: HTMLTableRowElement;
private diffs: GrDiffCursorable[] = [];
@@ -135,16 +126,16 @@
this.cursorManager.scrollMode = ScrollMode.KEEP_VISIBLE;
this.cursorManager.focusOnMove = true;
- window.addEventListener('scroll', this._boundHandleWindowScroll);
+ window.addEventListener('scroll', this.boundHandleWindowScroll);
this.targetSubscription = this.cursorManager.target$.subscribe(target => {
- this.diffRow = target || undefined;
+ this.diffRowTR = (target ?? undefined) as HTMLTableRowElement | undefined;
});
}
dispose() {
this.cursorManager.unsetCursor();
if (this.targetSubscription) this.targetSubscription.unsubscribe();
- window.removeEventListener('scroll', this._boundHandleWindowScroll);
+ window.removeEventListener('scroll', this.boundHandleWindowScroll);
}
// Don't remove - used by clients embedding gr-diff outside of Gerrit.
@@ -159,22 +150,22 @@
moveLeft() {
this.side = Side.LEFT;
- if (this._isTargetBlank()) {
+ if (this.isTargetBlank()) {
this.moveUp();
}
}
moveRight() {
this.side = Side.RIGHT;
- if (this._isTargetBlank()) {
+ if (this.isTargetBlank()) {
this.moveUp();
}
}
moveDown() {
- if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
+ if (this.getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
return this.cursorManager.next({
- filter: (row: Element) => this._rowHasSide(row),
+ filter: (row: Element) => this.rowHasSide(row),
});
} else {
return this.cursorManager.next();
@@ -182,9 +173,9 @@
}
moveUp() {
- if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
+ if (this.getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
return this.cursorManager.previous({
- filter: (row: Element) => this._rowHasSide(row),
+ filter: (row: Element) => this.rowHasSide(row),
});
} else {
return this.cursorManager.previous();
@@ -192,9 +183,9 @@
}
moveToVisibleArea() {
- if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
+ if (this.getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
this.cursorManager.moveToVisibleArea((row: Element) =>
- this._rowHasSide(row)
+ this.rowHasSide(row)
);
} else {
this.cursorManager.moveToVisibleArea();
@@ -203,19 +194,19 @@
moveToNextChunk(clipToTop?: boolean): CursorMoveResult {
const result = this.cursorManager.next({
- filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
+ filter: (row: HTMLElement) => this.isFirstRowOfChunk(row),
getTargetHeight: target => fromRowToChunk(target)?.scrollHeight || 0,
clipToTop,
});
- this._fixSide();
+ this.fixSide();
return result;
}
moveToPreviousChunk(): CursorMoveResult {
const result = this.cursorManager.previous({
- filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
+ filter: (row: HTMLElement) => this.isFirstRowOfChunk(row),
});
- this._fixSide();
+ this.fixSide();
return result;
}
@@ -224,17 +215,17 @@
return CursorMoveResult.CLIPPED;
}
const result = this.cursorManager.next({
- filter: (row: HTMLElement) => this._rowHasThread(row),
+ filter: (row: HTMLElement) => this.rowHasThread(row),
});
- this._fixSide();
+ this.fixSide();
return result;
}
moveToPreviousCommentThread(): CursorMoveResult {
const result = this.cursorManager.previous({
- filter: (row: HTMLElement) => this._rowHasThread(row),
+ filter: (row: HTMLElement) => this.rowHasThread(row),
});
- this._fixSide();
+ this.fixSide();
return result;
}
@@ -244,7 +235,7 @@
path?: string,
intentionalMove?: boolean
) {
- const row = this._findRowByNumberAndFile(number, side, path);
+ const row = this.findRowByNumberAndFile(number, side, path);
if (row) {
this.side = side;
this.cursorManager.setCursor(row, undefined, intentionalMove);
@@ -252,47 +243,50 @@
}
/**
- * Get the line number element targeted by the cursor row and side.
+ * The target of the diff cursor is always a <tr> element. That is the first
+ * direct child of a <gr-diff-row> element. We typically want to retrieve
+ * the `GrDiffRow`, because it supplies methods that we can use without
+ * making further assumptions about the internal DOM structure.
*/
- getTargetLineElement(): HTMLElement | null {
- let lineElSelector = '.lineNum';
-
- if (!this.diffRow) {
- return null;
+ getTargetDiffRow(): GrDiffRow | undefined {
+ let el: HTMLElement | undefined = this.diffRowTR;
+ while (el) {
+ if (el.tagName === 'GR-DIFF-ROW') return el as GrDiffRow;
+ el = el.parentElement ?? undefined;
}
-
- if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
- lineElSelector += this.side === Side.LEFT ? '.left' : '.right';
- }
-
- return this.diffRow.querySelector(lineElSelector);
+ return undefined;
}
- getTargetDiffElement(): GrDiff | null {
- if (!this.diffRow) return null;
+ getTargetLineNumber(): LineNumber | undefined {
+ const diffRow = this.getTargetDiffRow();
+ return diffRow?.lineNumber(this.side);
+ }
- const hostOwner = this.diffRow.getRootNode() as ShadowRoot;
+ getTargetDiffElement(): GrDiff | undefined {
+ if (!this.diffRowTR) return undefined;
+
+ const hostOwner = this.diffRowTR.getRootNode() as ShadowRoot;
if (hostOwner?.host?.tagName === 'GR-DIFF') {
return hostOwner.host as GrDiff;
}
- return null;
+ return undefined;
}
moveToFirstChunk() {
this.cursorManager.moveToStart();
- if (this.diffRow && !this._isFirstRowOfChunk(this.diffRow)) {
+ if (this.diffRowTR && !this.isFirstRowOfChunk(this.diffRowTR)) {
this.moveToNextChunk(true);
} else {
- this._fixSide();
+ this.fixSide();
}
}
moveToLastChunk() {
this.cursorManager.moveToEnd();
- if (this.diffRow && !this._isFirstRowOfChunk(this.diffRow)) {
+ if (this.diffRowTR && !this.isFirstRowOfChunk(this.diffRowTR)) {
this.moveToPreviousChunk();
} else {
- this._fixSide();
+ this.fixSide();
}
}
@@ -306,8 +300,8 @@
* reset the scroll behavior, use reInitAndUpdateStops() instead.
*/
reInitCursor() {
- this._updateStops();
- if (!this.diffRow) {
+ this.updateStops();
+ if (!this.diffRowTR) {
// does not scroll during init unless requested
this.cursorManager.scrollMode = this.initialLineNumber
? ScrollMode.KEEP_VISIBLE
@@ -326,7 +320,7 @@
this.cursorManager.scrollMode = ScrollMode.KEEP_VISIBLE;
}
- private _boundHandleWindowScroll = () => {
+ private boundHandleWindowScroll = () => {
if (this.preventAutoScrollOnManualScroll) {
this.cursorManager.scrollMode = ScrollMode.NEVER;
this.cursorManager.focusOnMove = false;
@@ -336,25 +330,25 @@
reInitAndUpdateStops() {
this.resetScrollMode();
- this._updateStops();
+ this.updateStops();
}
private boundHandleDiffLoadingChanged = () => {
- this._updateStops();
+ this.updateStops();
};
- private _boundHandleDiffRenderStart = () => {
+ private boundHandleDiffRenderStart = () => {
this.preventAutoScrollOnManualScroll = true;
};
- private _boundHandleDiffRenderContent = () => {
- this._updateStops();
+ private boundHandleDiffRenderContent = () => {
+ this.updateStops();
// When done rendering, turn focus on move and automatic scrolling back on
this.cursorManager.focusOnMove = true;
this.preventAutoScrollOnManualScroll = false;
};
- private _boundHandleDiffLineSelected = (
+ private boundHandleDiffLineSelected = (
e: CustomEvent<LineSelectedEventDetail>
) => {
this.moveToLineNumber(e.detail.number, e.detail.side, e.detail.path);
@@ -367,73 +361,34 @@
if (diffWithRangeSelected) {
diffWithRangeSelected.createRangeComment();
} else {
- const line = this.getTargetLineElement();
+ const diffRow = this.getTargetDiffRow();
+ const lineNumber = diffRow?.lineNumber(this.side);
const diff = this.getTargetDiffElement();
- if (diff && line) {
- diff.addDraftAtLine(line);
+ if (diff && lineNumber) {
+ diff.addDraftAtLine(lineNumber, this.side);
}
}
}
- /**
- * Get an object describing the location of the cursor. Such as
- * {leftSide: false, number: 123} for line 123 of the revision, or
- * {leftSide: true, number: 321} for line 321 of the base patch.
- * Returns null if an address is not available.
- */
- getAddress(): Address | null {
- if (!this.diffRow) {
- return null;
- }
- // Get the line-number cell targeted by the cursor. If the mode is unified
- // then prefer the revision cell if available.
- return this.getAddressFor(this.diffRow, this.side);
- }
-
- private getAddressFor(diffRow: HTMLElement, side: Side): Address | null {
- let cell;
- if (this._getViewMode() === DiffViewMode.UNIFIED) {
- cell = diffRow.querySelector('.lineNum.right');
- if (!cell) {
- cell = diffRow.querySelector('.lineNum.left');
- }
- } else {
- cell = diffRow.querySelector('.lineNum.' + side);
- }
- if (!cell) {
+ private getViewMode() {
+ if (!this.diffRowTR) {
return null;
}
- const number = cell.getAttribute('data-value');
- if (!number || number === 'FILE') {
- return null;
- }
-
- return {
- leftSide: cell.matches('.left'),
- number: Number(number),
- };
- }
-
- _getViewMode() {
- if (!this.diffRow) {
- return null;
- }
-
- if (this.diffRow.classList.contains('side-by-side')) {
+ if (this.diffRowTR.classList.contains('side-by-side')) {
return DiffViewMode.SIDE_BY_SIDE;
} else {
return DiffViewMode.UNIFIED;
}
}
- _rowHasSide(row: Element) {
+ private rowHasSide(row: Element) {
const selector =
(this.side === Side.LEFT ? '.left' : '.right') + ' + .content';
return !!row.querySelector(selector);
}
- _isFirstRowOfChunk(row: HTMLElement) {
+ private isFirstRowOfChunk(row: HTMLElement) {
const chunk = fromRowToChunk(row);
if (!chunk) return false;
@@ -444,7 +399,7 @@
return firstRow === row;
}
- _rowHasThread(row: HTMLElement): boolean {
+ private rowHasThread(row: HTMLElement): boolean {
const slots = [
...row.querySelectorAll<HTMLSlotElement>('.thread-group > slot'),
];
@@ -455,70 +410,38 @@
* If we jumped to a row where there is no content on the current side then
* switch to the alternate side.
*/
- _fixSide() {
+ private fixSide() {
if (
- this._getViewMode() === DiffViewMode.SIDE_BY_SIDE &&
- this._isTargetBlank()
+ this.getViewMode() === DiffViewMode.SIDE_BY_SIDE &&
+ this.isTargetBlank()
) {
this.side = this.side === Side.LEFT ? Side.RIGHT : Side.LEFT;
}
}
- _isTargetBlank() {
- if (!this.diffRow) {
- return false;
- }
-
- const actions = this._getActionsForRow();
- return (
- (this.side === Side.LEFT && !actions.left) ||
- (this.side === Side.RIGHT && !actions.right)
- );
+ private isTargetBlank() {
+ const line = this.getTargetDiffRow()?.line(this.side);
+ return line?.type === GrDiffLineType.BLANK;
}
private fireCursorMoved(
- event: 'line-cursor-moved-out' | 'line-cursor-moved-in',
- row: HTMLElement,
- side: Side
+ event: 'line-cursor-moved-out' | 'line-cursor-moved-in'
) {
- const address = this.getAddressFor(row, side);
- if (address) {
- const {leftSide, number} = address;
- fire(row, event, {
- lineNum: number,
- side: leftSide ? Side.LEFT : Side.RIGHT,
- });
- }
+ const lineNum = this.getTargetLineNumber();
+ if (!lineNum) return;
+ fire(this.diffRowTR, event, {lineNum, side: this.side});
}
private updateSideClass() {
- if (!this.diffRow) {
+ if (!this.diffRowTR) {
return;
}
- toggleClass(this.diffRow, LEFT_SIDE_CLASS, this.side === Side.LEFT);
- toggleClass(this.diffRow, RIGHT_SIDE_CLASS, this.side === Side.RIGHT);
+ toggleClass(this.diffRowTR, LEFT_SIDE_CLASS, this.side === Side.LEFT);
+ toggleClass(this.diffRowTR, RIGHT_SIDE_CLASS, this.side === Side.RIGHT);
}
- _isActionType(type: GrDiffRowType) {
- return (
- type !== GrDiffLineType.BLANK && type !== GrDiffGroupType.CONTEXT_CONTROL
- );
- }
-
- _getActionsForRow() {
- const actions = {left: false, right: false};
- if (this.diffRow) {
- actions.left = this._isActionType(
- this.diffRow.getAttribute('left-type') as GrDiffRowType
- );
- actions.right = this._isActionType(
- this.diffRow.getAttribute('right-type') as GrDiffRowType
- );
- }
- return actions;
- }
-
- _updateStops() {
+ // visible for testing
+ updateStops() {
this.cursorManager.stops = this.diffs.reduce(
(stops: Stop[], diff) => stops.concat(diff.getCursorStops()),
[]
@@ -534,7 +457,7 @@
this.addEventListeners(diff);
}
this.diffs.push(...diffs);
- this._updateStops();
+ this.updateStops();
}
unregisterDiff(diff: GrDiffCursorable) {
@@ -551,15 +474,12 @@
'loading-changed',
this.boundHandleDiffLoadingChanged
);
- diff.removeEventListener('render-start', this._boundHandleDiffRenderStart);
+ diff.removeEventListener('render-start', this.boundHandleDiffRenderStart);
diff.removeEventListener(
'render-content',
- this._boundHandleDiffRenderContent
+ this.boundHandleDiffRenderContent
);
- diff.removeEventListener(
- 'line-selected',
- this._boundHandleDiffLineSelected
- );
+ diff.removeEventListener('line-selected', this.boundHandleDiffLineSelected);
}
private addEventListeners(diff: GrDiffCursorable) {
@@ -567,12 +487,13 @@
'loading-changed',
this.boundHandleDiffLoadingChanged
);
- diff.addEventListener('render-start', this._boundHandleDiffRenderStart);
- diff.addEventListener('render-content', this._boundHandleDiffRenderContent);
- diff.addEventListener('line-selected', this._boundHandleDiffLineSelected);
+ diff.addEventListener('render-start', this.boundHandleDiffRenderStart);
+ diff.addEventListener('render-content', this.boundHandleDiffRenderContent);
+ diff.addEventListener('line-selected', this.boundHandleDiffLineSelected);
}
- _findRowByNumberAndFile(
+ // visible for testing
+ findRowByNumberAndFile(
targetNumber: LineNumber,
side: Side,
path?: string
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
index d8406c0..70fece3 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
@@ -17,7 +17,7 @@
import {createDefaultDiffPrefs} from '../../../constants/constants';
import {GrDiffCursor} from './gr-diff-cursor';
import {waitForEventOnce} from '../../../utils/event-util';
-import {DiffInfo, DiffViewMode, Side} from '../../../api/diff';
+import {DiffInfo, DiffViewMode, FILE, Side} from '../../../api/diff';
import {GrDiff} from '../gr-diff/gr-diff';
import {assertIsDefined} from '../../../utils/common-util';
@@ -27,12 +27,13 @@
setup(async () => {
diffElement = await fixture(html`<gr-diff></gr-diff>`);
+ diffElement.path = 'some/path.ts';
cursor = new GrDiffCursor();
cursor.replaceDiffs([diffElement]);
const promise = mockPromise();
const setupDone = () => {
- cursor._updateStops();
+ cursor.updateStops();
cursor.moveToFirstChunk();
diffElement.removeEventListener('render', setupDone);
promise.resolve();
@@ -48,23 +49,23 @@
});
test('diff cursor functionality (side-by-side)', () => {
- assert.isOk(cursor.diffRow);
+ assert.isOk(cursor.diffRowTR);
const deltaRows = queryAll<HTMLTableRowElement>(
diffElement,
'.section.delta tr.diff-row'
);
- assert.equal(cursor.diffRow, deltaRows[0]);
+ assert.equal(cursor.diffRowTR, deltaRows[0]);
cursor.moveDown();
- assert.notEqual(cursor.diffRow, deltaRows[0]);
- assert.equal(cursor.diffRow, deltaRows[1]);
+ assert.notEqual(cursor.diffRowTR, deltaRows[0]);
+ assert.equal(cursor.diffRowTR, deltaRows[1]);
cursor.moveUp();
- assert.notEqual(cursor.diffRow, deltaRows[1]);
- assert.equal(cursor.diffRow, deltaRows[0]);
+ assert.notEqual(cursor.diffRowTR, deltaRows[1]);
+ assert.equal(cursor.diffRowTR, deltaRows[0]);
});
test('moveToFirstChunk', async () => {
@@ -104,7 +105,7 @@
await waitForEventOnce(diffElement, 'render');
await waitForEventOnce(diffElement, 'render');
- cursor._updateStops();
+ cursor.updateStops();
const chunks = [
...queryAll(diffElement, '.section.delta'),
@@ -118,19 +119,19 @@
// Verify it works on fresh diff.
cursor.moveToFirstChunk();
- assert.ok(cursor.diffRow);
- assert.equal(cursor.diffRow, rows[0]);
+ assert.ok(cursor.diffRowTR);
+ assert.equal(cursor.diffRowTR, rows[0]);
assert.equal(cursor.side, Side.RIGHT);
// Verify it works from other cursor positions.
cursor.moveToNextChunk();
- assert.ok(cursor.diffRow);
- assert.equal(cursor.diffRow, rows[1]);
+ assert.ok(cursor.diffRowTR);
+ assert.equal(cursor.diffRowTR, rows[1]);
assert.equal(cursor.side, Side.LEFT);
cursor.moveToFirstChunk();
- assert.ok(cursor.diffRow);
- assert.equal(cursor.diffRow, rows[0]);
+ assert.ok(cursor.diffRowTR);
+ assert.equal(cursor.diffRowTR, rows[0]);
assert.equal(cursor.side, Side.RIGHT);
});
@@ -165,7 +166,7 @@
await waitForEventOnce(diffElement, 'render');
await waitForEventOnce(diffElement, 'render');
- cursor._updateStops();
+ cursor.updateStops();
const chunks = [
...queryAll(diffElement, '.section.delta'),
@@ -179,19 +180,19 @@
// Verify it works on fresh diff.
cursor.moveToLastChunk();
- assert.ok(cursor.diffRow);
- assert.equal(cursor.diffRow, rows[1]);
+ assert.ok(cursor.diffRowTR);
+ assert.equal(cursor.diffRowTR, rows[1]);
assert.equal(cursor.side, Side.RIGHT);
// Verify it works from other cursor positions.
cursor.moveToPreviousChunk();
- assert.ok(cursor.diffRow);
- assert.equal(cursor.diffRow, rows[0]);
+ assert.ok(cursor.diffRowTR);
+ assert.equal(cursor.diffRowTR, rows[0]);
assert.equal(cursor.side, Side.LEFT);
cursor.moveToLastChunk();
- assert.ok(cursor.diffRow);
- assert.equal(cursor.diffRow, rows[1]);
+ assert.ok(cursor.diffRowTR);
+ assert.equal(cursor.diffRowTR, rows[1]);
assert.equal(cursor.side, Side.RIGHT);
});
@@ -237,22 +238,22 @@
});
test('diff cursor functionality (unified)', () => {
- assert.isOk(cursor.diffRow);
+ assert.isOk(cursor.diffRowTR);
const rows = [
...queryAll(diffElement, '.section.delta tr.diff-row'),
] as HTMLTableRowElement[];
- assert.equal(cursor.diffRow, rows[0]);
+ assert.equal(cursor.diffRowTR, rows[0]);
cursor.moveDown();
- assert.notEqual(cursor.diffRow, rows[0]);
- assert.equal(cursor.diffRow, rows[1]);
+ assert.notEqual(cursor.diffRowTR, rows[0]);
+ assert.equal(cursor.diffRowTR, rows[1]);
cursor.moveUp();
- assert.notEqual(cursor.diffRow, rows[1]);
- assert.equal(cursor.diffRow, rows[0]);
+ assert.notEqual(cursor.diffRowTR, rows[1]);
+ assert.equal(cursor.diffRowTR, rows[0]);
});
});
@@ -275,7 +276,7 @@
// Because the first delta in this diff is on the right, it should be set
// to the right side.
assert.equal(cursor.side, Side.RIGHT);
- assert.equal(cursor.diffRow, deltaRows[0]);
+ assert.equal(cursor.diffRowTR, deltaRows[0]);
const firstIndex = cursor.cursorManager.index;
// Move the side to the left. Because this delta only has a right side, we
@@ -284,8 +285,8 @@
cursor.moveLeft();
assert.equal(cursor.side, Side.LEFT);
- assert.notEqual(cursor.diffRow, rows[0]);
- assert.equal(cursor.diffRow, rowBeforeFirstDelta);
+ assert.notEqual(cursor.diffRowTR, rows[0]);
+ assert.equal(cursor.diffRowTR, rowBeforeFirstDelta);
assert.equal(cursor.cursorManager.index, firstIndex - 1);
// If we move down, we should skip everything in the first delta because
@@ -293,8 +294,8 @@
cursor.moveDown();
assert.equal(cursor.side, Side.LEFT);
- assert.notEqual(cursor.diffRow, rowBeforeFirstDelta);
- assert.notEqual(cursor.diffRow, rows[0]);
+ assert.notEqual(cursor.diffRowTR, rowBeforeFirstDelta);
+ assert.notEqual(cursor.diffRowTR, rows[0]);
assert.isTrue(cursor.cursorManager.index > firstIndex);
});
@@ -303,7 +304,7 @@
// We should be initialized to the first chunk. Since this chunk only has
// content on the right side, our side should be right.
- assert.equal(cursor.diffRow, deltaChunks[0].querySelector('tr'));
+ assert.equal(cursor.diffRowTR, deltaChunks[0].querySelector('tr'));
assert.equal(cursor.side, Side.RIGHT);
// Move to the next chunk.
@@ -311,7 +312,7 @@
// Since this chunk only has content on the left side. we should have been
// automatically moved over.
- assert.equal(cursor.diffRow, deltaChunks[1].querySelector('tr'));
+ assert.equal(cursor.diffRowTR, deltaChunks[1].querySelector('tr'));
assert.equal(cursor.side, Side.LEFT);
});
@@ -365,7 +366,7 @@
test('getTargetDiffElement', () => {
cursor.initialLineNumber = 1;
- assert.isTrue(!!cursor.diffRow);
+ assert.isTrue(!!cursor.diffRowTR);
assert.equal(cursor.getTargetDiffElement(), diffElement);
});
@@ -431,51 +432,56 @@
'createRangeComment'
);
const addDraftAtLineStub = sinon.stub(diffElement, 'addDraftAtLine');
- cursor.diffRow = undefined;
+ cursor.diffRowTR = undefined;
cursor.createCommentInPlace();
assert.isFalse(createRangeCommentStub.called);
assert.isFalse(addDraftAtLineStub.called);
});
});
- test('getAddress', () => {
+ test('getTargetLineNumber', () => {
// It should initialize to the first chunk: line 5 of the revision.
- assert.deepEqual(cursor.getAddress(), {leftSide: false, number: 5});
+ assert.deepEqual(cursor.getTargetLineNumber(), 5);
+ assert.deepEqual(cursor.side, Side.RIGHT);
// Revision line 4 is up.
cursor.moveUp();
- assert.deepEqual(cursor.getAddress(), {leftSide: false, number: 4});
+ assert.deepEqual(cursor.getTargetLineNumber(), 4);
+ assert.deepEqual(cursor.side, Side.RIGHT);
// Base line 4 is left.
cursor.moveLeft();
- assert.deepEqual(cursor.getAddress(), {leftSide: true, number: 4});
+ assert.deepEqual(cursor.getTargetLineNumber(), 4);
+ assert.deepEqual(cursor.side, Side.LEFT);
// Moving to the next chunk takes it back to the start.
cursor.moveToNextChunk();
- assert.deepEqual(cursor.getAddress(), {leftSide: false, number: 5});
+ assert.deepEqual(cursor.getTargetLineNumber(), 5);
+ assert.deepEqual(cursor.side, Side.RIGHT);
// The following chunk is a removal starting on line 10 of the base.
cursor.moveToNextChunk();
- assert.deepEqual(cursor.getAddress(), {leftSide: true, number: 10});
+ assert.deepEqual(cursor.getTargetLineNumber(), 10);
+ assert.deepEqual(cursor.side, Side.LEFT);
// Should be null if there is no selection.
cursor.cursorManager.unsetCursor();
- assert.isNotOk(cursor.getAddress());
+ assert.isUndefined(cursor.getTargetLineNumber());
});
- test('_findRowByNumberAndFile', () => {
+ test('findRowByNumberAndFile', () => {
// Get the first ab row after the first chunk.
const rows = [...queryAll<HTMLTableRowElement>(diffElement, 'tr')];
const row = rows[9];
assert.ok(row);
// It should be line 8 on the right, but line 5 on the left.
- assert.equal(cursor._findRowByNumberAndFile(8, Side.RIGHT), row);
- assert.equal(cursor._findRowByNumberAndFile(5, Side.LEFT), row);
+ assert.equal(cursor.findRowByNumberAndFile(8, Side.RIGHT), row);
+ assert.equal(cursor.findRowByNumberAndFile(5, Side.LEFT), row);
});
test('expand context updates stops', async () => {
- const spy = sinon.spy(cursor, '_updateStops');
+ const spy = sinon.spy(cursor, 'updateStops');
const controls = queryAndAssert(diffElement, 'gr-context-controls');
const showContext = queryAndAssert<HTMLElement>(controls, '.showContext');
showContext.click();
@@ -485,7 +491,7 @@
});
test('updates stops when loading changes', () => {
- const spy = sinon.spy(cursor, '_updateStops');
+ const spy = sinon.spy(cursor, 'updateStops');
diffElement.dispatchEvent(new Event('loading-changed'));
assert.isTrue(spy.called);
});
@@ -532,26 +538,17 @@
// Goto second last line of the first diff
cursor.moveToLineNumber(lastLine - 1, Side.RIGHT);
- assert.equal(
- cursor.getTargetLineElement()!.textContent?.trim(),
- `${lastLine - 1}`
- );
+ assert.equal(cursor.getTargetLineNumber(), lastLine - 1);
// Can move down until we reach the loading file
cursor.moveDown();
assert.equal(getTargetDiffIndex(), 0);
- assert.equal(
- cursor.getTargetLineElement()!.textContent?.trim(),
- lastLine.toString()
- );
+ assert.equal(cursor.getTargetLineNumber(), lastLine);
// Cannot move down while still loading the diff we would switch to
cursor.moveDown();
assert.equal(getTargetDiffIndex(), 0);
- assert.equal(
- cursor.getTargetLineElement()!.textContent?.trim(),
- lastLine.toString()
- );
+ assert.equal(cursor.getTargetLineNumber(), lastLine);
// Diff 1 finishing to load
diffElements[1].diff = createDiff();
@@ -561,7 +558,7 @@
cursor.moveDown(); // LOST
cursor.moveDown(); // FILE
assert.equal(getTargetDiffIndex(), 1);
- assert.equal(cursor.getTargetLineElement()!.textContent?.trim(), 'File');
+ assert.equal(cursor.getTargetLineNumber(), FILE);
});
});
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
index 1cdfbc3..5f890ce 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -15,11 +15,11 @@
getLineElByChild,
getLineNumberByChild,
getSideByLineEl,
- GrDiffThreadElement,
+ GrDiffCommentThread,
} from '../gr-diff/gr-diff-utils';
import {debounce, DelayedTask} from '../../../utils/async-util';
import {assertIsDefined, queryAndAssert} from '../../../utils/common-util';
-import {fire} from '../../../utils/event-util';
+import {DiffModel} from '../gr-diff-model/gr-diff-model';
interface SidedRange {
side: Side;
@@ -44,6 +44,7 @@
*/
export interface DiffBuilderInterface {
getContentTdByLineEl(lineEl?: Element): Element | undefined;
+ diffModel: DiffModel;
}
/**
@@ -134,64 +135,31 @@
);
}
- private getThreadEl(e: Event): GrDiffThreadElement | null {
- for (const pathEl of e.composedPath()) {
- if (
- pathEl instanceof HTMLElement &&
- pathEl.classList.contains('comment-thread')
- ) {
- return pathEl as GrDiffThreadElement;
- }
- }
- return null;
- }
-
private toggleRangeElHighlight(
- threadEl: GrDiffThreadElement | null,
+ thread: GrDiffCommentThread,
highlightRange = false
) {
- const rootId = threadEl?.rootId;
+ const rootId = thread?.rootId;
if (!rootId) return;
if (!this.diffTable) return;
- if (highlightRange) {
- const selector = `.range.${strToClassName(rootId)}`;
- const rangeNodes = this.diffTable.querySelectorAll(selector);
- rangeNodes.forEach(rangeNode => {
- rangeNode.classList.add('rangeHoverHighlight');
- });
- const hintNode = this.diffTable.querySelector(
- `gr-ranged-comment-hint[threadElRootId="${rootId}"]`
- );
- hintNode?.shadowRoot
- ?.querySelectorAll('.rangeHighlight')
- .forEach(highlightNode =>
- highlightNode.classList.add('rangeHoverHighlight')
- );
- } else {
- const selector = `.rangeHoverHighlight.${strToClassName(rootId)}`;
- const rangeNodes = this.diffTable.querySelectorAll(selector);
- rangeNodes.forEach(rangeNode => {
- rangeNode.classList.remove('rangeHoverHighlight');
- });
- const hintNode = this.diffTable.querySelector(
- `gr-ranged-comment-hint[threadElRootId="${rootId}"]`
- );
- hintNode?.shadowRoot
- ?.querySelectorAll('.rangeHoverHighlight')
- .forEach(highlightNode =>
- highlightNode.classList.remove('rangeHoverHighlight')
- );
+ const highlightClass = highlightRange ? 'range' : 'rangeHoverHighlight';
+ const selector = `.${highlightClass}.${strToClassName(rootId)}`;
+ const rangeNodes = this.diffTable.querySelectorAll(selector);
+ for (const rangeNode of rangeNodes) {
+ rangeNode.classList.toggle('rangeHoverHighlight', highlightRange);
}
}
- private handleCommentThreadMouseenter = (e: Event) => {
- const threadEl = this.getThreadEl(e);
- this.toggleRangeElHighlight(threadEl, /* highlightRange= */ true);
+ private handleCommentThreadMouseenter = (
+ e: CustomEvent<GrDiffCommentThread>
+ ) => {
+ this.toggleRangeElHighlight(e.detail, /* highlightRange= */ true);
};
- private handleCommentThreadMouseleave = (e: Event) => {
- const threadEl = this.getThreadEl(e);
- this.toggleRangeElHighlight(threadEl, /* highlightRange= */ false);
+ private handleCommentThreadMouseleave = (
+ e: CustomEvent<GrDiffCommentThread>
+ ) => {
+ this.toggleRangeElHighlight(e.detail, /* highlightRange= */ false);
};
/**
@@ -407,7 +375,7 @@
// is empty to see that it's at the end of a line.
const content = domRange.cloneContents().querySelector('.contentText');
if (isMouseUp && this.getLength(content) === 0) {
- this.fireCreateRangeComment(start.side, {
+ this.createRangeComment(start.side, {
start_line: start.line,
start_character: 0,
end_line: start.line,
@@ -457,10 +425,9 @@
}
}
- private fireCreateRangeComment(side: Side, range: CommentRange) {
- if (this.diffTable) {
- fire(this.diffTable, 'create-range-comment', {side, range});
- }
+ private createRangeComment(side: Side, range: CommentRange) {
+ assertIsDefined(this.diffBuilder, 'diffBuilder');
+ this.diffBuilder?.diffModel.createCommentOnRange(range, side);
this.removeActionBox();
}
@@ -468,7 +435,7 @@
e.stopPropagation();
assertIsDefined(this.selectedRange, 'selectedRange');
const {side, range} = this.selectedRange;
- this.fireCreateRangeComment(side, range);
+ this.createRangeComment(side, range);
};
// visible for testing
@@ -512,14 +479,3 @@
}
}
}
-
-export interface CreateRangeCommentEventDetail {
- side: Side;
- range: CommentRange;
-}
-
-declare global {
- interface HTMLElementEventMap {
- 'create-range-comment': CustomEvent<CreateRangeCommentEventDetail>;
- }
-}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight_test.ts
index f04e6a2..32decb1 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight_test.ts
@@ -7,21 +7,21 @@
import './gr-diff-highlight';
import {getTextOffset} from './gr-range-normalizer';
import {fixture, fixtureCleanup, html, assert} from '@open-wc/testing';
-import {
- GrDiffHighlight,
- DiffBuilderInterface,
- CreateRangeCommentEventDetail,
-} from './gr-diff-highlight';
+import {GrDiffHighlight, DiffBuilderInterface} from './gr-diff-highlight';
import {Side} from '../../../api/diff';
import {SinonStubbedMember} from 'sinon';
import {queryAndAssert} from '../../../utils/common-util';
-import {GrDiffThreadElement} from '../gr-diff/gr-diff-utils';
+import {
+ GrDiffThreadElement,
+ getDataFromCommentThreadEl,
+} from '../gr-diff/gr-diff-utils';
import {
stubElement,
waitQueryAndAssert,
waitUntil,
} from '../../../test/test-utils';
import {GrSelectionActionBox} from '../gr-selection-action-box/gr-selection-action-box';
+import {DiffModel} from '../gr-diff-model/gr-diff-model';
// Splitting long lines in html into shorter rows breaks tests:
// zero-length text nodes and new lines are not expected in some places
@@ -132,16 +132,13 @@
let hlRange: HTMLElement;
let element: GrDiffHighlight;
let diff: HTMLElement;
- let builder: {
- getContentTdByLineEl: SinonStubbedMember<
- DiffBuilderInterface['getContentTdByLineEl']
- >;
- };
+ let builder: DiffBuilderInterface;
setup(async () => {
diff = await fixture<HTMLTableElement>(diffTable);
builder = {
getContentTdByLineEl: sinon.stub(),
+ diffModel: new DiffModel(document),
};
element = new GrDiffHighlight();
element.init(diff, builder);
@@ -152,6 +149,8 @@
) as unknown as GrDiffThreadElement;
threadEl.className = 'comment-thread';
threadEl.rootId = 'id314';
+ threadEl.setAttribute('line-num', '12');
+ threadEl.setAttribute('diff-side', 'right');
diff.appendChild(threadEl);
});
@@ -164,6 +163,7 @@
assert.isFalse(hlRange.classList.contains('rangeHoverHighlight'));
threadEl.dispatchEvent(
new CustomEvent('comment-thread-mouseenter', {
+ detail: getDataFromCommentThreadEl(threadEl),
bubbles: true,
composed: true,
})
@@ -176,6 +176,7 @@
hlRange.classList.add('rangeHoverHighlight');
threadEl.dispatchEvent(
new CustomEvent('comment-thread-mouseleave', {
+ detail: getDataFromCommentThreadEl(threadEl),
bubbles: true,
composed: true,
})
@@ -187,23 +188,23 @@
test(`create-range-comment for range when create-comment-requested
is fired`, () => {
const removeActionBoxStub = sinon.stub(element, 'removeActionBox');
- element.selectedRange = {
- side: Side.LEFT,
- range: {
- start_line: 7,
- start_character: 11,
- end_line: 24,
- end_character: 42,
- },
+ const range = {
+ start_line: 7,
+ start_character: 11,
+ end_line: 24,
+ end_character: 42,
};
- const requestEvent = new CustomEvent('create-comment-requested');
- let createRangeEvent: CustomEvent<CreateRangeCommentEventDetail>;
- diff.addEventListener('create-range-comment', e => {
- createRangeEvent = e;
- });
- diff.dispatchEvent(requestEvent);
- if (!createRangeEvent!) assert.fail('event not set');
- assert.deepEqual(element.selectedRange, createRangeEvent.detail);
+ element.selectedRange = {side: Side.LEFT, range};
+ const createCommentStub = sinon.stub(
+ builder.diffModel,
+ 'createCommentOnRange'
+ );
+
+ diff.dispatchEvent(new CustomEvent('create-comment-requested'));
+
+ assert.isTrue(createCommentStub.called);
+ assert.deepEqual(createCommentStub.lastCall.args[0], range);
+ assert.equal(createCommentStub.lastCall.args[1], Side.LEFT);
assert.isTrue(removeActionBoxStub.called);
});
});
@@ -211,18 +212,18 @@
suite('selection', () => {
let element: GrDiffHighlight;
let diff: HTMLElement;
- let builder: {
- getContentTdByLineEl: SinonStubbedMember<
- DiffBuilderInterface['getContentTdByLineEl']
- >;
- };
+ let getContentTdByLineElStub: SinonStubbedMember<
+ DiffBuilderInterface['getContentTdByLineEl']
+ >;
let contentStubs;
setup(async () => {
diff = await fixture<HTMLTableElement>(diffTable);
- builder = {
- getContentTdByLineEl: sinon.stub(),
+ const builder: DiffBuilderInterface = {
+ getContentTdByLineEl: () => undefined,
+ diffModel: new DiffModel(document),
};
+ getContentTdByLineElStub = sinon.stub(builder, 'getContentTdByLineEl');
element = new GrDiffHighlight();
element.init(diff, builder);
contentStubs = [];
@@ -251,7 +252,7 @@
contentTd,
contentText,
});
- builder.getContentTdByLineEl.withArgs(lineEl).returns(contentTd);
+ getContentTdByLineElStub.withArgs(lineEl).returns(contentTd);
return contentText;
};
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts b/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
index b8539ef..8becace 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
@@ -6,13 +6,17 @@
import {Observable, combineLatest, from} from 'rxjs';
import {debounceTime, filter, switchMap, withLatestFrom} from 'rxjs/operators';
import {
+ CreateCommentEventDetail,
DiffInfo,
DiffLayer,
DiffPreferencesInfo,
DiffResponsiveMode,
DiffViewMode,
DisplayLine,
+ LineNumber,
+ LineSelectedEventDetail,
RenderPreferences,
+ Side,
} from '../../../api/diff';
import {define} from '../../../models/dependency';
import {Model} from '../../../models/model';
@@ -35,6 +39,8 @@
import {assert} from '../../../utils/common-util';
import {isImageDiff} from '../../../utils/diff-util';
import {ImageInfo} from '../../../types/common';
+import {fire} from '../../../utils/event-util';
+import {CommentRange} from '../../../api/rest-api';
export interface DiffState {
diff?: DiffInfo;
@@ -119,6 +125,11 @@
diffState => diffState.errorMessage
);
+ readonly comments$: Observable<GrDiffCommentThread[]> = select(
+ this.state$,
+ diffState => diffState.comments ?? []
+ );
+
readonly groups$: Observable<GrDiffGroup[]> = select(
this.state$,
diffState => diffState.groups ?? []
@@ -140,7 +151,14 @@
computeKeyLocations(diffState.lineOfInterest, diffState.comments ?? [])
);
- constructor() {
+ constructor(
+ /**
+ * Normally a reference to the <gr-diff> component. Used for firing events
+ * that are meant for <gr-diff> or the host of <gr-diff>. For tests this
+ * can also be just `document`.
+ */
+ private readonly eventTarget: EventTarget
+ ) {
super({
diffPrefs: createDefaultDiffPrefs(),
renderPrefs: {},
@@ -191,4 +209,30 @@
groups.splice(i, 1, ...newGroups);
this.updateState({groups});
}
+
+ selectLine(number: LineNumber, side: Side) {
+ const path = this.getState().path;
+ if (!path) return;
+
+ const detail: LineSelectedEventDetail = {number, side, path};
+ fire(this.eventTarget, 'line-selected', detail);
+ }
+
+ createCommentOnLine(lineNum: LineNumber, side: Side) {
+ const detail: CreateCommentEventDetail = {
+ side,
+ lineNum,
+ range: undefined,
+ };
+ fire(this.eventTarget, 'create-comment', detail);
+ }
+
+ createCommentOnRange(range: CommentRange, side: Side) {
+ const detail: CreateCommentEventDetail = {
+ side,
+ lineNum: range.end_line,
+ range,
+ };
+ fire(this.eventTarget, 'create-comment', detail);
+ }
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-element_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-element_test.ts
index 6f476e5..56336f0 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-element_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-element_test.ts
@@ -39,7 +39,7 @@
};
setup(async () => {
- model = new DiffModel();
+ model = new DiffModel(document);
element = (
await fixture(
wrapInProvider(
@@ -102,9 +102,9 @@
<td class="blame gr-diff" data-line-number="LOST"></td>
<td class="gr-diff left lineNum" data-value="LOST"></td>
<td class="gr-diff lineNum right" data-value="LOST"></td>
- <td class="both content gr-diff lost no-intraline-info right">
- <div class="thread-group" data-side="right"></div>
- </td>
+ <td
+ class="both content gr-diff lost no-intraline-info right"
+ ></td>
</tr>
</tbody>
<tbody class="both gr-diff section">
@@ -122,7 +122,7 @@
id="left-button-FILE"
tabindex="-1"
>
- File
+ FILE
</button>
</td>
<td class="gr-diff lineNum right" data-value="FILE">
@@ -133,12 +133,12 @@
id="right-button-FILE"
tabindex="-1"
>
- File
+ FILE
</button>
</td>
- <td class="both content file gr-diff no-intraline-info right">
- <div class="thread-group" data-side="right"></div>
- </td>
+ <td
+ class="both content file gr-diff no-intraline-info right"
+ ></td>
</tr>
</tbody>
<tbody class="both gr-diff section">
@@ -176,7 +176,6 @@
data-side="right"
id="right-content-1"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -213,7 +212,6 @@
data-side="right"
id="right-content-2"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -250,7 +248,6 @@
data-side="right"
id="right-content-3"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -287,7 +284,6 @@
data-side="right"
id="right-content-4"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -316,7 +312,6 @@
data-side="right"
id="right-content-5"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -343,7 +338,6 @@
data-side="right"
id="right-content-6"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -370,7 +364,6 @@
data-side="right"
id="right-content-7"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -409,7 +402,6 @@
data-side="right"
id="right-content-8"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -446,7 +438,6 @@
data-side="right"
id="right-content-9"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -483,7 +474,6 @@
data-side="right"
id="right-content-10"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -520,7 +510,6 @@
data-side="right"
id="right-content-11"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -557,7 +546,6 @@
data-side="right"
id="right-content-12"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -586,7 +574,6 @@
data-side="left"
id="left-content-10"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
</tr>
<tr
@@ -613,7 +600,6 @@
data-side="left"
id="left-content-11"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
</tr>
<tr
@@ -640,7 +626,6 @@
data-side="left"
id="left-content-12"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
</tr>
<tr
@@ -667,7 +652,6 @@
data-side="left"
id="left-content-13"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
</tr>
</tbody>
@@ -696,7 +680,6 @@
data-side="right"
id="right-content-13"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -723,7 +706,6 @@
data-side="right"
id="right-content-14"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -752,7 +734,6 @@
data-side="left"
id="left-content-16"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
</tr>
<tr
@@ -779,7 +760,6 @@
data-side="right"
id="right-content-15"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -818,7 +798,6 @@
data-side="right"
id="right-content-16"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -855,7 +834,6 @@
data-side="right"
id="right-content-17"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -892,7 +870,6 @@
data-side="right"
id="right-content-18"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -952,7 +929,6 @@
data-side="right"
id="right-content-37"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -989,7 +965,6 @@
data-side="right"
id="right-content-38"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1026,7 +1001,6 @@
data-side="right"
id="right-content-39"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1055,7 +1029,6 @@
data-side="right"
id="right-content-40"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1082,7 +1055,6 @@
data-side="right"
id="right-content-41"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1109,7 +1081,6 @@
data-side="right"
id="right-content-42"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1136,7 +1107,6 @@
data-side="right"
id="right-content-43"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1175,7 +1145,6 @@
data-side="right"
id="right-content-44"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1212,7 +1181,6 @@
data-side="right"
id="right-content-45"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1249,7 +1217,6 @@
data-side="right"
id="right-content-46"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1286,7 +1253,6 @@
data-side="right"
id="right-content-47"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1323,7 +1289,6 @@
data-side="right"
id="right-content-48"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1376,14 +1341,14 @@
<td class="blame gr-diff" data-line-number="LOST"></td>
<td class="gr-diff left lineNum" data-value="LOST"></td>
<td class="gr-diff left no-intraline-info sign"></td>
- <td class="both content gr-diff left lost no-intraline-info">
- <div class="thread-group" data-side="left"></div>
- </td>
+ <td
+ class="both content gr-diff left lost no-intraline-info"
+ ></td>
<td class="gr-diff lineNum right" data-value="LOST"></td>
<td class="gr-diff no-intraline-info right sign"></td>
- <td class="both content gr-diff lost no-intraline-info right">
- <div class="thread-group" data-side="right"></div>
- </td>
+ <td
+ class="both content gr-diff lost no-intraline-info right"
+ ></td>
</tr>
</tbody>
<tbody class="both gr-diff section">
@@ -1403,13 +1368,13 @@
id="left-button-FILE"
tabindex="-1"
>
- File
+ FILE
</button>
</td>
<td class="gr-diff left no-intraline-info sign"></td>
- <td class="both content file gr-diff left no-intraline-info">
- <div class="thread-group" data-side="left"></div>
- </td>
+ <td
+ class="both content file gr-diff left no-intraline-info"
+ ></td>
<td class="gr-diff lineNum right" data-value="FILE">
<button
aria-label="Add file comment"
@@ -1418,13 +1383,13 @@
id="right-button-FILE"
tabindex="-1"
>
- File
+ FILE
</button>
</td>
<td class="gr-diff no-intraline-info right sign"></td>
- <td class="both content file gr-diff no-intraline-info right">
- <div class="thread-group" data-side="right"></div>
- </td>
+ <td
+ class="both content file gr-diff no-intraline-info right"
+ ></td>
</tr>
</tbody>
<tbody class="both gr-diff section">
@@ -1454,7 +1419,6 @@
data-side="left"
id="left-content-1"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="1">
<button
@@ -1474,7 +1438,6 @@
data-side="right"
id="right-content-1"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1503,7 +1466,6 @@
data-side="left"
id="left-content-2"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="2">
<button
@@ -1523,7 +1485,6 @@
data-side="right"
id="right-content-2"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1552,7 +1513,6 @@
data-side="left"
id="left-content-3"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="3">
<button
@@ -1572,7 +1532,6 @@
data-side="right"
id="right-content-3"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1601,7 +1560,6 @@
data-side="left"
id="left-content-4"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="4">
<button
@@ -1621,7 +1579,6 @@
data-side="right"
id="right-content-4"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1657,7 +1614,6 @@
data-side="right"
id="right-content-5"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1691,7 +1647,6 @@
data-side="right"
id="right-content-6"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1725,7 +1680,6 @@
data-side="right"
id="right-content-7"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1756,7 +1710,6 @@
data-side="left"
id="left-content-5"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="8">
<button
@@ -1776,7 +1729,6 @@
data-side="right"
id="right-content-8"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1805,7 +1757,6 @@
data-side="left"
id="left-content-6"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="9">
<button
@@ -1825,7 +1776,6 @@
data-side="right"
id="right-content-9"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1854,7 +1804,6 @@
data-side="left"
id="left-content-7"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="10">
<button
@@ -1874,7 +1823,6 @@
data-side="right"
id="right-content-10"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1903,7 +1851,6 @@
data-side="left"
id="left-content-8"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="11">
<button
@@ -1923,7 +1870,6 @@
data-side="right"
id="right-content-11"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1952,7 +1898,6 @@
data-side="left"
id="left-content-9"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="12">
<button
@@ -1972,7 +1917,6 @@
data-side="right"
id="right-content-12"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -2003,7 +1947,6 @@
data-side="left"
id="left-content-10"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
@@ -2037,7 +1980,6 @@
data-side="left"
id="left-content-11"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
@@ -2071,7 +2013,6 @@
data-side="left"
id="left-content-12"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
@@ -2105,7 +2046,6 @@
data-side="left"
id="left-content-13"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
@@ -2141,7 +2081,6 @@
data-side="left"
id="left-content-14"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="13">
<button
@@ -2161,7 +2100,6 @@
data-side="right"
id="right-content-13"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2190,7 +2128,6 @@
data-side="left"
id="left-content-15"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="14">
<button
@@ -2210,7 +2147,6 @@
data-side="right"
id="right-content-14"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -2241,7 +2177,6 @@
data-side="left"
id="left-content-16"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="15">
<button
@@ -2261,7 +2196,6 @@
data-side="right"
id="right-content-15"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -2292,7 +2226,6 @@
data-side="left"
id="left-content-17"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="16">
<button
@@ -2312,7 +2245,6 @@
data-side="right"
id="right-content-16"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2341,7 +2273,6 @@
data-side="left"
id="left-content-18"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="17">
<button
@@ -2361,7 +2292,6 @@
data-side="right"
id="right-content-17"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2390,7 +2320,6 @@
data-side="left"
id="left-content-19"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="18">
<button
@@ -2410,7 +2339,6 @@
data-side="right"
id="right-content-18"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -2479,7 +2407,6 @@
data-side="left"
id="left-content-38"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="37">
<button
@@ -2499,7 +2426,6 @@
data-side="right"
id="right-content-37"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2528,7 +2454,6 @@
data-side="left"
id="left-content-39"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="38">
<button
@@ -2548,7 +2473,6 @@
data-side="right"
id="right-content-38"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2577,7 +2501,6 @@
data-side="left"
id="left-content-40"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="39">
<button
@@ -2597,7 +2520,6 @@
data-side="right"
id="right-content-39"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -2633,7 +2555,6 @@
data-side="right"
id="right-content-40"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2667,7 +2588,6 @@
data-side="right"
id="right-content-41"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2701,7 +2621,6 @@
data-side="right"
id="right-content-42"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2735,7 +2654,6 @@
data-side="right"
id="right-content-43"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -2766,7 +2684,6 @@
data-side="left"
id="left-content-41"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="44">
<button
@@ -2786,7 +2703,6 @@
data-side="right"
id="right-content-44"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2815,7 +2731,6 @@
data-side="left"
id="left-content-42"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="45">
<button
@@ -2835,7 +2750,6 @@
data-side="right"
id="right-content-45"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2864,7 +2778,6 @@
data-side="left"
id="left-content-43"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="46">
<button
@@ -2884,7 +2797,6 @@
data-side="right"
id="right-content-46"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2913,7 +2825,6 @@
data-side="left"
id="left-content-44"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="47">
<button
@@ -2933,7 +2844,6 @@
data-side="right"
id="right-content-47"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -2962,7 +2872,6 @@
data-side="left"
id="left-content-45"
></div>
- <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="48">
<button
@@ -2982,7 +2891,6 @@
data-side="right"
id="right-content-48"
></div>
- <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -3057,17 +2965,13 @@
id="left-button-FILE"
tabindex="-1"
>
- File
+ FILE
</button>
</td>
<td class="gr-diff left no-intraline-info sign"></td>
<td
class="both content file gr-diff left no-intraline-info"
- >
- <div class="thread-group" data-side="left">
- <slot name="left-FILE"> </slot>
- </div>
- </td>
+ ></td>
<td class="gr-diff lineNum right" data-value="FILE">
<button
aria-label="Add file comment"
@@ -3076,17 +2980,13 @@
id="right-button-FILE"
tabindex="-1"
>
- File
+ FILE
</button>
</td>
<td class="gr-diff no-intraline-info right sign"></td>
<td
class="both content file gr-diff no-intraline-info right"
- >
- <div class="thread-group" data-side="right">
- <slot name="right-FILE"> </slot>
- </div>
- </td>
+ ></td>
</tr>
</tbody>
<tbody class="binary-diff gr-diff">
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts
index cfb64b9..c41dc91 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts
@@ -163,8 +163,9 @@
}
/**
- * This is all the data that gr-diff extracts from comment thread elements.
- * Otherwise gr-diff treats such elements as a black box.
+ * This is all the data that gr-diff extracts from comment thread elements,
+ * see `GrDiffThreadElement`. Otherwise gr-diff treats such elements as a black
+ * box.
*/
export interface GrDiffCommentThread {
side: Side;
@@ -173,8 +174,12 @@
rootId?: string;
}
-export function toCommentThreadModel(
- threadEl: HTMLElement
+/**
+ * Retrieves all the data from a comment thread element that the gr-diff API
+ * contract defines for such elements.
+ */
+export function getDataFromCommentThreadEl(
+ threadEl?: EventTarget | null
): GrDiffCommentThread | undefined {
if (!isThreadEl(threadEl)) return undefined;
const side = getSide(threadEl);
@@ -300,14 +305,19 @@
// For Gerrit these are instances of GrCommentThread, but other gr-diff users
// have different HTML elements in use for comment threads.
// TODO: Also document the required HTML attributes that thread elements must
-// have, e.g. 'diff-side', 'range', 'line-num'.
+// have, e.g. 'diff-side', 'range' (optional), 'line-num'.
+// Comment widgets are also required to have `comment-thread` in their css
+// class list.
export interface GrDiffThreadElement extends HTMLElement {
rootId: string;
}
-export function isThreadEl(node: Node): node is GrDiffThreadElement {
+export function isThreadEl(
+ node?: Node | EventTarget | null
+): node is GrDiffThreadElement {
return (
- node.nodeType === Node.ELEMENT_NODE &&
+ !!node &&
+ (node as Node).nodeType === Node.ELEMENT_NODE &&
(node as Element).classList.contains('comment-thread')
);
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts
index 639b1ac..44f4f60 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts
@@ -12,7 +12,7 @@
getRange,
computeKeyLocations,
GrDiffCommentThread,
- toCommentThreadModel,
+ getDataFromCommentThreadEl,
compareComments,
GrDiffThreadElement,
computeContext,
@@ -290,7 +290,7 @@
el.setAttribute('line-num', '3');
el.rootId = 'ab12';
- assert.deepEqual(toCommentThreadModel(el), {
+ assert.deepEqual(getDataFromCommentThreadEl(el), {
line: 3,
side: Side.LEFT,
range: undefined,
@@ -306,7 +306,7 @@
el.setAttribute('diff-side', 'left');
el.rootId = 'ab12';
- assert.deepEqual(toCommentThreadModel(el), {
+ assert.deepEqual(getDataFromCommentThreadEl(el), {
line: FILE,
side: Side.LEFT,
range: undefined,
@@ -318,11 +318,11 @@
const el = document.createElement(
'div'
) as unknown as GrDiffThreadElement;
- assert.isUndefined(toCommentThreadModel(el));
+ assert.isUndefined(getDataFromCommentThreadEl(el));
el.className = 'comment-thread';
- assert.isUndefined(toCommentThreadModel(el));
+ assert.isUndefined(getDataFromCommentThreadEl(el));
el.setAttribute('line-num', '3');
- assert.isUndefined(toCommentThreadModel(el));
+ assert.isUndefined(getDataFromCommentThreadEl(el));
});
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index c118dd9..ec6ed03 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -16,35 +16,23 @@
import './gr-diff-element';
import '../gr-diff-builder/gr-diff-row';
import {
- getLine,
- getLineElByChild,
getLineNumber,
- getRange,
- getSide,
- GrDiffThreadElement,
- isLongCommentRange,
isThreadEl,
- rangesEqual,
getResponsiveMode,
isResponsive,
isNewDiff,
getSideByLineEl,
compareComments,
- toCommentThreadModel,
+ getDataFromCommentThreadEl,
FullContext,
DiffContextExpandedEventDetail,
+ GrDiffCommentThread,
} from '../gr-diff/gr-diff-utils';
import {BlameInfo, CommentRange, ImageInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {
- CreateRangeCommentEventDetail,
- GrDiffHighlight,
-} from '../gr-diff-highlight/gr-diff-highlight';
+import {GrDiffHighlight} from '../gr-diff-highlight/gr-diff-highlight';
import {CoverageRange, DiffLayer, isDefined} from '../../../types/types';
-import {
- CommentRangeLayer,
- GrRangedCommentLayer,
-} from '../gr-ranged-comment-layer/gr-ranged-comment-layer';
+import {GrRangedCommentLayer} from '../gr-ranged-comment-layer/gr-ranged-comment-layer';
import {DiffViewMode, Side} from '../../../constants/constants';
import {fire, fireAlert} from '../../../utils/event-util';
import {MovedLinkClickedEvent, ValueChangedEvent} from '../../../types/events';
@@ -55,10 +43,9 @@
GrDiff as GrDiffApi,
DisplayLine,
LineNumber,
- LOST,
ContentLoadNeededEventDetail,
} from '../../../api/diff';
-import {isHtmlElement, isSafari, toggleClass} from '../../../utils/dom-util';
+import {isSafari, toggleClass} from '../../../utils/dom-util';
import {assertIsDefined} from '../../../utils/common-util';
import {GrDiffSelection} from '../gr-diff-selection/gr-diff-selection';
import {property, query, state} from 'lit/decorators.js';
@@ -165,10 +152,6 @@
@property({type: Boolean})
noRenderOnPrefsChange?: boolean;
- // Private but used in tests.
- @state()
- commentRanges: CommentRangeLayer[] = [];
-
// explicitly highlight a range if it is not associated with any comment
@property({type: Object})
highlightRange?: CommentRange;
@@ -252,7 +235,7 @@
highlights = new GrDiffHighlight();
// Private but used in tests.
- diffModel = new DiffModel();
+ diffModel = new DiffModel(this);
/**
* Just the layers that are passed in from the outside. Will be joined with
@@ -306,13 +289,8 @@
() => this.diffModel.groups$,
groups => (this.groups = groups)
);
- this.addEventListener(
- 'create-range-comment',
- (e: CustomEvent<CreateRangeCommentEventDetail>) =>
- this.handleCreateRangeComment(e)
- );
this.addEventListener('moved-link-clicked', (e: MovedLinkClickedEvent) => {
- this.dispatchSelectedLine(e.detail.lineNum, e.detail.side);
+ this.diffModel.selectLine(e.detail.lineNum, e.detail.side);
});
this.addEventListener(
'diff-context-expanded-internal-new',
@@ -430,7 +408,7 @@
override render() {
fire(this, 'render-start', {});
- return html`<gr-diff-element @click=${this.handleTap}></gr-diff-element>`;
+ return html`<gr-diff-element></gr-diff-element>`;
}
private addSelectionListeners() {
@@ -472,63 +450,22 @@
: document.getSelection();
}
- private updateRanges(
- addedThreadEls: GrDiffThreadElement[],
- removedThreadEls: GrDiffThreadElement[]
- ) {
- function commentRangeFromThreadEl(
- threadEl: GrDiffThreadElement
- ): CommentRangeLayer | undefined {
- const side = getSide(threadEl);
- if (!side) return undefined;
- const range = getRange(threadEl);
- if (!range) return undefined;
+ private commentThreadRedispatcher = (
+ target: EventTarget | null,
+ eventName: 'comment-thread-mouseenter' | 'comment-thread-mouseleave'
+ ) => {
+ if (!isThreadEl(target)) return;
+ const data = getDataFromCommentThreadEl(target);
+ if (data) fire(target, eventName, data);
+ };
- return {side, range, rootId: threadEl.rootId};
- }
+ private commentThreadEnterRedispatcher = (e: Event) => {
+ this.commentThreadRedispatcher(e.target, 'comment-thread-mouseenter');
+ };
- // TODO(brohlfs): Rewrite `.map().filter() as ...` with `.reduce()` instead.
- const addedCommentRanges = addedThreadEls
- .map(commentRangeFromThreadEl)
- .filter(range => !!range) as CommentRangeLayer[];
- const removedCommentRanges = removedThreadEls
- .map(commentRangeFromThreadEl)
- .filter(range => !!range) as CommentRangeLayer[];
- for (const removedCommentRange of removedCommentRanges) {
- const i = this.commentRanges.findIndex(
- cr =>
- cr.side === removedCommentRange.side &&
- rangesEqual(cr.range, removedCommentRange.range)
- );
- this.commentRanges.splice(i, 1);
- }
-
- if (addedCommentRanges?.length) {
- this.commentRanges.push(...addedCommentRanges);
- }
- if (this.highlightRange) {
- this.commentRanges.push({
- side: Side.RIGHT,
- range: this.highlightRange,
- rootId: '',
- });
- }
-
- this.rangeLayer?.updateRanges(this.commentRanges);
- }
-
- // Dispatch events that are handled by the gr-diff-highlight.
- private redispatchHoverEvents(
- hoverEl: HTMLElement,
- threadEl: GrDiffThreadElement
- ) {
- hoverEl.addEventListener('mouseenter', () => {
- fire(threadEl, 'comment-thread-mouseenter', {});
- });
- hoverEl.addEventListener('mouseleave', () => {
- fire(threadEl, 'comment-thread-mouseleave', {});
- });
- }
+ private commentThreadLeaveRedispatcher = (e: Event) => {
+ this.commentThreadRedispatcher(e.target, 'comment-thread-mouseleave');
+ };
/** TODO: Can be removed when diff-old is gone. */
cancel() {}
@@ -565,105 +502,21 @@
}
// Private but used in tests.
- handleTap(e: Event) {
- const el = e.target as Element;
-
- if (
- el.getAttribute('data-value') !== LOST &&
- (el.classList.contains('lineNum') ||
- el.classList.contains('lineNumButton'))
- ) {
- this.addDraftAtLine(el);
- } else if (
- el.tagName === 'HL' ||
- el.classList.contains('content') ||
- el.classList.contains('contentText')
- ) {
- const target = getLineElByChild(el);
- if (target) {
- this.selectLine(target);
- }
- }
- }
-
- // Private but used in tests.
selectLine(el: Element) {
const lineNumber = Number(el.getAttribute('data-value'));
const side = el.classList.contains('left') ? Side.LEFT : Side.RIGHT;
- this.dispatchSelectedLine(lineNumber, side);
+ this.diffModel.selectLine(lineNumber, side);
}
- private dispatchSelectedLine(number: LineNumber, side: Side) {
- fire(this, 'line-selected', {
- number,
- side,
- path: this.path,
- });
- }
-
- addDraftAtLine(el: Element) {
- this.selectLine(el);
-
- const lineNum = getLineNumber(el);
- if (lineNum === null) {
- fireAlert(this, 'Invalid line number');
- return;
- }
-
- this.createComment(el, lineNum);
+ addDraftAtLine(lineNum: LineNumber, side: Side) {
+ this.diffModel.createCommentOnLine(lineNum, side);
}
createRangeComment() {
- if (!this.isRangeSelected()) {
- throw Error('Selection is needed for new range comment');
- }
const selectedRange = this.highlights.selectedRange;
- if (!selectedRange) throw Error('selected range not set');
+ assertIsDefined(selectedRange, 'no range selected');
const {side, range} = selectedRange;
- this.createCommentForSelection(side, range);
- }
-
- createCommentForSelection(side: Side, range: CommentRange) {
- const lineNum = range.end_line;
- const lineEl = this.getLineElByNumber(lineNum, side);
- if (lineEl) {
- this.createComment(lineEl, lineNum, side, range);
- }
- }
-
- private handleCreateRangeComment(
- e: CustomEvent<CreateRangeCommentEventDetail>
- ) {
- const range = e.detail.range;
- const side = e.detail.side;
- this.createCommentForSelection(side, range);
- }
-
- // Private but used in tests.
- createComment(
- lineEl: Element,
- lineNum: LineNumber,
- side?: Side,
- range?: CommentRange
- ) {
- const contentEl = this.getContentTdByLineEl(lineEl);
- if (!contentEl) throw new Error('content el not found for line el');
- side = side ?? this.getCommentSideByLineAndContent(lineEl, contentEl);
- fire(this, 'create-comment', {
- side,
- lineNum,
- range,
- });
- }
-
- private getCommentSideByLineAndContent(
- lineEl: Element,
- contentEl: Element
- ): Side {
- return lineEl.classList.contains(Side.LEFT) ||
- contentEl.classList.contains('remove')
- ? Side.LEFT
- : Side.RIGHT;
+ this.diffModel.createCommentOnRange(range, side);
}
private lineOfInterestChanged() {
@@ -781,101 +634,33 @@
* This must be called once, but only after diff lines are rendered. Otherwise
* `processNodes()` will fail to lookup the HTML elements that it wants to
* manipulate.
+ *
+ * TODO: Validate whether the above comment is still true. We don't look up
+ * elements anymore, and processing the nodes earlier might be beneficial
+ * performance wise.
*/
private observeNodes() {
if (this.nodeObserver) return;
- // First stop observing old nodes.
- // Then introduce a Mutation observer that watches for children being added
- // to gr-diff. If those children are `isThreadEl`, namely then they are
- // processed.
- this.nodeObserver = new MutationObserver(mutations => {
- const addedThreadEls = extractAddedNodes(mutations).filter(isThreadEl);
- const removedThreadEls =
- extractRemovedNodes(mutations).filter(isThreadEl);
- this.processNodes(addedThreadEls, removedThreadEls);
- });
+ // Watches children being added to gr-diff. We are expecting only comment
+ // widgets to be direct children.
+ this.nodeObserver = new MutationObserver(() => this.processNodes());
this.nodeObserver.observe(this, {childList: true});
- // Make sure to process existing gr-comment-threads that already exist.
- this.processNodes([...this.childNodes].filter(isThreadEl), []);
+ // Process existing comment widgets before the first observed change.
+ this.processNodes();
}
- private processNodes(
- addedThreadEls: GrDiffThreadElement[],
- removedThreadEls: GrDiffThreadElement[]
- ) {
- this.diffModel.updateState({
- comments: [...this.childNodes]
- .filter(isHtmlElement)
- .map(toCommentThreadModel)
- .filter(isDefined)
- .sort(compareComments),
- });
- this.updateRanges(addedThreadEls, removedThreadEls);
- addedThreadEls.forEach(threadEl =>
- this.redispatchHoverEvents(threadEl, threadEl)
- );
- // Removed nodes do not need to be handled because all this code does is
- // adding a slot for the added thread elements, and the extra slots do
- // not hurt. It's probably a bigger performance cost to remove them than
- // to keep them around. Medium term we can even consider to add one slot
- // for each line from the start.
- for (const threadEl of addedThreadEls) {
- const lineNum = getLine(threadEl);
- const commentSide = getSide(threadEl);
- const range = getRange(threadEl);
- if (!commentSide) continue;
- const lineEl = this.getLineElByNumber(lineNum, commentSide);
- // When the line the comment refers to does not exist, log an error
- // but don't crash. This can happen e.g. if the API does not fully
- // validate e.g. (robot) comments
- if (!lineEl) {
- console.error(
- 'thread attached to line ',
- commentSide,
- lineNum,
- ' which does not exist.'
- );
- continue;
- }
- const contentEl = this.getContentTdByLineEl(lineEl);
- if (!contentEl) continue;
- if (lineNum === LOST) {
- this.insertPortedCommentsWithoutRangeMessage(contentEl);
- }
-
- const slotAtt = threadEl.getAttribute('slot');
- if (range && isLongCommentRange(range) && slotAtt) {
- const longRangeCommentHint = document.createElement(
- 'gr-ranged-comment-hint'
- );
- longRangeCommentHint.range = range;
- longRangeCommentHint.setAttribute('threadElRootId', threadEl.rootId);
- longRangeCommentHint.setAttribute('slot', slotAtt);
- this.insertBefore(longRangeCommentHint, threadEl);
- this.redispatchHoverEvents(longRangeCommentHint, threadEl);
- }
+ private processNodes() {
+ const threadEls = [...this.childNodes].filter(isThreadEl);
+ const comments = threadEls
+ .map(getDataFromCommentThreadEl)
+ .filter(isDefined)
+ .sort(compareComments);
+ this.diffModel.updateState({comments});
+ this.rangeLayer.updateRanges(comments);
+ for (const el of threadEls) {
+ el.addEventListener('mouseenter', this.commentThreadEnterRedispatcher);
+ el.addEventListener('mouseleave', this.commentThreadLeaveRedispatcher);
}
-
- for (const threadEl of removedThreadEls) {
- this.querySelector(
- `gr-ranged-comment-hint[threadElRootId="${threadEl.rootId}"]`
- )?.remove();
- }
- }
-
- private insertPortedCommentsWithoutRangeMessage(lostCell: Element) {
- const existingMessage = lostCell.querySelector('div.lost-message');
- if (existingMessage) return;
-
- const div = document.createElement('div');
- div.className = 'lost-message';
- const icon = document.createElement('gr-icon');
- icon.setAttribute('icon', 'info');
- div.appendChild(icon);
- const span = document.createElement('span');
- span.innerText = 'Original comment position not found in this patchset';
- div.appendChild(span);
- lostCell.insertBefore(div, lostCell.firstChild);
}
/** TODO: Can be removed when diff-old is gone. */
@@ -1174,14 +959,6 @@
}
}
-function extractAddedNodes(mutations: MutationRecord[]) {
- return mutations.flatMap(mutation => [...mutation.addedNodes]);
-}
-
-function extractRemovedNodes(mutations: MutationRecord[]) {
- return mutations.flatMap(mutation => [...mutation.removedNodes]);
-}
-
function getLineNumberCellWidth(prefs: DiffPreferencesInfo) {
return prefs.font_size * 4;
}
@@ -1217,8 +994,8 @@
'gr-diff': LitElement;
}
interface HTMLElementEventMap {
- 'comment-thread-mouseenter': CustomEvent<{}>;
- 'comment-thread-mouseleave': CustomEvent<{}>;
+ 'comment-thread-mouseenter': CustomEvent<GrDiffCommentThread>;
+ 'comment-thread-mouseleave': CustomEvent<GrDiffCommentThread>;
'loading-changed': ValueChangedEvent<boolean>;
'render-required': CustomEvent<{}>;
/**
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
index 48370411..917decc 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -22,7 +22,6 @@
Side,
} from '../../../api/diff';
import {
- mockPromise,
mouseDown,
queryAll,
stubBaseUrl,
@@ -188,43 +187,6 @@
assert.isFalse(element.classList.contains('no-left'));
});
- test('handleTap lineNum', async () => {
- const addDraftStub = sinon.stub(element, 'addDraftAtLine');
- const el = document.createElement('div');
- el.className = 'lineNum';
- const promise = mockPromise();
- el.addEventListener('click', e => {
- element.handleTap(e);
- assert.isTrue(addDraftStub.called);
- assert.equal(addDraftStub.lastCall.args[0], el);
- promise.resolve();
- });
- el.click();
- await promise;
- });
-
- test('handleTap content', async () => {
- const content = document.createElement('div');
- const lineEl = document.createElement('div');
- lineEl.className = 'lineNum';
- const row = document.createElement('div');
- row.appendChild(lineEl);
- row.appendChild(content);
-
- const selectStub = sinon.stub(element, 'selectLine');
-
- content.className = 'content';
- const promise = mockPromise();
- content.addEventListener('click', e => {
- element.handleTap(e);
- assert.isTrue(selectStub.called);
- assert.equal(selectStub.lastCall.args[0], lineEl);
- promise.resolve();
- });
- content.click();
- await promise;
- });
-
suite('getCursorStops', () => {
async function setupDiff() {
element.diff = createDiff();
@@ -286,26 +248,11 @@
});
suite('logged in', async () => {
- let fakeLineEl: HTMLElement;
setup(async () => {
element.loggedIn = true;
-
- fakeLineEl = {
- getAttribute: sinon.stub().returns(42),
- classList: {
- contains: sinon.stub().returns(true),
- },
- } as unknown as HTMLElement;
await element.updateComplete;
});
- test('addDraftAtLine', () => {
- sinon.stub(element, 'selectLine');
- const createCommentStub = sinon.stub(element, 'createComment');
- element.addDraftAtLine(fakeLineEl);
- assert.isTrue(createCommentStub.calledWithExactly(fakeLineEl, 42));
- });
-
test('adds long range comment hint', async () => {
const range = {
start_line: 1,
@@ -372,35 +319,6 @@
1
);
});
-
- test('removes long range comment hint when comment is discarded', async () => {
- const range = {
- start_line: 1,
- end_line: 7,
- start_character: 0,
- end_character: 0,
- };
- const threadEl = document.createElement('div');
- threadEl.className = 'comment-thread';
- threadEl.setAttribute('diff-side', 'right');
- threadEl.setAttribute('line-num', '1');
- threadEl.setAttribute('range', JSON.stringify(range));
- threadEl.setAttribute('slot', 'right-1');
- const content = [
- {
- ab: Array(8).fill('text'),
- },
- ];
- await setupSampleDiff({content});
-
- element.appendChild(threadEl);
- await waitUntil(() => element.commentRanges.length === 1);
-
- threadEl.remove();
- await waitUntil(() => element.commentRanges.length === 0);
-
- assert.isEmpty(element.querySelectorAll('gr-ranged-comment-hint'));
- });
});
suite('blame', () => {
diff --git a/polygerrit-ui/app/embed/diff/gr-ranged-comment-hint/gr-ranged-comment-hint.ts b/polygerrit-ui/app/embed/diff/gr-ranged-comment-hint/gr-ranged-comment-hint.ts
index d7883a0..906c130 100644
--- a/polygerrit-ui/app/embed/diff/gr-ranged-comment-hint/gr-ranged-comment-hint.ts
+++ b/polygerrit-ui/app/embed/diff/gr-ranged-comment-hint/gr-ranged-comment-hint.ts
@@ -5,7 +5,7 @@
*/
import '../gr-range-header/gr-range-header';
import {CommentRange} from '../../../types/common';
-import {LitElement, css, html} from 'lit';
+import {LitElement, css, html, nothing} from 'lit';
import {customElement, property} from 'lit/decorators.js';
import {sharedStyles} from '../../../styles/shared-styles';
import {grRangedCommentTheme} from '../gr-ranged-comment-themes/gr-ranged-comment-theme';
@@ -32,16 +32,13 @@
}
override render() {
- return html`<div class="rangeHighlight row">
- <gr-range-header icon="mode_comment" filled
- >${this._computeRangeLabel(this.range)}</gr-range-header
- >
- </div>`;
- }
-
- _computeRangeLabel(range?: CommentRange): string {
- if (!range) return '';
- return `Long comment range ${range.start_line} - ${range.end_line}`;
+ if (!this.range) return nothing;
+ const text = `Long comment range ${this.range.start_line} - ${this.range.end_line}`;
+ return html`
+ <div class="rangeHighlight row">
+ <gr-range-header icon="mode_comment" filled>${text}</gr-range-header>
+ </div>
+ `;
}
}
diff --git a/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts b/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
index 24729ff9..98acc5a 100644
--- a/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
+++ b/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
@@ -9,20 +9,22 @@
import {Side} from '../../../constants/constants';
import {CommentRange} from '../../../types/common';
import {DiffLayer, DiffLayerListener} from '../../../types/types';
-import {isLongCommentRange} from '../gr-diff/gr-diff-utils';
+import {
+ GrDiffCommentThread,
+ isLongCommentRange,
+} from '../gr-diff/gr-diff-utils';
import {GrDiffLineType} from '../../../api/diff';
-/**
- * Enhanced CommentRange by UI state. Interface for incoming ranges set from the
- * outside.
- *
- * TODO(TS): Unify with what is used in gr-diff when these objects are created.
- */
-export interface CommentRangeLayer {
+/** Force `range` to be set for the objects that we are working with. */
+export interface CommentRangeLayer extends Partial<GrDiffCommentThread> {
side: Side;
range: CommentRange;
- // New drafts don't have a rootId.
- rootId?: string;
+}
+
+function isCommentRangeLayer<T extends Partial<GrDiffCommentThread>>(
+ x: T | CommentRangeLayer | undefined
+): x is CommentRangeLayer {
+ return !!x && !!(x as CommentRangeLayer).range;
}
/** Can be used for array functions like `some()`. */
@@ -124,7 +126,9 @@
}
}
- updateRanges(newRanges: CommentRangeLayer[]) {
+ updateRanges(rawRanges: Partial<GrDiffCommentThread>[]) {
+ const newRanges: CommentRangeLayer[] =
+ rawRanges.filter(isCommentRangeLayer);
for (const newRange of newRanges) {
if (this.knownRanges.some(equals(newRange))) continue;
this.addRange(newRange);
diff --git a/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.ts b/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.ts
index 5bfd94d..4de6d01 100644
--- a/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.ts
@@ -18,6 +18,7 @@
const rangeA: CommentRangeLayer = {
side: Side.LEFT,
+ line: 39,
range: {
end_character: 9,
end_line: 39,
@@ -29,6 +30,7 @@
const rangeB: CommentRangeLayer = {
side: Side.RIGHT,
+ line: 12,
range: {
end_character: 22,
end_line: 12,
@@ -40,6 +42,7 @@
const rangeC: CommentRangeLayer = {
side: Side.RIGHT,
+ line: 100,
range: {
end_character: 15,
end_line: 100,
@@ -50,6 +53,7 @@
const rangeD: CommentRangeLayer = {
side: Side.RIGHT,
+ line: 55,
range: {
end_character: 2,
end_line: 55,
@@ -61,6 +65,7 @@
const rangeE: CommentRangeLayer = {
side: Side.RIGHT,
+ line: 71,
range: {
end_character: 1,
end_line: 71,
@@ -71,6 +76,7 @@
const rangeF: CommentRangeLayer = {
side: Side.RIGHT,
+ line: 24,
range: {
end_character: 0,
end_line: 24,
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 06f6321..a0360ef 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -1147,7 +1147,6 @@
undefined,
listChangesOptionsToHex(
ListChangesOption.CHANGE_ACTIONS,
- ListChangesOption.CURRENT_ACTIONS,
ListChangesOption.CURRENT_REVISION,
ListChangesOption.DETAILED_LABELS,
// TODO: remove this option and merge requirements from dashboard req
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
index 764aa98..696e142 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
@@ -67,7 +67,9 @@
const EXPECTED_QUERY_OPTIONS = listChangesOptionsToHex(
ListChangesOption.CHANGE_ACTIONS,
- ListChangesOption.CURRENT_ACTIONS,
+ // Current actions can be costly to calculate (e.g submit action)
+ // They are not used in bulk actions.
+ // ListChangesOption.CURRENT_ACTIONS,
ListChangesOption.CURRENT_REVISION,
ListChangesOption.DETAILED_LABELS,
ListChangesOption.SUBMIT_REQUIREMENTS
diff --git a/polygerrit-ui/app/test/test-app-context-init.ts b/polygerrit-ui/app/test/test-app-context-init.ts
index 73ee6aa..da6a92d 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -53,6 +53,6 @@
highlightServiceToken,
() => new MockHighlightService(appContext.reportingService)
);
- dependencies.set(diffModelToken, () => new DiffModel());
+ dependencies.set(diffModelToken, () => new DiffModel(document));
return dependencies;
}
diff --git a/polygerrit-ui/app/utils/diff-util.ts b/polygerrit-ui/app/utils/diff-util.ts
index 28c22e5..0d52bd7 100644
--- a/polygerrit-ui/app/utils/diff-util.ts
+++ b/polygerrit-ui/app/utils/diff-util.ts
@@ -10,6 +10,10 @@
// syntax highlighting for the entire file.
export const SYNTAX_MAX_LINE_LENGTH = 500;
+export function otherSide(side: Side) {
+ return side === Side.LEFT ? Side.RIGHT : Side.LEFT;
+}
+
export function countLines(diff?: DiffInfo, side?: Side) {
if (!diff?.content || !side) return 0;
return diff.content.reduce((sum, chunk) => {
diff --git a/tools/deps.bzl b/tools/deps.bzl
index 7d4499a..592e4a7 100644
--- a/tools/deps.bzl
+++ b/tools/deps.bzl
@@ -3,7 +3,7 @@
CAFFEINE_VERS = "2.9.2"
ANTLR_VERS = "3.5.2"
COMMONMARK_VERSION = "0.21.0"
-FLEXMARK_VERS = "0.50.50"
+FLEXMARK_VERS = "0.64.0"
GREENMAIL_VERS = "1.5.5"
MAIL_VERS = "1.6.0"
MIME4J_VERS = "0.8.1"
@@ -191,157 +191,183 @@
maven_jar(
name = "gfm-tables",
artifact = "org.commonmark:commonmark-ext-gfm-tables:" + COMMONMARK_VERSION,
+ attach_source = False,
sha1 = "fb7d65fa89a4cfcd2f51535d2549b570cf1dbd1a",
)
maven_jar(
name = "flexmark",
artifact = "com.vladsch.flexmark:flexmark:" + FLEXMARK_VERS,
- sha1 = "7f61e190cff7e1bea64906408ccfa583b5ac9fc2",
+ sha1 = "bb5fcdf1335a35c4c0285fee2683a32e6a70cd59",
)
maven_jar(
name = "flexmark-ext-abbreviation",
artifact = "com.vladsch.flexmark:flexmark-ext-abbreviation:" + FLEXMARK_VERS,
- sha1 = "8f62f49cfaf8d33391e48a3b79e941d94e444e50",
+ sha1 = "48a64d5d1e5d1e0f8efde75f051b16800952eeb3",
)
maven_jar(
name = "flexmark-ext-anchorlink",
artifact = "com.vladsch.flexmark:flexmark-ext-anchorlink:" + FLEXMARK_VERS,
- sha1 = "1d530e44b4439abf53ce9dcc784ffa5b9fd6ce89",
+ sha1 = "e263a138896153bad6b69828d65c409fd99292a8",
)
maven_jar(
name = "flexmark-ext-autolink",
artifact = "com.vladsch.flexmark:flexmark-ext-autolink:" + FLEXMARK_VERS,
- sha1 = "4026aa60fd6e146c2d4931acb19b2409c6a79b8a",
+ sha1 = "e380ee48d5a779d08f071aaaf9a69e5985f24802",
)
maven_jar(
name = "flexmark-ext-definition",
artifact = "com.vladsch.flexmark:flexmark-ext-definition:" + FLEXMARK_VERS,
- sha1 = "bb74d36459e8e34653761aaa0095220b8b7b6cb6",
+ sha1 = "892d83daafb1255489377a4331c6b04cd9fef288",
)
maven_jar(
name = "flexmark-ext-emoji",
artifact = "com.vladsch.flexmark:flexmark-ext-emoji:" + FLEXMARK_VERS,
- sha1 = "36d36cb227f831b81b636d3f901f07db06b8d84d",
+ sha1 = "63777b6b82456fb08587537db949c67ac6ac5d66",
)
maven_jar(
name = "flexmark-ext-escaped-character",
artifact = "com.vladsch.flexmark:flexmark-ext-escaped-character:" + FLEXMARK_VERS,
- sha1 = "09f0cef3260b6f6371d6066cf32ce6a7dc2a7922",
+ sha1 = "19b48848dd0d242c8dd527113cf91711847cf7f1",
)
maven_jar(
name = "flexmark-ext-footnotes",
artifact = "com.vladsch.flexmark:flexmark-ext-footnotes:" + FLEXMARK_VERS,
- sha1 = "cdf0b79f026b09c6b87d91e61eb29ada1577aa5c",
+ sha1 = "a885e4c512f6f73b3ea02a26f71e0116e720b33e",
)
maven_jar(
name = "flexmark-ext-gfm-issues",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-issues:" + FLEXMARK_VERS,
- sha1 = "e40d347e242e35d4344553120cb9e69c1c975f41",
+ sha1 = "f1f2e840cd295bfaad006e7bddb715edfd747e44",
)
maven_jar(
name = "flexmark-ext-gfm-strikethrough",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-strikethrough:" + FLEXMARK_VERS,
- sha1 = "a6948f6e4fc2ec1059d6b53e73d4a30c24f6e05d",
- )
-
- maven_jar(
- name = "flexmark-ext-gfm-tables",
- artifact = "com.vladsch.flexmark:flexmark-ext-gfm-tables:" + FLEXMARK_VERS,
- sha1 = "92d3eb0c5dc79924448c186d89717f1df853aaa0",
+ sha1 = "9e4d01143752ebe98d45ac29c77e17fa8cc8b947",
)
maven_jar(
name = "flexmark-ext-gfm-tasklist",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-tasklist:" + FLEXMARK_VERS,
- sha1 = "e6fb4d9e46c96e61a07fbb40cf653db58ed95a95",
+ sha1 = "ad5d3dda8a17100ee01606386aacb73d41e35764",
)
maven_jar(
name = "flexmark-ext-gfm-users",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-users:" + FLEXMARK_VERS,
- sha1 = "44c83bdccfd41a399f3f7b11ba72728382dd3f5a",
+ sha1 = "59846560518f6ef5282d333e1ba1a68dc5ea8c43",
)
maven_jar(
name = "flexmark-ext-ins",
artifact = "com.vladsch.flexmark:flexmark-ext-ins:" + FLEXMARK_VERS,
- sha1 = "b0d174d86ac2348420993dc2c997fad5f02c68fa",
+ sha1 = "b5a7e72e337d31e2956f662b37ec1f47289e7751",
)
maven_jar(
name = "flexmark-ext-jekyll-front-matter",
artifact = "com.vladsch.flexmark:flexmark-ext-jekyll-front-matter:" + FLEXMARK_VERS,
- sha1 = "2bfb31c67fd10af058b90f1b364f881f6276de80",
+ sha1 = "5543c826f325444edacc6920992753568aa046f6",
)
maven_jar(
name = "flexmark-ext-superscript",
artifact = "com.vladsch.flexmark:flexmark-ext-superscript:" + FLEXMARK_VERS,
- sha1 = "d210b007b46339082f79b6caf632b19ac8efc341",
+ sha1 = "bcee70d603fb7acf9ef490036b14a5c1cb3f533c",
)
maven_jar(
name = "flexmark-ext-tables",
artifact = "com.vladsch.flexmark:flexmark-ext-tables:" + FLEXMARK_VERS,
- sha1 = "be77790470aa9bd90011067221504162a1d7b083",
+ sha1 = "52b829753bb928cd3f26c43d39639eb95b3f9f88",
)
maven_jar(
name = "flexmark-ext-toc",
artifact = "com.vladsch.flexmark:flexmark-ext-toc:" + FLEXMARK_VERS,
- sha1 = "91d6d63ff5b70c3ebae98867bd7a346d71cb0ce1",
+ sha1 = "69b96da85758688a564babea371fc9268a11d718",
)
maven_jar(
name = "flexmark-ext-typographic",
artifact = "com.vladsch.flexmark:flexmark-ext-typographic:" + FLEXMARK_VERS,
- sha1 = "f51e247df80628df509a3af92a1efa1efb83d746",
+ sha1 = "5e2cb29da6c43eecffbd34a4f69976f61a383e97",
)
maven_jar(
name = "flexmark-ext-wikilink",
artifact = "com.vladsch.flexmark:flexmark-ext-wikilink:" + FLEXMARK_VERS,
- sha1 = "d14aeb078dbaf3e166621ae9499595a4a57b22ab",
+ sha1 = "5ada81fecf0d68cdcd5c763da0f5f22aa885d559",
)
maven_jar(
name = "flexmark-ext-yaml-front-matter",
artifact = "com.vladsch.flexmark:flexmark-ext-yaml-front-matter:" + FLEXMARK_VERS,
- sha1 = "aba328b70e15f2553aac0a74e391edab37bf7b30",
- )
-
- maven_jar(
- name = "flexmark-formatter",
- artifact = "com.vladsch.flexmark:flexmark-formatter:" + FLEXMARK_VERS,
- sha1 = "3e4ad3b1be29d41e8c35cadb70761768505f2164",
- )
-
- maven_jar(
- name = "flexmark-html-parser",
- artifact = "com.vladsch.flexmark:flexmark-html-parser:" + FLEXMARK_VERS,
- sha1 = "3c59b0061c50c9a8a48c46d4f4498d8eba249921",
+ sha1 = "377b29278cad7603aa3ca705c31a9f48cb6f8fb9",
)
maven_jar(
name = "flexmark-profile-pegdown",
artifact = "com.vladsch.flexmark:flexmark-profile-pegdown:" + FLEXMARK_VERS,
- sha1 = "c01a2c2eebe06230858956d45847cee233790d7c",
+ sha1 = "c67446ffd3653416ed26f924ba99f8125fb74791",
)
maven_jar(
- name = "flexmark-util",
- artifact = "com.vladsch.flexmark:flexmark-util:" + FLEXMARK_VERS,
- sha1 = "fdfce72f5eb9ec53085804fd0c8d15f3680ae359",
+ name = "flexmark-util-data",
+ artifact = "com.vladsch.flexmark:flexmark-util-data:" + FLEXMARK_VERS,
+ attach_source = False,
+ sha1 = "63162607faa7c98f6e50a9d1e826004c5475841d",
+ )
+
+ maven_jar(
+ name = "flexmark-util-ast",
+ artifact = "com.vladsch.flexmark:flexmark-util-ast:" + FLEXMARK_VERS,
+ attach_source = False,
+ sha1 = "10fff87e61b7c3bb12afddf7b418977cd02acdc8",
+ )
+
+ maven_jar(
+ name = "flexmark-util-misc",
+ artifact = "com.vladsch.flexmark:flexmark-util-misc:" + FLEXMARK_VERS,
+ attach_source = False,
+ sha1 = "d2129f4f2b55fbf645e3499c7b0cdddcfef81112",
+ )
+
+ maven_jar(
+ name = "flexmark-util-visitor",
+ artifact = "com.vladsch.flexmark:flexmark-util-visitor:" + FLEXMARK_VERS,
+ attach_source = False,
+ sha1 = "7eb0030ae2a2eec80d74790caeb4cdb6b4bf8e17",
+ )
+
+ maven_jar(
+ name = "flexmark-util-builder",
+ artifact = "com.vladsch.flexmark:flexmark-util-builder:" + FLEXMARK_VERS,
+ attach_source = False,
+ sha1 = "2d6adaf6053f72de86a050fe6967049f7c2d3500",
+ )
+
+ maven_jar(
+ name = "flexmark-util-sequence",
+ artifact = "com.vladsch.flexmark:flexmark-util-sequence:" + FLEXMARK_VERS,
+ attach_source = False,
+ sha1 = "299e3b4a4272ba9dcd6b9081b8d24d1a672a0921",
+ )
+
+ maven_jar(
+ name = "flexmark-util-html",
+ artifact = "com.vladsch.flexmark:flexmark-util-html:" + FLEXMARK_VERS,
+ attach_source = False,
+ sha1 = "fc52f860cf45f57468d8f14ee63c1e6469ee3f47",
)
# Transitive dependency of flexmark and gitiles
