Skip to content

Commit d1cde8c

Browse files
committed
extension/src/welcome: readd the fix for go.showWelcome handling
The original fix for #3319 also included a minor refactoring attempt - centralize welcome page show logic in shouldShowGoWelcomePage. But that unfortunately introduced a new bug, i.e., failed to apply the Web-based IDE exclusion rule correctly. Reverted the previous change to remove the refactoring. And, this change adds the specific fix to address the bug reported in #3319. Test coverage would be nice, but it is tricky without major refactoring or complicating test logic to stub or inject dependencies. Renamed shouldShowGoWelcomePage to make it clear that this is to decide whether we have news to show purely based on the extension's version. For #3319. Change-Id: Ia4915e0201e73d136b3efcec7c0cf4f5a5e4559f Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/581118 kokoro-CI: kokoro <[email protected]> Commit-Queue: Hyang-Ah Hana Kim <[email protected]> Reviewed-by: Peter Weinberger <[email protected]>
1 parent cfc795c commit d1cde8c

File tree

2 files changed

+13
-7
lines changed

2 files changed

+13
-7
lines changed

extension/src/welcome.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import path = require('path');
1212
import semver = require('semver');
1313
import { extensionId } from './const';
1414
import { GoExtensionContext } from './context';
15-
import { extensionInfo } from './config';
15+
import { extensionInfo, getGoConfig } from './config';
1616
import { getFromGlobalState, updateGlobalState } from './stateUtils';
1717
import { createRegisterCommand } from './commands';
1818

@@ -30,8 +30,14 @@ export class WelcomePanel {
3030
});
3131
}
3232

33-
// Show the Go welcome page on update.
34-
if (!extensionInfo.isInCloudIDE && vscode.workspace.getConfiguration('go.showWelcome')) {
33+
// Show the Go welcome page on update unless one of the followings is true:
34+
// * the extension is running in Cloud IDE or
35+
// * the user explicitly opted out (go.showWelcome === false)
36+
//
37+
// It is difficult to write useful tests for this suppression logic
38+
// without major refactoring or complicating tests to enable
39+
// dependency injection or stubbing.
40+
if (!extensionInfo.isInCloudIDE && getGoConfig().get('showWelcome') !== false) {
3541
showGoWelcomePage();
3642
}
3743
}
@@ -269,15 +275,15 @@ function showGoWelcomePage() {
269275

270276
const savedGoExtensionVersion = getFromGlobalState(goExtensionVersionKey, '');
271277

272-
if (shouldShowGoWelcomePage(showVersions, goExtensionVersion, savedGoExtensionVersion)) {
278+
if (hasNewsForNewVersion(showVersions, goExtensionVersion, savedGoExtensionVersion)) {
273279
vscode.commands.executeCommand('go.welcome');
274280
}
275281
if (goExtensionVersion !== savedGoExtensionVersion) {
276282
updateGlobalState(goExtensionVersionKey, goExtensionVersion);
277283
}
278284
}
279285

280-
export function shouldShowGoWelcomePage(showVersions: string[], newVersion: string, oldVersion: string): boolean {
286+
export function hasNewsForNewVersion(showVersions: string[], newVersion: string, oldVersion: string): boolean {
281287
if (newVersion === oldVersion) {
282288
return false;
283289
}

extension/test/integration/welcome.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import vscode = require('vscode');
77
import assert from 'assert';
8-
import { shouldShowGoWelcomePage } from '../../src/welcome';
8+
import { hasNewsForNewVersion } from '../../src/welcome';
99
import { extensionId } from '../../src/const';
1010
import { WelcomePanel } from '../../src/welcome';
1111

@@ -54,7 +54,7 @@ suite('WelcomePanel Tests', () => {
5454
const [showVersions, newVersion, oldVersion, expected] = c;
5555

5656
test(`shouldShowGoWelcomePage(${JSON.stringify(showVersions)}, ${newVersion}, ${oldVersion})`, () => {
57-
assert.strictEqual(shouldShowGoWelcomePage(showVersions, newVersion, oldVersion), expected);
57+
assert.strictEqual(hasNewsForNewVersion(showVersions, newVersion, oldVersion), expected);
5858
});
5959
});
6060
});

0 commit comments

Comments
 (0)