extension/src: stop reveal active symbol when outline is invsible vscode TreeView.reveal will forcely open the side bar and the package symbol outline even when the side bar is hidden or the package symbol is folded. This behavior is invasive for developer who close the side bar or fold the package outline when focusing on coding. In addition, refactor the test framework which use a poll mechanism to avoid uncondition sleep. This poll mechanism will keep checking the condition periodically until valid. For golang/vscode-go#3998 Change-Id: Ie885ebe8bcc18a7ede58d3524dc28ee2305b1019 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/782480 Auto-Submit: Hongxiang Jiang <hxjiang@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Madeline Kalil <mkalil@google.com>
diff --git a/extension/src/goPackageOutline.ts b/extension/src/goPackageOutline.ts index c88b7e6..7eecdae 100644 --- a/extension/src/goPackageOutline.ts +++ b/extension/src/goPackageOutline.ts
@@ -32,7 +32,8 @@ private packageItem = this.createPackageItem(); private packageSymbols: PackageSymbol[] = []; - private lastRevealed?: PackageSymbol; + // public for testing purpose. + public lastRevealed?: PackageSymbol; static setup(ctx: vscode.ExtensionContext) { const provider = new this(ctx); @@ -42,6 +43,13 @@ }); ctx.subscriptions.push(provider.view); ctx.subscriptions.push( + provider.view.onDidChangeVisibility((e) => { + if (e.visible) { + void provider.revealActiveSymbol(vscode.window.activeTextEditor); + } + }) + ); + ctx.subscriptions.push( vscode.commands.registerCommand('go.packageOutline.sortByName', () => provider.setSortOrder(PackageOutlineSortOrder.Name) ), @@ -186,7 +194,7 @@ } private async revealActiveSymbol(editor?: vscode.TextEditor) { - if (!this.view || !editor || editor.document !== this.activeDocument) { + if (!this.view || !this.view.visible || !editor || editor.document !== this.activeDocument) { return; } const symbol = this.findSymbolAtPosition(this.packageSymbols, editor.document.uri, editor.selection.active); @@ -201,7 +209,9 @@ try { await this.view.reveal(symbol, { expand: true, select: true }); } catch (e) { - console.log('ERROR', e); + // Catch race condition errors when reveal active symbol fire while the + // package outline is closing. + console.log('Package outline reveal error', e); } }
diff --git a/extension/test/integration/goPackageOutline.test.ts b/extension/test/integration/goPackageOutline.test.ts index 3dcb8de..80404e3 100644 --- a/extension/test/integration/goPackageOutline.test.ts +++ b/extension/test/integration/goPackageOutline.test.ts
@@ -10,7 +10,7 @@ import { updateGoVarsFromConfig } from '../../src/goInstallTools'; import { window } from 'vscode'; import { Env } from '../gopls/goplsTestEnv.utils'; - +import { sleep, poll } from './testutils'; import { getGoConfig } from '../../src/config'; import vscode = require('vscode'); @@ -37,11 +37,7 @@ }); test('opening a document should trigger package outline response', async () => { - const document = await vscode.workspace.openTextDocument( - vscode.Uri.file(path.join(fixtureDir, 'symbols_1.go')) - ); - await window.showTextDocument(document); - await waitForOutlineResult(provider, 'package_outline_test'); + await openDocInPkgOutline('symbols_1.go'); const res = provider.result; assert.strictEqual(res?.PackageName, 'package_outline_test'); assert.strictEqual(res?.Files.length, 2); @@ -51,11 +47,7 @@ }); test('clicking on symbol should navigate to definition', async () => { - const document = await vscode.workspace.openTextDocument( - vscode.Uri.file(path.join(fixtureDir, 'symbols_1.go')) - ); - await window.showTextDocument(document); - await waitForOutlineResult(provider, 'package_outline_test'); + const document = await openDocInPkgOutline('symbols_1.go'); await vscode.commands.executeCommand('setContext', 'go.showPackageOutline'); const children = await provider.getChildren(); const receiver = children?.find((symbol) => symbol.label === 'TestReceiver'); @@ -70,11 +62,7 @@ }); test('clicking on symbol in different file should open file', async () => { - const document = await vscode.workspace.openTextDocument( - vscode.Uri.file(path.join(fixtureDir, 'symbols_1.go')) - ); - await window.showTextDocument(document); - await waitForOutlineResult(provider, 'package_outline_test'); + await openDocInPkgOutline('symbols_1.go'); await vscode.commands.executeCommand('setContext', 'go.showPackageOutline'); const children = await provider.getChildren(); const receiver = children?.find((symbol) => symbol.label === 'TestReceiver'); @@ -92,11 +80,7 @@ }); test('sort by name orders symbols alphabetically', async () => { - const document = await vscode.workspace.openTextDocument( - vscode.Uri.file(path.join(fixtureDir, 'symbols_1.go')) - ); - await window.showTextDocument(document); - await waitForOutlineResult(provider, 'package_outline_test'); + await openDocInPkgOutline('symbols_1.go'); await vscode.commands.executeCommand('go.packageOutline.sortByName'); const children = await provider.getChildren(); assert.deepStrictEqual( @@ -113,11 +97,7 @@ }); test('sort by position orders symbols by source location', async () => { - const document = await vscode.workspace.openTextDocument( - vscode.Uri.file(path.join(fixtureDir, 'symbols_1.go')) - ); - await window.showTextDocument(document); - await waitForOutlineResult(provider, 'package_outline_test'); + await openDocInPkgOutline('symbols_1.go'); const children = await provider.getChildren(); assert.deepStrictEqual( (children ?? []).slice(1).map((symbol) => symbol.label), @@ -126,23 +106,64 @@ }); test('cursor changes reveal the active symbol', async () => { - const document1 = await vscode.workspace.openTextDocument( - vscode.Uri.file(path.join(fixtureDir, 'symbols_1.go')) - ); - await window.showTextDocument(document1); - await waitForOutlineResult(provider, 'package_outline_test'); - await moveCursor(document1, 19); - await sleep(500); // wait for tree view reveal - assert.strictEqual(((provider as unknown) as { lastRevealed?: PackageSymbol }).lastRevealed?.label, 'method1'); + const document1 = await openDocInPkgOutline('symbols_1.go'); + await vscode.commands.executeCommand('go.package.outline.focus'); + await poll(() => assert.strictEqual(provider.view?.visible, true)); - const document2 = await vscode.workspace.openTextDocument( - vscode.Uri.file(path.join(fixtureDir, 'symbols_2.go')) - ); - await window.showTextDocument(document2); - await waitForOutlineResult(provider, 'package_outline_test'); + await moveCursor(document1, 19); + await poll(() => assert.strictEqual(provider.lastRevealed?.label, 'method1')); + + const document2 = await openDocInPkgOutline('symbols_2.go'); await moveCursor(document2, 2); - await sleep(500); // wait for tree view reveal - assert.strictEqual(((provider as unknown) as { lastRevealed?: PackageSymbol }).lastRevealed?.label, 'method2'); + await poll(() => assert.strictEqual(provider.lastRevealed?.label, 'method2')); + }); + + test('does not reveal active symbol when sidebar is closed', async () => { + const document = await openDocInPkgOutline('symbols_1.go'); + + const view = provider.view; + assert.ok(view, 'view is undefined'); + + await vscode.commands.executeCommand('go.package.outline.focus'); + await sleep(500); + assert.strictEqual(view.visible, true, 'view should be visible initially'); + + await vscode.commands.executeCommand('workbench.action.closeSidebar'); + await sleep(500); + assert.strictEqual(view.visible, false, 'view should be invisible after closing sidebar'); + + // Reset to verify that no new symbol is revealed when the cursor moves. + provider.lastRevealed = undefined; + await moveCursor(document, 19); + await sleep(500); + + assert.strictEqual(view.visible, false, 'view should remain invisible'); + assert.strictEqual(provider.lastRevealed, undefined, 'should not reveal active symbol'); + }); + + test('does not reveal active symbol when focused on "Code Search" view', async () => { + const document = await openDocInPkgOutline('symbols_1.go'); + + const view = provider.view; + assert.ok(view, 'view is undefined'); + + await vscode.commands.executeCommand('go.package.outline.focus'); + await sleep(500); + assert.strictEqual(view.visible, true, 'view should be visible initially'); + + await vscode.commands.executeCommand('workbench.view.search'); + await sleep(500); + assert.strictEqual(view.visible, false, 'view should be invisible after switching container'); + + // Reset to verify that no new symbol is revealed when the cursor moves. + provider.lastRevealed = undefined; + + // Move the cursor to the position of a package symbol. + await moveCursor(document, 19); + await sleep(500); + + assert.strictEqual(view.visible, false, 'view should remain invisible'); + assert.strictEqual(provider.lastRevealed, undefined, 'should not reveal active symbol'); }); test('non-go file does not trigger outline', async () => { @@ -153,21 +174,25 @@ await sleep(500); // wait for gopls response assert.strictEqual(provider.result, undefined); }); + + /** + * Helper to open a file, show it in the active window editor, and wait until + * the outline result is available for the package 'package_outline_test'. + * + * @param filename The name of the file to open, relative to the fixture directory. + */ + async function openDocInPkgOutline(filename: string): Promise<vscode.TextDocument> { + const document = await vscode.workspace.openTextDocument(vscode.Uri.file(path.join(fixtureDir, filename))); + await window.showTextDocument(document); + await waitForOutlineResult(provider, 'package_outline_test'); + return document; + } }); -function sleep(ms: number) { - return new Promise((resolve) => setTimeout(resolve, ms)); -} - async function waitForOutlineResult(provider: GoPackageOutlineProvider, packageName: string) { - const deadline = Date.now() + 5000; - while (Date.now() < deadline) { - if (provider.result?.PackageName === packageName) { - return; - } - await sleep(100); - } - assert.fail(`timed out waiting for outline result for ${packageName}`); + await poll(() => { + assert.strictEqual(provider.result?.PackageName, packageName, `expected package name to be ${packageName}`); + }, 5000); } async function moveCursor(document: vscode.TextDocument, line: number, character = 0) {
diff --git a/extension/test/integration/testutils.ts b/extension/test/integration/testutils.ts index 7f85315..a8cd46d 100644 --- a/extension/test/integration/testutils.ts +++ b/extension/test/integration/testutils.ts
@@ -1,3 +1,38 @@ +/*--------------------------------------------------------- + * Copyright 2026 The Go Authors. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------*/ + export function affectedByIssue832(): boolean { return process.platform === 'win32'; } + +export function sleep(ms: number) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** + * Repeatedly executes the provided assertion function until it succeeds or the + * timeout is reached. If the assertion fails (throws an error), it will be + * retried every 100 milliseconds. + * + * @param assertion The synchronous or asynchronous assertion callback to run. + * @param timeoutMs The maximum duration in milliseconds to attempt polling + * before throwing a timeout error. Defaults to 1000ms. + * @throws The last thrown error from the assertion function if the timeout is + * reached. + */ +export async function poll(assertion: () => void | Promise<void>, timeoutMs = 1000): Promise<void> { + const deadline = Date.now() + timeoutMs; + let lastError: any; + while (Date.now() < deadline) { + try { + await assertion(); + return; + } catch (err) { + lastError = err; + } + await sleep(100); + } + throw lastError || new Error('Timed out polling condition'); +}