diff options
13 files changed, 177 insertions, 292 deletions
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 844f4c6182..8aa5c7f669 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -2916,6 +2916,11 @@ Adds and/or removes custom keyed values from a change. The custom keyed values to add or remove must be provided in the request body inside a link:#custom-keyed-values-input[CustomKeyedValuesInput] entity. +Note that custom keyed values are expected to be small in both key and value. +A typical use-case would be storing the ID to some external system, in which +case the key would be something unique to that system and the value would be +the ID. + .Request ---- POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/custom_keyed_values HTTP/1.0 diff --git a/java/com/google/gerrit/server/git/TagSet.java b/java/com/google/gerrit/server/git/TagSet.java index a528c8fd60..bffc479c4f 100644 --- a/java/com/google/gerrit/server/git/TagSet.java +++ b/java/com/google/gerrit/server/git/TagSet.java @@ -14,6 +14,8 @@ package com.google.gerrit.server.git; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; @@ -41,6 +43,7 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevFlag; import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevWalk; import org.roaringbitmap.RoaringBitmap; @@ -209,6 +212,7 @@ class TagSet { try (TagWalk rw = new TagWalk(git)) { rw.setRetainBody(false); + RevFlag isTag = rw.newFlag("tag"); for (Ref ref : git.getRefDatabase() .getRefsByPrefixWithExclusions(RefDatabase.ALL, SKIPPABLE_REF_PREFIXES)) { @@ -218,9 +222,9 @@ class TagSet { } else if (isTag(ref)) { // For a tag, remember where it points to. try { - addTag(rw, git.getRefDatabase().peel(ref)); + addTag(rw, git.getRefDatabase().peel(ref), isTag); } catch (IOException e) { - addTag(rw, ref); + addTag(rw, ref, isTag); } } else { @@ -229,17 +233,10 @@ class TagSet { } } - // Traverse the complete history. Copy any flags from a commit to - // all of its ancestors. This automatically updates any Tag object - // as the TagCommit and the stored Tag object share the same - // underlying bit set. + // Traverse the complete history and propagate reachability to parents. TagCommit c; while ((c = (TagCommit) rw.next()) != null) { - RoaringBitmap mine = c.refFlags; - int pCnt = c.getParentCount(); - for (int pIdx = 0; pIdx < pCnt; pIdx++) { - ((TagCommit) c.getParent(pIdx)).refFlags.or(mine); - } + c.propagateReachabilityToParents(isTag); } } catch (IOException e) { logger.atWarning().withCause(e).log("Error building tags for repository %s", projectName); @@ -356,9 +353,7 @@ class TagSet { refs.putAll(old.refs); for (Tag srcTag : old.tags) { - RoaringBitmap mine = new RoaringBitmap(); - mine.or(srcTag.refFlags); - tags.add(new Tag(srcTag, mine)); + tags.add(new Tag(srcTag)); } for (TagMatcher.LostRef lost : m.lostRefs) { @@ -369,7 +364,7 @@ class TagSet { } } - private void addTag(TagWalk rw, Ref ref) { + private void addTag(TagWalk rw, Ref ref, RevFlag isTag) { ObjectId id = ref.getPeeledObjectId(); if (id == null) { id = ref.getObjectId(); @@ -378,7 +373,12 @@ class TagSet { if (!tags.contains(id)) { RoaringBitmap flags; try { - flags = ((TagCommit) rw.parseCommit(id)).refFlags; + TagCommit commit = ((TagCommit) rw.parseCommit(id)); + commit.add(isTag); + if (commit.refFlags == null) { + commit.refFlags = new RoaringBitmap(); + } + flags = commit.refFlags; } catch (IncorrectObjectTypeException notCommit) { flags = new RoaringBitmap(); } catch (IOException e) { @@ -395,6 +395,9 @@ class TagSet { rw.markStart(commit); int flag = refs.size(); + if (commit.refFlags == null) { + commit.refFlags = new RoaringBitmap(); + } commit.refFlags.add(flag); refs.put(ref.getName(), new CachedRef(ref, flag)); } catch (IncorrectObjectTypeException notCommit) { @@ -432,8 +435,13 @@ class TagSet { // RoaringBitmap in TagCommit.refFlags. @VisibleForTesting final RoaringBitmap refFlags; + Tag(Tag src) { + this(src, src.refFlags.clone()); + } + Tag(AnyObjectId id, RoaringBitmap flags) { super(id); + checkNotNull(flags); this.refFlags = flags; } @@ -485,13 +493,50 @@ class TagSet { } // TODO(hanwen): this would be better named as CommitWithReachability, as it also holds non-tags. + // However, non-tags will have a null refFlags field. private static final class TagCommit extends RevCommit { /** CachedRef.flag => isVisible, indicating if this commit is reachable from the ref. */ - final RoaringBitmap refFlags; + RoaringBitmap refFlags; TagCommit(AnyObjectId id) { super(id); - refFlags = new RoaringBitmap(); + } + + /** + * Copy any flags from this commit to all of its ancestors. + * + * <p>Do not maintain a reference to the flags on non-tag commits after copying their flags to + * their ancestors. The flag copying automatically updates any Tag object as the TagCommit and + * the stored Tag object share the same underlying RoaringBitmap. + * + * @param isTag {@code RevFlag} indicating if this TagCommit is a tag + */ + void propagateReachabilityToParents(RevFlag isTag) { + RoaringBitmap mine = refFlags; + if (mine != null) { + boolean canMoveBitmap = false; + if (!has(isTag)) { + refFlags = null; + canMoveBitmap = true; + } + int pCnt = getParentCount(); + for (int pIdx = 0; pIdx < pCnt; pIdx++) { + TagCommit commit = (TagCommit) getParent(pIdx); + RoaringBitmap parentFlags = commit.refFlags; + if (parentFlags == null) { + if (canMoveBitmap) { + // This commit is not itself a Tag, so in order to reduce cloning overhead, migrate + // its refFlags object to its first parent with null refFlags + commit.refFlags = mine; + canMoveBitmap = false; + } else { + commit.refFlags = mine.clone(); + } + } else { + parentFlags.or(mine); + } + } + } } } } diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java index 9e85d8ce73..c0531e513b 100644 --- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java +++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java @@ -875,6 +875,47 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/tags/new-tag"); } + // rcMaster (c1 master master-tag) <- rcBranch (c2 branch branch-tag) <- rcBranch (c2 branch) <- + // newcommit1 <- newcommit2 (new-branch) + @Test + public void uploadPackReachableTagVisibleFromLeafBranch() throws Exception { + try (Repository repo = repoManager.openRepository(project)) { + // rcBranch (c2 branch) <- newcommit1 (new-branch) + PushOneCommit.Result r = + pushFactory + .create(admin.newIdent(), testRepo) + .setParent(rcBranch) + .to("refs/heads/new-branch"); + r.assertOkStatus(); + RevCommit branchRc = r.getCommit(); + + // rcBranch (c2) <- newcommit1 <- newcommit2 (new-branch) + r = + pushFactory + .create(admin.newIdent(), testRepo) + .setParent(branchRc) + .to("refs/heads/new-branch"); + r.assertOkStatus(); + } + + projectOperations + .project(project) + .forUpdate() + .add(deny(Permission.READ).ref("refs/heads/master").group(REGISTERED_USERS)) + .add(deny(Permission.READ).ref("refs/heads/branch").group(REGISTERED_USERS)) + .add(allow(Permission.READ).ref("refs/heads/new-branch").group(REGISTERED_USERS)) + .update(); + + requestScopeOperations.setApiUser(user.id()); + assertUploadPackRefs( + "refs/heads/new-branch", + // 'master' and 'branch' branches are not visible but 'master-tag' and 'branch-tag' are + // reachable from new-branch (since PushOneCommit always bases changes on each other). + "refs/tags/branch-tag", + "refs/tags/master-tag"); + // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead. + } + // first ls-remote: rcBranch (c2 branch) <- newcommit1 (updated-tag) // second ls-remote: rcBranch (c2 branch updated-tag) @Test diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts index 57bb8757f4..aa569adb62 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts @@ -1025,23 +1025,17 @@ export class GrChangeActions } private actionsChanged() { - this.hidden = - Object.keys(this.actions).length === 0 && - Object.keys(this.revisionActions).length === 0 && - this.additionalActions.length === 0; this.actionLoadingMessage = ''; this.disabledMenuActions = []; - if (Object.keys(this.revisionActions).length !== 0) { - if (!this.revisionActions.download) { - this.revisionActions = { - ...this.revisionActions, - download: DOWNLOAD_ACTION, - }; - fire(this, 'revision-actions-changed', { - value: this.revisionActions, - }); - } + if (!this.revisionActions.download) { + this.revisionActions = { + ...this.revisionActions, + download: DOWNLOAD_ACTION, + }; + fire(this, 'revision-actions-changed', { + value: this.revisionActions, + }); } if ( !this.actions.includedIn && diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts index ae6044912d..f0836093f1 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts @@ -390,7 +390,7 @@ suite('gr-change-view tests', () => { </gr-copy-clipboard> </div> <div class="commitActions"> - <gr-change-actions hidden="" id="actions"> </gr-change-actions> + <gr-change-actions id="actions"> </gr-change-actions> </div> </div> <h2 class="assistive-tech-only">Change metadata</h2> diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts index d053928d26..97b8de3913 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts @@ -835,10 +835,14 @@ export class GrFileList extends LitElement { if (changedProperties.has('files')) { this.filesChanged(); this.numFilesShown = Math.min(this.files.length, DEFAULT_NUM_FILES_SHOWN); + fire(this, 'files-shown-changed', {length: this.numFilesShown}); } if (changedProperties.has('expandedFiles')) { this.expandedFilesChanged(changedProperties.get('expandedFiles')); } + if (changedProperties.has('numFilesShown')) { + fire(this, 'files-shown-changed', {length: this.numFilesShown}); + } } override connectedCallback() { diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts index bd81828907..5fcafc3ff8 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts @@ -395,7 +395,6 @@ export class GrSettingsView extends LitElement { this.serverConfig?.auth.use_contributor_agreements, () => html`<li><a href="#Agreements">Agreements</a></li>` )} - <li><a href="#MailFilters">Mail Filters</a></li> <gr-endpoint-decorator name="settings-menu-item"> </gr-endpoint-decorator> </ul> @@ -680,91 +679,6 @@ export class GrSettingsView extends LitElement { <gr-agreements-list id="agreementsList"></gr-agreements-list> </fieldset>` )} - <h2 id="MailFilters">Mail Filters</h2> - <fieldset class="filters"> - <p> - Gerrit emails include metadata about the change to support writing - mail filters. - </p> - <p> - Here are some example Gmail queries that can be used for filters - or for searching through archived messages. View the - <a - href=${this.getFilterDocsLink(this.docsBaseUrl)} - target="_blank" - rel="noopener noreferrer" - >Gerrit documentation</a - > - for the complete set of footers. - </p> - <table> - <tbody> - <tr> - <th>Name</th> - <th>Query</th> - </tr> - <tr> - <td>Changes requesting my review</td> - <td> - <code class="queryExample"> - "Gerrit-Reviewer: <em>Your Name</em> - <<em>your.email@example.com</em>>" - </code> - </td> - </tr> - <tr> - <td>Changes requesting my attention</td> - <td> - <code class="queryExample"> - "Gerrit-Attention: <em>Your Name</em> - <<em>your.email@example.com</em>>" - </code> - </td> - </tr> - <tr> - <td>Changes from a specific owner</td> - <td> - <code class="queryExample"> - "Gerrit-Owner: <em>Owner name</em> - <<em>owner.email@example.com</em>>" - </code> - </td> - </tr> - <tr> - <td>Changes targeting a specific branch</td> - <td> - <code class="queryExample"> - "Gerrit-Branch: <em>branch-name</em>" - </code> - </td> - </tr> - <tr> - <td>Changes in a specific project</td> - <td> - <code class="queryExample"> - "Gerrit-Project: <em>project-name</em>" - </code> - </td> - </tr> - <tr> - <td>Messages related to a specific Change ID</td> - <td> - <code class="queryExample"> - "Gerrit-Change-Id: <em>Change ID</em>" - </code> - </td> - </tr> - <tr> - <td>Messages related to a specific change number</td> - <td> - <code class="queryExample"> - "Gerrit-Change-Number: <em>change number</em>" - </code> - </td> - </tr> - </tbody> - </table> - </fieldset> <gr-endpoint-decorator name="settings-screen"> </gr-endpoint-decorator> </div> diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts index d77d35cda2..30a2922922 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts @@ -148,7 +148,6 @@ suite('gr-settings-view tests', () => { <li><a href="#EmailAddresses"> Email Addresses </a></li> <li><a href="#Groups"> Groups </a></li> <li><a href="#Identities"> Identities </a></li> - <li><a href="#MailFilters"> Mail Filters </a></li> <gr-endpoint-decorator name="settings-menu-item"> </gr-endpoint-decorator> </ul> @@ -454,92 +453,6 @@ suite('gr-settings-view tests', () => { <fieldset> <gr-identities id="identities"> </gr-identities> </fieldset> - <h2 id="MailFilters">Mail Filters</h2> - <fieldset class="filters"> - <p> - Gerrit emails include metadata about the change to support writing - mail filters. - </p> - <p> - Here are some example Gmail queries that can be used for filters - or for searching through archived messages. View the - <a - href="https://test.com/user-notify.html" - rel="noopener noreferrer" - target="_blank" - > - Gerrit documentation - </a> - for the complete set of footers. - </p> - <table> - <tbody> - <tr> - <th>Name</th> - <th>Query</th> - </tr> - <tr> - <td>Changes requesting my review</td> - <td> - <code class="queryExample"> - "Gerrit-Reviewer: <em> Your Name </em> < - <em> your.email@example.com </em> >" - </code> - </td> - </tr> - <tr> - <td>Changes requesting my attention</td> - <td> - <code class="queryExample"> - "Gerrit-Attention: <em> Your Name </em> < - <em> your.email@example.com </em> >" - </code> - </td> - </tr> - <tr> - <td>Changes from a specific owner</td> - <td> - <code class="queryExample"> - "Gerrit-Owner: <em> Owner name </em> < - <em> owner.email@example.com </em> >" - </code> - </td> - </tr> - <tr> - <td>Changes targeting a specific branch</td> - <td> - <code class="queryExample"> - "Gerrit-Branch: <em> branch-name </em> " - </code> - </td> - </tr> - <tr> - <td>Changes in a specific project</td> - <td> - <code class="queryExample"> - "Gerrit-Project: <em> project-name </em> " - </code> - </td> - </tr> - <tr> - <td>Messages related to a specific Change ID</td> - <td> - <code class="queryExample"> - "Gerrit-Change-Id: <em> Change ID </em> " - </code> - </td> - </tr> - <tr> - <td>Messages related to a specific change number</td> - <td> - <code class="queryExample"> - "Gerrit-Change-Number: <em> change number </em> " - </code> - </td> - </tr> - </tbody> - </table> - </fieldset> <gr-endpoint-decorator name="settings-screen"> </gr-endpoint-decorator> </div> diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts index d27be9fc5f..e2229b5ce7 100644 --- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts +++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts @@ -1187,7 +1187,10 @@ export class GrComment extends LitElement { async createSuggestEdit(e: MouseEvent) { e.stopPropagation(); const line = await this.getCommentedCode(); - this.messageText += `${USER_SUGGESTION_START_PATTERN}${line}${'\n```'}`; + const addNewLine = this.messageText.length !== 0; + this.messageText += `${ + addNewLine ? '\n' : '' + }${USER_SUGGESTION_START_PATTERN}${line}${'\n```'}`; } async getCommentedCode() { diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts index 70e653afb2..e557ca8bea 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts @@ -147,20 +147,17 @@ suite('gr-change-actions-js-api-interface tests', () => { test('move action button to overflow', async () => { const key = changeActions.add(ActionType.REVISION, 'Bork!'); await element.updateComplete; - assert.isTrue(queryAndAssert<GrDropdown>(element, '#moreActions').hidden); - assert.isOk( - queryAndAssert<GrButton>(element, `[data-action-key="${key}"]`) - ); + + let items = queryAndAssert<GrDropdown>(element, '#moreActions').items; + assert.isFalse(items?.some(item => item.name === 'Bork!')); + assert.isOk(query<GrButton>(element, `[data-action-key="${key}"]`)); + changeActions.setActionOverflow(ActionType.REVISION, key, true); await element.updateComplete; + + items = queryAndAssert<GrDropdown>(element, '#moreActions').items; + assert.isTrue(items?.some(item => item.name === 'Bork!')); assert.isNotOk(query<GrButton>(element, `[data-action-key="${key}"]`)); - assert.isFalse( - queryAndAssert<GrDropdown>(element, '#moreActions').hidden - ); - assert.strictEqual( - queryAndAssert<GrDropdown>(element, '#moreActions').items![0].name, - 'Bork!' - ); }); test('change actions priority', async () => { diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts index c29417e0c2..4afdf00742 100644 --- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts +++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts @@ -122,9 +122,11 @@ export class GrTextarea extends LitElement { private changeNum?: NumericChangeId; + // Represents the current location of the ':' or '@' that triggered a drop-down. // private but used in tests specialCharIndex = -1; + // Represents the current search string being used to query either emoji or mention suggestions. // private but used in tests currentSearchString?: string; @@ -280,8 +282,7 @@ export class GrTextarea extends LitElement { this.fireChangedEvents(); // Add to updated because we want this.textarea.selectionStart and // this.textarea is null in the willUpdate lifecycle - this.computeSpecialCharIndex(); - this.computeCurrentSearchString(); + this.computeIndexAndSearchString(); this.handleTextChanged(); } } @@ -373,6 +374,13 @@ export class GrTextarea extends LitElement { return; } + const selection = this.getVisibleDropdown().getCurrentText(); + if (selection === '') { + // Nothing was selected, so treat this like a newline and reset the dropdown. + this.indent(e); + this.resetDropdown(); + return; + } e.preventDefault(); e.stopPropagation(); this.setValue(this.getVisibleDropdown().getCurrentText()); @@ -402,17 +410,17 @@ export class GrTextarea extends LitElement { // below needs to happen after iron-autogrow-textarea has set the // incorrect value. await this.updateComplete; - this.textarea!.selectionStart = specialCharIndex + 1; - this.textarea!.selectionEnd = specialCharIndex + 1; + this.textarea!.selectionStart = specialCharIndex + text.length + 1; + this.textarea!.selectionEnd = specialCharIndex + text.length + 1; this.resetDropdown(); } private addValueToText(value: string) { if (!this.text) return ''; return ( - this.text.substr(0, this.specialCharIndex ?? 0) + + this.text.substring(0, this.specialCharIndex ?? 0) + value + - this.text.substr(this.textarea!.selectionStart) + this.text.substring(this.textarea!.selectionStart) ); } @@ -425,7 +433,7 @@ export class GrTextarea extends LitElement { */ updateCaratPosition() { if (typeof this.textarea!.value === 'string') { - this.hiddenText!.textContent = this.textarea!.value.substr( + this.hiddenText!.textContent = this.textarea!.value.substring( 0, this.textarea!.selectionStart ); @@ -436,12 +444,7 @@ export class GrTextarea extends LitElement { return caratSpan; } - private shouldResetDropdown( - text: string, - charIndex: number, - suggestions?: Item[], - char?: string - ) { + private shouldResetDropdown(text: string, charIndex: number, char?: string) { // Under any of the following conditions, close and reset the dropdown: // - The cursor is no longer at the end of the current search string // - The search string is an space or new line @@ -452,32 +455,10 @@ export class GrTextarea extends LitElement { (this.currentSearchString ?? '').length + charIndex + 1 || this.currentSearchString === ' ' || this.currentSearchString === '\n' || - !(text[charIndex] === char) || - !suggestions || - !suggestions.length + !(text[charIndex] === char) ); } - // When special char is detected, set index. We are interested only on - // special char after space or in beginning of textarea - // In case of mentions we are interested if previous char is '\n' as well - private getSpecialCharIndex(text: string) { - const charAtCursor = text[this.textarea!.selectionStart - 1]; - if ( - this.textarea!.selectionStart < 2 || - text[this.textarea!.selectionStart - 2] === ' ' - ) { - return this.textarea!.selectionStart - 1; - } - if ( - charAtCursor === '@' && - text[this.textarea!.selectionStart - 2] === '\n' - ) { - return this.textarea!.selectionStart - 1; - } - return -1; - } - private async computeSuggestions() { this.suggestions = []; if (this.currentSearchString === undefined) { @@ -513,7 +494,6 @@ export class GrTextarea extends LitElement { this.shouldResetDropdown( this.text, this.specialCharIndex, - this.suggestions, this.text[this.specialCharIndex] ) ) { @@ -539,26 +519,20 @@ export class GrTextarea extends LitElement { ); } - private computeSpecialCharIndex() { - const charAtCursor = this.text[this.textarea!.selectionStart - 1]; - - if (charAtCursor === '@' && this.specialCharIndex === -1) { - this.specialCharIndex = this.getSpecialCharIndex(this.text); - } - if (charAtCursor === ':' && this.specialCharIndex === -1) { - this.specialCharIndex = this.getSpecialCharIndex(this.text); - } - } - - private computeCurrentSearchString() { - if (this.specialCharIndex === -1) { + private computeIndexAndSearchString() { + const currentCarat = this.textarea?.selectionStart ?? this.text.length; + const m = this.text + .substring(0, currentCarat) + .match(/(?:^|\s)([:@][\S]*)$/); + if (!m) { + this.specialCharIndex = -1; this.currentSearchString = undefined; return; } - this.currentSearchString = this.text.substr( - this.specialCharIndex + 1, - this.textarea!.selectionStart - this.specialCharIndex - 1 - ); + this.currentSearchString = m[1].substring(1); + if (this.specialCharIndex !== -1) return; + + this.specialCharIndex = currentCarat - m[1].length; } // Private but used in tests. @@ -645,7 +619,7 @@ export class GrTextarea extends LitElement { // When nothing is selected, selectionStart is the caret position. We want // the indentation level of the current line, not the end of the text which // may be different. - const currentLine = this.textarea!.textarea.value.substr( + const currentLine = this.textarea!.textarea.value.substring( 0, this.textarea!.selectionStart ) diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts index 4dcaa80b2d..aa32f7d668 100644 --- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts @@ -220,20 +220,6 @@ suite('gr-textarea tests', () => { ]); }); - test('emoji selector does not open when previous char is \n', async () => { - element.textarea!.focus(); - await waitUntil(() => element.textarea!.focused === true); - - element.textarea!.selectionStart = 1; - element.textarea!.selectionEnd = 1; - element.text = '\n:'; - - await element.updateComplete; - - assert.isTrue(element.emojiSuggestions!.isHidden); - assert.isTrue(element.mentionsSuggestions!.isHidden); - }); - test('selecting mentions from dropdown', async () => { stubRestApi('getSuggestedAccounts').returns( Promise.resolve([ @@ -300,19 +286,19 @@ suite('gr-textarea tests', () => { assert.isTrue(element.emojiSuggestions!.isHidden); assert.isFalse(element.mentionsSuggestions!.isHidden); - element.text = '@h '; + element.text = '@h'; await waitUntil(() => element.suggestions.length > 0); await element.updateComplete; assert.isTrue(element.emojiSuggestions!.isHidden); assert.isFalse(element.mentionsSuggestions!.isHidden); - element.text = '@h :'; + element.text = '@h:'; await waitUntil(() => element.suggestions.length > 0); await element.updateComplete; assert.isTrue(element.emojiSuggestions!.isHidden); assert.isFalse(element.mentionsSuggestions!.isHidden); - element.text = '@h :D'; + element.text = '@h:D'; await waitUntil(() => element.suggestions.length > 0); await element.updateComplete; assert.isTrue(element.emojiSuggestions!.isHidden); @@ -347,11 +333,16 @@ suite('gr-textarea tests', () => { element.text = ':D@'; await element.updateComplete; // emoji dropdown hidden since we have no more suggestions - assert.isTrue(element.emojiSuggestions!.isHidden); + assert.isFalse(element.emojiSuggestions!.isHidden); assert.isTrue(element.mentionsSuggestions!.isHidden); element.text = ':D@b'; await element.updateComplete; + assert.isFalse(element.emojiSuggestions!.isHidden); + assert.isTrue(element.mentionsSuggestions!.isHidden); + + element.text = ':D@b '; + await element.updateComplete; assert.isTrue(element.emojiSuggestions!.isHidden); assert.isTrue(element.mentionsSuggestions!.isHidden); }); 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 5d23da12fb..b3c7dad688 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts @@ -319,9 +319,14 @@ export class GrDiff extends LitElement implements GrDiffApi { if (this.diffElement) { this.highlights.init(this.diffElement, this); } + this.observeNodes(); } override disconnectedCallback() { + if (this.nodeObserver) { + this.nodeObserver.disconnect(); + this.nodeObserver = undefined; + } this.removeSelectionListeners(); this.diffSelection.cleanup(); this.highlights.cleanup(); @@ -422,7 +427,6 @@ export class GrDiff extends LitElement implements GrDiffApi { if (changedProperties.has('groups')) { if (this.groups?.length > 0) { this.loading = false; - this.observeNodes(); } } } |