From 066c5844050d223f28e72189ede3e9619f30cdec Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 3 Feb 2021 19:20:44 +0100 Subject: [PATCH 01/20] enable remote smoke tests --- package.json | 2 +- test/automation/src/code.ts | 20 +++++++++++++------ .../src/areas/extensions/extensions.test.ts | 3 --- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index ca26be9f76f..9ed1fa60ae3 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "code-oss-dev", "version": "1.54.0", - "distro": "3b4f6074b6501e7e9f09a53dcaffce592d264b77", + "distro": "bc881dcef106ab3d9f62a6fda15b804eef929ef1", "author": { "name": "Microsoft Corporation" }, diff --git a/test/automation/src/code.ts b/test/automation/src/code.ts index a947b1c100e..0f04f08fa26 100644 --- a/test/automation/src/code.ts +++ b/test/automation/src/code.ts @@ -120,7 +120,7 @@ export async function spawn(options: SpawnOptions): Promise { let child: cp.ChildProcess | undefined; let connectDriver: typeof connectElectronDriver; - copyExtension(options, 'vscode-notebook-tests'); + copyExtension(options.extensionsPath, 'vscode-notebook-tests'); if (options.web) { await launch(options.userDataDir, options.workspacePath, options.codePath, options.extensionsPath); @@ -150,11 +150,19 @@ export async function spawn(options: SpawnOptions): Promise { if (codePath) { // running against a build: copy the test resolver extension - copyExtension(options, 'vscode-test-resolver'); + copyExtension(options.extensionsPath, 'vscode-test-resolver'); } args.push('--enable-proposed-api=vscode.vscode-test-resolver'); const remoteDataDir = `${options.userDataDir}-server`; mkdirp.sync(remoteDataDir); + + if (codePath) { + // running against a build: copy the test resolver extension into remote extensions dir + const remoteExtensionsDir = path.join(remoteDataDir, 'extensions'); + mkdirp.sync(remoteExtensionsDir); + copyExtension(remoteExtensionsDir, 'vscode-notebook-tests'); + } + env['TESTRESOLVER_DATA_FOLDER'] = remoteDataDir; } @@ -186,11 +194,11 @@ export async function spawn(options: SpawnOptions): Promise { return connect(connectDriver, child, outPath, handle, options.logger); } -async function copyExtension(options: SpawnOptions, extId: string): Promise { - const testResolverExtPath = path.join(options.extensionsPath, extId); - if (!fs.existsSync(testResolverExtPath)) { +async function copyExtension(extensionsPath: string, extId: string): Promise { + const dest = path.join(extensionsPath, extId); + if (!fs.existsSync(dest)) { const orig = path.join(repoPath, 'extensions', extId); - await new Promise((c, e) => ncp(orig, testResolverExtPath, err => err ? e(err) : c())); + await new Promise((c, e) => ncp(orig, dest, err => err ? e(err) : c())); } } diff --git a/test/smoke/src/areas/extensions/extensions.test.ts b/test/smoke/src/areas/extensions/extensions.test.ts index d5f6004bc3d..510e2e06579 100644 --- a/test/smoke/src/areas/extensions/extensions.test.ts +++ b/test/smoke/src/areas/extensions/extensions.test.ts @@ -23,9 +23,6 @@ export function setup() { await app.workbench.extensions.waitForExtensionsViewlet(); - if (app.remote) { - await app.reload(); - } await app.workbench.quickaccess.runCommand('Smoke Test Check'); await app.workbench.statusbar.waitForStatusbarText('smoke test', 'VS Code Smoke Test Check'); }); From 9c479a4518d72edc4da77df0b9439a6c1187eefc Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 3 Feb 2021 20:48:45 +0100 Subject: [PATCH 02/20] enable remote smoke tests --- build/azure-pipelines/darwin/product-build-darwin.yml | 10 ++++++++++ package.json | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/build/azure-pipelines/darwin/product-build-darwin.yml b/build/azure-pipelines/darwin/product-build-darwin.yml index 283cb4741b3..fe9c1cd77f1 100644 --- a/build/azure-pipelines/darwin/product-build-darwin.yml +++ b/build/azure-pipelines/darwin/product-build-darwin.yml @@ -244,6 +244,16 @@ steps: displayName: Run smoke tests (Electron) condition: and(succeeded(), eq(variables['VSCODE_ARCH'], 'x64'), eq(variables['VSCODE_STEP_ON_IT'], 'false')) + - script: | + set -e + APP_ROOT=$(agent.builddirectory)/VSCode-darwin-$(VSCODE_ARCH) + APP_NAME="`ls $APP_ROOT | head -n 1`" + VSCODE_REMOTE_SERVER_PATH="$(agent.builddirectory)/vscode-reh-darwin" \ + yarn smoketest-no-compile --build "$APP_ROOT/$APP_NAME" --remote + timeoutInMinutes: 5 + displayName: Run smoke tests (Remote) + condition: and(succeeded(), eq(variables['VSCODE_ARCH'], 'x64'), eq(variables['VSCODE_STEP_ON_IT'], 'false')) + - script: | set -e VSCODE_REMOTE_SERVER_PATH="$(agent.builddirectory)/vscode-reh-web-darwin" \ diff --git a/package.json b/package.json index 9ed1fa60ae3..ca26be9f76f 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "code-oss-dev", "version": "1.54.0", - "distro": "bc881dcef106ab3d9f62a6fda15b804eef929ef1", + "distro": "3b4f6074b6501e7e9f09a53dcaffce592d264b77", "author": { "name": "Microsoft Corporation" }, From 4e6d9ce2b7904f792c147a39a40fdcc3b0594aa8 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 3 Feb 2021 21:42:12 +0100 Subject: [PATCH 03/20] add server cli test --- test/automation/src/application.ts | 4 ++++ test/automation/src/extensions.ts | 6 ++++++ test/automation/src/playwrightDriver.ts | 4 +++- .../src/areas/extensions/extensions.test.ts | 16 ++++++++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/test/automation/src/application.ts b/test/automation/src/application.ts index 9f6480ab42a..0440d68d4e5 100644 --- a/test/automation/src/application.ts +++ b/test/automation/src/application.ts @@ -51,6 +51,10 @@ export class Application { return !!this.options.remote; } + get web(): boolean { + return !!this.options.web; + } + private _workspacePathOrFolder: string; get workspacePathOrFolder(): string { return this._workspacePathOrFolder; diff --git a/test/automation/src/extensions.ts b/test/automation/src/extensions.ts index fa1fe2fc0ba..b2bedb8f5c8 100644 --- a/test/automation/src/extensions.ts +++ b/test/automation/src/extensions.ts @@ -32,6 +32,12 @@ export class Extensions extends Viewlet { await this.code.waitAndClick(SEARCH_BOX); await this.code.waitForActiveElement(SEARCH_BOX); await this.code.waitForTypeInEditor(SEARCH_BOX, `@id:${id}`); + await this.code.waitForElement(`div.extensions-viewlet[id="workbench.view.extensions"] .monaco-list-row[data-extension-id="${id}"]`); + } + + async openExtension(id: string): Promise { + await this.searchForExtension(id); + await this.code.waitAndClick(`div.extensions-viewlet[id="workbench.view.extensions"] .monaco-list-row[data-extension-id="${id}"]`); } async installExtension(id: string, waitUntilEnabled: boolean): Promise { diff --git a/test/automation/src/playwrightDriver.ts b/test/automation/src/playwrightDriver.ts index 7d7c34deb25..f2371419b48 100644 --- a/test/automation/src/playwrightDriver.ts +++ b/test/automation/src/playwrightDriver.ts @@ -101,8 +101,10 @@ export async function launch(userDataDir: string, _workspacePath: string, codeSe VSCODE_REMOTE_SERVER_PATH: codeServerPath, ...process.env }; + const args = ['--browser', 'none', '--driver', 'web', '--extensions-dir', extPath]; let serverLocation: string | undefined; if (codeServerPath) { + args.push(...['--install-builtin-extension', 'github.vscode-pull-request-github', '--start-server']); serverLocation = join(codeServerPath, `server.${process.platform === 'win32' ? 'cmd' : 'sh'}`); console.log(`Starting built server from '${serverLocation}'`); } else { @@ -111,7 +113,7 @@ export async function launch(userDataDir: string, _workspacePath: string, codeSe } server = spawn( serverLocation, - ['--browser', 'none', '--driver', 'web', '--extensions-dir', extPath], + args, { env } ); server.stderr?.on('data', error => console.log(`Server stderr: ${error}`)); diff --git a/test/smoke/src/areas/extensions/extensions.test.ts b/test/smoke/src/areas/extensions/extensions.test.ts index 510e2e06579..a62314cd0e8 100644 --- a/test/smoke/src/areas/extensions/extensions.test.ts +++ b/test/smoke/src/areas/extensions/extensions.test.ts @@ -27,6 +27,22 @@ export function setup() { await app.workbench.statusbar.waitForStatusbarText('smoke test', 'VS Code Smoke Test Check'); }); + it(`extension installed by server cli`, async function () { + const app = this.app as Application; + + if (app.quality === Quality.Dev || !app.web) { + this.skip(); + return; + } + + await app.workbench.extensions.openExtensionsViewlet(); + + await app.workbench.extensions.openExtension('github.vscode-pull-request-github'); + + await this.code.waitForElement(`.extension-editor .monaco-action-bar .action-item:not(.disabled) .extension-action.uninstall`); + await this.code.waitForElement(`.extension-editor .monaco-action-bar .action-item:not(.disabled) .extension-action[title="Disable this extension"]`); + }); + after(async function () { const app = this.app as Application; await app.workbench.settingsEditor.clearUserSettings(); From a9ccd48a1f73f285d186a90827284dfce1a31fcd Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Thu, 4 Feb 2021 08:44:06 +0100 Subject: [PATCH 04/20] enable extension tests in web --- .../src/areas/extensions/extensions.test.ts | 20 +++++-------------- test/smoke/src/main.ts | 2 +- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/test/smoke/src/areas/extensions/extensions.test.ts b/test/smoke/src/areas/extensions/extensions.test.ts index a62314cd0e8..72a3460d8fe 100644 --- a/test/smoke/src/areas/extensions/extensions.test.ts +++ b/test/smoke/src/areas/extensions/extensions.test.ts @@ -15,7 +15,9 @@ export function setup() { return; } - await app.workbench.settingsEditor.addUserSetting('webview.experimental.useIframes', 'true'); + if (!app.web) { + await app.workbench.settingsEditor.addUserSetting('webview.experimental.useIframes', 'true'); + } await app.workbench.extensions.openExtensionsViewlet(); @@ -27,24 +29,12 @@ export function setup() { await app.workbench.statusbar.waitForStatusbarText('smoke test', 'VS Code Smoke Test Check'); }); - it(`extension installed by server cli`, async function () { + after(async function () { const app = this.app as Application; - - if (app.quality === Quality.Dev || !app.web) { - this.skip(); + if (app.web) { return; } - await app.workbench.extensions.openExtensionsViewlet(); - - await app.workbench.extensions.openExtension('github.vscode-pull-request-github'); - - await this.code.waitForElement(`.extension-editor .monaco-action-bar .action-item:not(.disabled) .extension-action.uninstall`); - await this.code.waitForElement(`.extension-editor .monaco-action-bar .action-item:not(.disabled) .extension-action[title="Disable this extension"]`); - }); - - after(async function () { - const app = this.app as Application; await app.workbench.settingsEditor.clearUserSettings(); }); diff --git a/test/smoke/src/main.ts b/test/smoke/src/main.ts index cf3c02b0efb..13023eb50c8 100644 --- a/test/smoke/src/main.ts +++ b/test/smoke/src/main.ts @@ -315,7 +315,7 @@ describe(`VSCode Smoke Tests (${opts.web ? 'Web' : 'Electron'})`, () => { setupDataLanguagesTests(); setupDataEditorTests(); setupDataStatusbarTests(!!opts.web); - if (!opts.web) { setupDataExtensionTests(); } + setupDataExtensionTests(); if (!opts.web) { setupDataMultirootTests(); } if (!opts.web) { setupDataLocalizationTests(); } if (!opts.web) { setupLaunchTests(); } From 370a7ee077d49f37111271a99b08e13a3d3c897e Mon Sep 17 00:00:00 2001 From: Rachel Macfarlane Date: Wed, 3 Feb 2021 09:58:08 -0800 Subject: [PATCH 05/20] Don't show a modal on 'getSession' for access requests if 'createIfNone' is false, fixes #111529 --- src/vs/vscode.d.ts | 3 + .../api/browser/mainThreadAuthentication.ts | 295 +++++------------ .../workbench/api/common/extHost.protocol.ts | 9 - .../api/common/extHostAuthentication.ts | 39 +-- .../browser/authenticationService.ts | 298 +++++++++++++++++- 5 files changed, 363 insertions(+), 281 deletions(-) diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 7c5c6828b6a..b8f07070bde 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -12287,6 +12287,9 @@ declare module 'vscode' { * on the accounts activity bar icon. An entry for the extension will be added under the menu to sign in. This * allows quietly prompting the user to sign in. * + * If there is a matching session but the extension has not been granted access to it, setting this to true + * will also result in an immediate modal dialog, and false will add a numbered badge to the accounts icon. + * * Defaults to false. */ createIfNone?: boolean; diff --git a/src/vs/workbench/api/browser/mainThreadAuthentication.ts b/src/vs/workbench/api/browser/mainThreadAuthentication.ts index 98bb98d8cee..4ed2572a8f7 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/src/vs/workbench/api/browser/mainThreadAuthentication.ts @@ -7,67 +7,15 @@ import { Disposable } from 'vs/base/common/lifecycle'; import * as modes from 'vs/editor/common/modes'; import * as nls from 'vs/nls'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; -import { IAuthenticationService, AllowedExtension, readAllowedExtensions, getAuthenticationProviderActivationEvent } from 'vs/workbench/services/authentication/browser/authenticationService'; +import { IAuthenticationService, AllowedExtension, readAllowedExtensions, getAuthenticationProviderActivationEvent, addAccountUsage, readAccountUsages, removeAccountUsage } from 'vs/workbench/services/authentication/browser/authenticationService'; import { ExtHostAuthenticationShape, ExtHostContext, IExtHostContext, MainContext, MainThreadAuthenticationShape } from '../common/extHost.protocol'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage'; import Severity from 'vs/base/common/severity'; import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput'; import { INotificationService } from 'vs/platform/notification/common/notification'; -import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService'; import { fromNow } from 'vs/base/common/date'; import { ActivationKind, IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; -import { isWeb } from 'vs/base/common/platform'; - -const VSO_ALLOWED_EXTENSIONS = ['github.vscode-pull-request-github', 'github.vscode-pull-request-github-insiders', 'vscode.git', 'ms-vsonline.vsonline', 'vscode.github-browser', 'ms-vscode.github-browser', 'github.codespaces']; - -interface IAccountUsage { - extensionId: string; - extensionName: string; - lastUsed: number; -} - -function readAccountUsages(storageService: IStorageService, providerId: string, accountName: string,): IAccountUsage[] { - const accountKey = `${providerId}-${accountName}-usages`; - const storedUsages = storageService.get(accountKey, StorageScope.GLOBAL); - let usages: IAccountUsage[] = []; - if (storedUsages) { - try { - usages = JSON.parse(storedUsages); - } catch (e) { - // ignore - } - } - - return usages; -} - -function removeAccountUsage(storageService: IStorageService, providerId: string, accountName: string): void { - const accountKey = `${providerId}-${accountName}-usages`; - storageService.remove(accountKey, StorageScope.GLOBAL); -} - -function addAccountUsage(storageService: IStorageService, providerId: string, accountName: string, extensionId: string, extensionName: string) { - const accountKey = `${providerId}-${accountName}-usages`; - const usages = readAccountUsages(storageService, providerId, accountName); - - const existingUsageIndex = usages.findIndex(usage => usage.extensionId === extensionId); - if (existingUsageIndex > -1) { - usages.splice(existingUsageIndex, 1, { - extensionId, - extensionName, - lastUsed: Date.now() - }); - } else { - usages.push({ - extensionId, - extensionName, - lastUsed: Date.now() - }); - } - - storageService.store(accountKey, JSON.stringify(usages), StorageScope.GLOBAL, StorageTarget.MACHINE); -} export class MainThreadAuthenticationProvider extends Disposable { private _accounts = new Map(); // Map account name to session ids @@ -220,7 +168,6 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu @IDialogService private readonly dialogService: IDialogService, @IStorageService private readonly storageService: IStorageService, @INotificationService private readonly notificationService: INotificationService, - @IRemoteAgentService private readonly remoteAgentService: IRemoteAgentService, @IQuickInputService private readonly quickInputService: IQuickInputService, @IExtensionService private readonly extensionService: IExtensionService ) { @@ -246,10 +193,6 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu })); } - $getProviderIds(): Promise { - return Promise.resolve(this.authenticationService.getProviderIds()); - } - async $registerAuthenticationProvider(id: string, label: string, supportsMultipleAccounts: boolean): Promise { const provider = new MainThreadAuthenticationProvider(this._proxy, id, label, supportsMultipleAccounts, this.notificationService, this.storageService, this.quickInputService, this.dialogService); await provider.initialize(); @@ -268,172 +211,17 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu this.authenticationService.sessionsUpdate(id, event); } - $getSessions(id: string): Promise> { - return this.authenticationService.getSessions(id); - } - - $login(providerId: string, scopes: string[]): Promise { - return this.authenticationService.login(providerId, scopes); - } - $logout(providerId: string, sessionId: string): Promise { return this.authenticationService.logout(providerId, sessionId); } - async $requestNewSession(providerId: string, scopes: string[], extensionId: string, extensionName: string): Promise { - return this.authenticationService.requestNewSession(providerId, scopes, extensionId, extensionName); - } - - async $getSession(providerId: string, scopes: string[], extensionId: string, extensionName: string, options: { createIfNone: boolean, clearSessionPreference: boolean }): Promise { - const orderedScopes = scopes.sort().join(' '); - const sessions = (await this.$getSessions(providerId)).filter(session => session.scopes.slice().sort().join(' ') === orderedScopes); - const label = this.authenticationService.getLabel(providerId); - - if (sessions.length) { - if (!this.authenticationService.supportsMultipleAccounts(providerId)) { - const session = sessions[0]; - const allowed = await this.$getSessionsPrompt(providerId, session.account.label, label, extensionId, extensionName); - if (allowed) { - return session; - } else { - throw new Error('User did not consent to login.'); - } - } - - // On renderer side, confirm consent, ask user to choose between accounts if multiple sessions are valid - const selected = await this.$selectSession(providerId, label, extensionId, extensionName, sessions, scopes, !!options.clearSessionPreference); - return sessions.find(session => session.id === selected.id); - } else { - if (options.createIfNone) { - const isAllowed = await this.$loginPrompt(label, extensionName); - if (!isAllowed) { - throw new Error('User did not consent to login.'); - } - - const session = await this.authenticationService.login(providerId, scopes); - await this.$setTrustedExtensionAndAccountPreference(providerId, session.account.label, extensionId, extensionName, session.id); - return session; - } else { - await this.$requestNewSession(providerId, scopes, extensionId, extensionName); - return undefined; - } - } - } - - async $selectSession(providerId: string, providerName: string, extensionId: string, extensionName: string, potentialSessions: modes.AuthenticationSession[], scopes: string[], clearSessionPreference: boolean): Promise { - if (!potentialSessions.length) { - throw new Error('No potential sessions found'); - } - - if (clearSessionPreference) { - this.storageService.remove(`${extensionName}-${providerId}`, StorageScope.GLOBAL); - } else { - const existingSessionPreference = this.storageService.get(`${extensionName}-${providerId}`, StorageScope.GLOBAL); - if (existingSessionPreference) { - const matchingSession = potentialSessions.find(session => session.id === existingSessionPreference); - if (matchingSession) { - const allowed = await this.$getSessionsPrompt(providerId, matchingSession.account.label, providerName, extensionId, extensionName); - if (allowed) { - return matchingSession; - } - } - } - } - - return new Promise((resolve, reject) => { - const quickPick = this.quickInputService.createQuickPick<{ label: string, session?: modes.AuthenticationSession }>(); - quickPick.ignoreFocusOut = true; - const items: { label: string, session?: modes.AuthenticationSession }[] = potentialSessions.map(session => { - return { - label: session.account.label, - session - }; - }); - - items.push({ - label: nls.localize('useOtherAccount', "Sign in to another account") - }); - - quickPick.items = items; - quickPick.title = nls.localize( - { - key: 'selectAccount', - comment: ['The placeholder {0} is the name of an extension. {1} is the name of the type of account, such as Microsoft or GitHub.'] - }, - "The extension '{0}' wants to access a {1} account", - extensionName, - providerName); - quickPick.placeholder = nls.localize('getSessionPlateholder', "Select an account for '{0}' to use or Esc to cancel", extensionName); - - quickPick.onDidAccept(async _ => { - const selected = quickPick.selectedItems[0]; - - const session = selected.session ?? await this.authenticationService.login(providerId, scopes); - - const accountName = session.account.label; - - const allowList = readAllowedExtensions(this.storageService, providerId, accountName); - if (!allowList.find(allowed => allowed.id === extensionId)) { - allowList.push({ id: extensionId, name: extensionName }); - this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL, StorageTarget.USER); - } - - this.storageService.store(`${extensionName}-${providerId}`, session.id, StorageScope.GLOBAL, StorageTarget.MACHINE); - - quickPick.dispose(); - resolve(session); - }); - - quickPick.onDidHide(_ => { - if (!quickPick.selectedItems[0]) { - reject('User did not consent to account access'); - } - - quickPick.dispose(); - }); - - quickPick.show(); - }); - } - - async $getSessionsPrompt(providerId: string, accountName: string, providerName: string, extensionId: string, extensionName: string): Promise { + private isAccessAllowed(providerId: string, accountName: string, extensionId: string): boolean { const allowList = readAllowedExtensions(this.storageService, providerId, accountName); const extensionData = allowList.find(extension => extension.id === extensionId); - if (extensionData) { - addAccountUsage(this.storageService, providerId, accountName, extensionId, extensionName); - return true; - } - - const remoteConnection = this.remoteAgentService.getConnection(); - const isVSO = remoteConnection !== null - ? remoteConnection.remoteAuthority.startsWith('vsonline') || remoteConnection.remoteAuthority.startsWith('codespaces') - : isWeb; - - if (isVSO && VSO_ALLOWED_EXTENSIONS.includes(extensionId)) { - addAccountUsage(this.storageService, providerId, accountName, extensionId, extensionName); - return true; - } - - const { choice } = await this.dialogService.show( - Severity.Info, - nls.localize('confirmAuthenticationAccess', "The extension '{0}' wants to access the {1} account '{2}'.", extensionName, providerName, accountName), - [nls.localize('allow', "Allow"), nls.localize('cancel', "Cancel")], - { - cancelId: 1 - } - ); - - const allow = choice === 0; - if (allow) { - addAccountUsage(this.storageService, providerId, accountName, extensionId, extensionName); - allowList.push({ id: extensionId, name: extensionName }); - this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL, StorageTarget.USER); - } - - return allow; + return !!extensionData; } - async $loginPrompt(providerName: string, extensionName: string): Promise { + private async loginPrompt(providerName: string, extensionName: string): Promise { const { choice } = await this.dialogService.show( Severity.Info, nls.localize('confirmLogin', "The extension '{0}' wants to sign in using {1}.", extensionName, providerName), @@ -446,7 +234,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu return choice === 0; } - async $setTrustedExtensionAndAccountPreference(providerId: string, accountName: string, extensionId: string, extensionName: string, sessionId: string): Promise { + private async setTrustedExtensionAndAccountPreference(providerId: string, accountName: string, extensionId: string, extensionName: string, sessionId: string): Promise { const allowList = readAllowedExtensions(this.storageService, providerId, accountName); if (!allowList.find(allowed => allowed.id === extensionId)) { allowList.push({ id: extensionId, name: extensionName }); @@ -454,6 +242,77 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu } this.storageService.store(`${extensionName}-${providerId}`, sessionId, StorageScope.GLOBAL, StorageTarget.MACHINE); - addAccountUsage(this.storageService, providerId, accountName, extensionId, extensionName); + + } + + private async selectSession(providerId: string, extensionId: string, extensionName: string, potentialSessions: modes.AuthenticationSession[], clearSessionPreference: boolean): Promise { + if (!potentialSessions.length) { + throw new Error('No potential sessions found'); + } + + if (clearSessionPreference) { + this.storageService.remove(`${extensionName}-${providerId}`, StorageScope.GLOBAL); + } else { + const existingSessionPreference = this.storageService.get(`${extensionName}-${providerId}`, StorageScope.GLOBAL); + if (existingSessionPreference) { + const matchingSession = potentialSessions.find(session => session.id === existingSessionPreference); + if (matchingSession) { + const allowed = await this.authenticationService.showGetSessionPrompt(providerId, matchingSession.account.label, extensionId, extensionName); + if (allowed) { + return matchingSession; + } + } + } + } + + return this.authenticationService.selectSession(providerId, extensionId, extensionName, potentialSessions); + } + + async $getSession(providerId: string, scopes: string[], extensionId: string, extensionName: string, options: { createIfNone: boolean, clearSessionPreference: boolean }): Promise { + const orderedScopes = scopes.sort().join(' '); + const sessions = (await this.authenticationService.getSessions(providerId)).filter(session => session.scopes.slice().sort().join(' ') === orderedScopes); + + const silent = !options.createIfNone; + let session: modes.AuthenticationSession | undefined; + if (sessions.length) { + if (!this.authenticationService.supportsMultipleAccounts(providerId)) { + session = sessions[0]; + const allowed = this.isAccessAllowed(providerId, session.account.label, extensionId); + if (!allowed) { + if (!silent) { + const didAcceptPrompt = await this.authenticationService.showGetSessionPrompt(providerId, session.account.label, extensionId, extensionName); + if (!didAcceptPrompt) { + throw new Error('User did not consent to login.'); + } + } else { + this.authenticationService.requestSessionAccess(providerId, extensionId, extensionName, [session]); + } + } + } else { + if (!silent) { + session = await this.selectSession(providerId, extensionId, extensionName, sessions, !!options.clearSessionPreference); + } else { + this.authenticationService.requestSessionAccess(providerId, extensionId, extensionName, sessions); + } + } + } else { + if (!silent) { + const isAllowed = await this.loginPrompt(providerId, extensionName); + if (!isAllowed) { + throw new Error('User did not consent to login.'); + } + + session = await this.authenticationService.login(providerId, scopes); + await this.setTrustedExtensionAndAccountPreference(providerId, session.account.label, extensionId, extensionName, session.id); + } else { + await this.authenticationService.requestNewSession(providerId, scopes, extensionId, extensionName); + } + } + + if (session) { + addAccountUsage(this.storageService, providerId, session.account.label, extensionId, extensionName); + } + + return session; } } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 3488d18f5e8..e2c1e6fbf8f 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -165,17 +165,8 @@ export interface MainThreadAuthenticationShape extends IDisposable { $registerAuthenticationProvider(id: string, label: string, supportsMultipleAccounts: boolean): void; $unregisterAuthenticationProvider(id: string): void; $ensureProvider(id: string): Promise; - $getProviderIds(): Promise; $sendDidChangeSessions(providerId: string, event: modes.AuthenticationSessionsChangeEvent): void; $getSession(providerId: string, scopes: string[], extensionId: string, extensionName: string, options: { createIfNone?: boolean, clearSessionPreference?: boolean }): Promise; - $selectSession(providerId: string, providerName: string, extensionId: string, extensionName: string, potentialSessions: modes.AuthenticationSession[], scopes: string[], clearSessionPreference: boolean): Promise; - $getSessionsPrompt(providerId: string, accountName: string, providerName: string, extensionId: string, extensionName: string): Promise; - $loginPrompt(providerName: string, extensionName: string): Promise; - $setTrustedExtensionAndAccountPreference(providerId: string, accountName: string, extensionId: string, extensionName: string, sessionId: string): Promise; - $requestNewSession(providerId: string, scopes: string[], extensionId: string, extensionName: string): Promise; - - $getSessions(providerId: string): Promise>; - $login(providerId: string, scopes: string[]): Promise; $logout(providerId: string, sessionId: string): Promise; } diff --git a/src/vs/workbench/api/common/extHostAuthentication.ts b/src/vs/workbench/api/common/extHostAuthentication.ts index 7376e9c66cf..5a3aab45657 100644 --- a/src/vs/workbench/api/common/extHostAuthentication.ts +++ b/src/vs/workbench/api/common/extHostAuthentication.ts @@ -83,45 +83,8 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { private async _getSession(requestingExtension: IExtensionDescription, extensionId: string, providerId: string, scopes: string[], options: vscode.AuthenticationGetSessionOptions = {}): Promise { await this._proxy.$ensureProvider(providerId); - const providerData = this._authenticationProviders.get(providerId); const extensionName = requestingExtension.displayName || requestingExtension.name; - - if (!providerData) { - return this._proxy.$getSession(providerId, scopes, extensionId, extensionName, options); - } - - const orderedScopes = scopes.sort().join(' '); - const sessions = (await providerData.provider.getSessions()).filter(session => session.scopes.slice().sort().join(' ') === orderedScopes); - - let session: vscode.AuthenticationSession | undefined = undefined; - if (sessions.length) { - if (!providerData.options.supportsMultipleAccounts) { - session = sessions[0]; - const allowed = await this._proxy.$getSessionsPrompt(providerId, session.account.label, providerData.label, extensionId, extensionName); - if (!allowed) { - throw new Error('User did not consent to login.'); - } - } else { - // On renderer side, confirm consent, ask user to choose between accounts if multiple sessions are valid - const selected = await this._proxy.$selectSession(providerId, providerData.label, extensionId, extensionName, sessions, scopes, !!options.clearSessionPreference); - session = sessions.find(session => session.id === selected.id); - } - - } else { - if (options.createIfNone) { - const isAllowed = await this._proxy.$loginPrompt(providerData.label, extensionName); - if (!isAllowed) { - throw new Error('User did not consent to login.'); - } - - session = await providerData.provider.login(scopes); - await this._proxy.$setTrustedExtensionAndAccountPreference(providerId, session.account.label, extensionId, extensionName, session.id); - } else { - await this._proxy.$requestNewSession(providerId, scopes, extensionId, extensionName); - } - } - - return session; + return this._proxy.$getSession(providerId, scopes, extensionId, extensionName, options); } async logout(providerId: string, sessionId: string): Promise { diff --git a/src/vs/workbench/services/authentication/browser/authenticationService.ts b/src/vs/workbench/services/authentication/browser/authenticationService.ts index f28d3149cf2..e8b2623a6da 100644 --- a/src/vs/workbench/services/authentication/browser/authenticationService.ts +++ b/src/vs/workbench/services/authentication/browser/authenticationService.ts @@ -23,9 +23,64 @@ import { IJSONSchema } from 'vs/base/common/jsonSchema'; import { flatten } from 'vs/base/common/arrays'; import { isFalsyOrWhitespace } from 'vs/base/common/strings'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; +import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { Severity } from 'vs/platform/notification/common/notification'; +import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput'; +import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService'; +import { isWeb } from 'vs/base/common/platform'; export function getAuthenticationProviderActivationEvent(id: string): string { return `onAuthenticationRequest:${id}`; } +export interface IAccountUsage { + extensionId: string; + extensionName: string; + lastUsed: number; +} + +const VSO_ALLOWED_EXTENSIONS = ['github.vscode-pull-request-github', 'github.vscode-pull-request-github-insiders', 'vscode.git', 'ms-vsonline.vsonline', 'vscode.github-browser', 'ms-vscode.github-browser', 'github.codespaces']; + +export function readAccountUsages(storageService: IStorageService, providerId: string, accountName: string,): IAccountUsage[] { + const accountKey = `${providerId}-${accountName}-usages`; + const storedUsages = storageService.get(accountKey, StorageScope.GLOBAL); + let usages: IAccountUsage[] = []; + if (storedUsages) { + try { + usages = JSON.parse(storedUsages); + } catch (e) { + // ignore + } + } + + return usages; +} + +export function removeAccountUsage(storageService: IStorageService, providerId: string, accountName: string): void { + const accountKey = `${providerId}-${accountName}-usages`; + storageService.remove(accountKey, StorageScope.GLOBAL); +} + +export function addAccountUsage(storageService: IStorageService, providerId: string, accountName: string, extensionId: string, extensionName: string) { + const accountKey = `${providerId}-${accountName}-usages`; + const usages = readAccountUsages(storageService, providerId, accountName); + + const existingUsageIndex = usages.findIndex(usage => usage.extensionId === extensionId); + if (existingUsageIndex > -1) { + usages.splice(existingUsageIndex, 1, { + extensionId, + extensionName, + lastUsed: Date.now() + }); + } else { + usages.push({ + extensionId, + extensionName, + lastUsed: Date.now() + }); + } + + storageService.store(accountKey, JSON.stringify(usages), StorageScope.GLOBAL, StorageTarget.MACHINE); +} + export type AuthenticationSessionInfo = { readonly id: string, readonly accessToken: string, readonly providerId: string, readonly canSignOut?: boolean }; export async function getCurrentAuthenticationSessionInfo(environmentService: IWorkbenchEnvironmentService, productService: IProductService): Promise { if (environmentService.options?.credentialsProvider) { @@ -53,7 +108,11 @@ export interface IAuthenticationService { getProviderIds(): string[]; registerAuthenticationProvider(id: string, provider: MainThreadAuthenticationProvider): void; unregisterAuthenticationProvider(id: string): void; - requestNewSession(id: string, scopes: string[], extensionId: string, extensionName: string): void; + showGetSessionPrompt(providerId: string, accountName: string, extensionId: string, extensionName: string): Promise; + selectSession(providerId: string, extensionId: string, extensionName: string, possibleSessions: AuthenticationSession[]): Promise; + requestSessionAccess(providerId: string, extensionId: string, extensionName: string, possibleSessions: AuthenticationSession[]): void; + completeSessionAccessRequest(providerId: string, extensionId: string, extensionName: string): Promise + requestNewSession(providerId: string, scopes: string[], extensionId: string, extensionName: string): Promise; sessionsUpdate(providerId: string, event: AuthenticationSessionsChangeEvent): void; readonly onDidRegisterAuthenticationProvider: Event; @@ -134,6 +193,7 @@ export class AuthenticationService extends Disposable implements IAuthentication private _placeholderMenuItem: IDisposable | undefined; private _noAccountsMenuItem: IDisposable | undefined; private _signInRequestItems = new Map(); + private _sessionAccessRequestItems = new Map(); private _accountBadgeDisposable = this._register(new MutableDisposable()); private _authenticationProviders: Map = new Map(); @@ -157,7 +217,11 @@ export class AuthenticationService extends Disposable implements IAuthentication constructor( @IActivityService private readonly activityService: IActivityService, - @IExtensionService private readonly extensionService: IExtensionService + @IExtensionService private readonly extensionService: IExtensionService, + @IStorageService private readonly storageService: IStorageService, + @IRemoteAgentService private readonly remoteAgentService: IRemoteAgentService, + @IDialogService private readonly dialogService: IDialogService, + @IQuickInputService private readonly quickInputService: IQuickInputService ) { super(); this._placeholderMenuItem = MenuRegistry.appendMenuItem(MenuId.AccountsContext, { @@ -255,6 +319,13 @@ export class AuthenticationService extends Disposable implements IAuthentication this._authenticationProviders.delete(id); this._onDidUnregisterAuthenticationProvider.fire({ id, label: provider.label }); this.updateAccountsMenuItem(); + + const accessRequests = this._sessionAccessRequestItems.get(id) || {}; + Object.keys(accessRequests).forEach(extensionId => { + this.removeAccessRequest(id, extensionId); + }); + + this.updateBadgeCount(); } if (!this._authenticationProviders.size) { @@ -278,6 +349,12 @@ export class AuthenticationService extends Disposable implements IAuthentication if (event.added) { await this.updateNewSessionRequests(provider); } + + if (event.removed) { + await this.updateAccessRequests(id, event.removed); + } + + this.updateBadgeCount(); } } @@ -288,12 +365,9 @@ export class AuthenticationService extends Disposable implements IAuthentication } const sessions = await provider.getSessions(); - let changed = false; Object.keys(existingRequestsForProvider).forEach(requestedScopes => { if (sessions.some(session => session.scopes.slice().sort().join('') === requestedScopes)) { - // Request has been completed - changed = true; const sessionRequest = existingRequestsForProvider[requestedScopes]; sessionRequest?.disposables.forEach(item => item.dispose()); @@ -305,22 +379,214 @@ export class AuthenticationService extends Disposable implements IAuthentication } } }); + } - if (changed) { - this._accountBadgeDisposable.clear(); - - if (this._signInRequestItems.size > 0) { - let numberOfRequests = 0; - this._signInRequestItems.forEach(providerRequests => { - Object.keys(providerRequests).forEach(request => { - numberOfRequests += providerRequests[request].requestingExtensionIds.length; - }); + private async updateAccessRequests(providerId: string, removedSessionIds: readonly string[]) { + const providerRequests = this._sessionAccessRequestItems.get(providerId); + if (providerRequests) { + Object.keys(providerRequests).forEach(extensionId => { + removedSessionIds.forEach(removedId => { + const indexOfSession = providerRequests[extensionId].possibleSessions.findIndex(session => session.id === removedId); + if (indexOfSession) { + providerRequests[extensionId].possibleSessions.splice(indexOfSession, 1); + } }); - const badge = new NumberBadge(numberOfRequests, () => nls.localize('sign in', "Sign in requested")); - this._accountBadgeDisposable.value = this.activityService.showAccountsActivity({ badge }); + if (!providerRequests[extensionId].possibleSessions.length) { + this.removeAccessRequest(providerId, extensionId); + } + }); + } + } + + private updateBadgeCount(): void { + this._accountBadgeDisposable.clear(); + + let numberOfRequests = 0; + this._signInRequestItems.forEach(providerRequests => { + Object.keys(providerRequests).forEach(request => { + numberOfRequests += providerRequests[request].requestingExtensionIds.length; + }); + }); + + this._sessionAccessRequestItems.forEach(accessRequest => { + numberOfRequests += Object.keys(accessRequest).length; + }); + + if (numberOfRequests > 0) { + const badge = new NumberBadge(numberOfRequests, () => nls.localize('sign in', "Sign in requested")); + this._accountBadgeDisposable.value = this.activityService.showAccountsActivity({ badge }); + } + } + + private removeAccessRequest(providerId: string, extensionId: string): void { + const providerRequests = this._sessionAccessRequestItems.get(providerId) || {}; + if (providerRequests[extensionId]) { + providerRequests[extensionId].disposables.forEach(d => d.dispose()); + delete providerRequests[extensionId]; + } + } + + async showGetSessionPrompt(providerId: string, accountName: string, extensionId: string, extensionName: string): Promise { + const allowList = readAllowedExtensions(this.storageService, providerId, accountName); + const extensionData = allowList.find(extension => extension.id === extensionId); + if (extensionData) { + return true; + } + + const remoteConnection = this.remoteAgentService.getConnection(); + const isVSO = remoteConnection !== null + ? remoteConnection.remoteAuthority.startsWith('vsonline') || remoteConnection.remoteAuthority.startsWith('codespaces') + : isWeb; + + if (isVSO && VSO_ALLOWED_EXTENSIONS.includes(extensionId)) { + return true; + } + + const providerName = this.getLabel(providerId); + const { choice } = await this.dialogService.show( + Severity.Info, + nls.localize('confirmAuthenticationAccess', "The extension '{0}' wants to access the {1} account '{2}'.", extensionName, providerName, accountName), + [nls.localize('allow', "Allow"), nls.localize('cancel', "Cancel")], + { + cancelId: 1 + } + ); + + const allow = choice === 0; + if (allow) { + allowList.push({ id: extensionId, name: extensionName }); + this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL, StorageTarget.USER); + this.removeAccessRequest(providerId, extensionId); + } + + return allow; + } + + async selectSession(providerId: string, extensionId: string, extensionName: string, availableSessions: AuthenticationSession[]): Promise { + return new Promise((resolve, reject) => { + // This function should be used only when there are sessions to disambiguate. + if (!availableSessions.length) { + reject('No available sessions'); + } + + const quickPick = this.quickInputService.createQuickPick<{ label: string, session?: AuthenticationSession }>(); + quickPick.ignoreFocusOut = true; + const items: { label: string, session?: AuthenticationSession }[] = availableSessions.map(session => { + return { + label: session.account.label, + session: session + }; + }); + + items.push({ + label: nls.localize('useOtherAccount', "Sign in to another account") + }); + + const providerName = this.getLabel(providerId); + + quickPick.items = items; + + quickPick.title = nls.localize( + { + key: 'selectAccount', + comment: ['The placeholder {0} is the name of an extension. {1} is the name of the type of account, such as Microsoft or GitHub.'] + }, + "The extension '{0}' wants to access a {1} account", + extensionName, + providerName); + quickPick.placeholder = nls.localize('getSessionPlateholder', "Select an account for '{0}' to use or Esc to cancel", extensionName); + + quickPick.onDidAccept(async _ => { + const session = quickPick.selectedItems[0].session ?? await this.login(providerId, availableSessions[0].scopes as string[]); + const accountName = session.account.label; + + const allowList = readAllowedExtensions(this.storageService, providerId, accountName); + if (!allowList.find(allowed => allowed.id === extensionId)) { + allowList.push({ id: extensionId, name: extensionName }); + this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL, StorageTarget.USER); + this.removeAccessRequest(providerId, extensionId); + } + + this.storageService.store(`${extensionName}-${providerId}`, session.id, StorageScope.GLOBAL, StorageTarget.MACHINE); + + quickPick.dispose(); + resolve(session); + }); + + quickPick.onDidHide(_ => { + if (!quickPick.selectedItems[0]) { + reject('User did not consent to account access'); + } + + quickPick.dispose(); + }); + + quickPick.show(); + }); + } + + async completeSessionAccessRequest(providerId: string, extensionId: string, extensionName: string): Promise { + const providerRequests = this._sessionAccessRequestItems.get(providerId) || {}; + const existingRequest = providerRequests[extensionId]; + if (!existingRequest) { + return; + } + + const possibleSessions = existingRequest.possibleSessions; + const supportsMultipleAccounts = this.supportsMultipleAccounts(providerId); + + let session: AuthenticationSession | undefined; + if (supportsMultipleAccounts) { + try { + session = await this.selectSession(providerId, extensionId, extensionName, possibleSessions); + } catch (_) { + // ignore cancel + } + } else { + const approved = await this.showGetSessionPrompt(providerId, possibleSessions[0].account.label, extensionId, extensionName); + if (approved) { + session = possibleSessions[0]; } } + + if (session) { + addAccountUsage(this.storageService, providerId, session.account.label, extensionId, extensionName); + const providerName = this.getLabel(providerId); + this._onDidChangeSessions.fire({ providerId, label: providerName, event: { added: [], removed: [], changed: [session.id] } }); + } + } + + requestSessionAccess(providerId: string, extensionId: string, extensionName: string, possibleSessions: AuthenticationSession[]): void { + const providerRequests = this._sessionAccessRequestItems.get(providerId) || {}; + const hasExistingRequest = providerRequests[extensionId]; + if (hasExistingRequest) { + return; + } + + const menuItem = MenuRegistry.appendMenuItem(MenuId.AccountsContext, { + group: '3_accessRequests', + command: { + id: `${providerId}${extensionId}Access`, + title: nls.localize({ + key: 'accessRequest', + comment: ['The placeholder {0} will be replaced with an extension name. (1) is to indicate that this menu item contributes to a badge count'] + }, + "Grant access to {0}... (1)", extensionName) + } + }); + + const accessCommand = CommandsRegistry.registerCommand({ + id: `${providerId}${extensionId}Access`, + handler: async (accessor) => { + const authenticationService = accessor.get(IAuthenticationService); + authenticationService.completeSessionAccessRequest(providerId, extensionId, extensionName); + } + }); + + providerRequests[extensionId] = { possibleSessions, disposables: [menuItem, accessCommand] }; + this._sessionAccessRequestItems.set(providerId, providerRequests); + this.updateBadgeCount(); } async requestNewSession(providerId: string, scopes: string[], extensionId: string, extensionName: string): Promise { From 5d650cd8178453a2d4cc22936c796ccea28d0c77 Mon Sep 17 00:00:00 2001 From: Jackson Kearl Date: Wed, 3 Feb 2021 10:37:40 -0800 Subject: [PATCH 06/20] Allow setting startupEditor to `readme` as either default value or user value. Fix github/codespaces#1580 --- .../workbench/contrib/welcome/page/browser/welcomePage.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/welcome/page/browser/welcomePage.ts b/src/vs/workbench/contrib/welcome/page/browser/welcomePage.ts index 63b1b51442a..99382e5c634 100644 --- a/src/vs/workbench/contrib/welcome/page/browser/welcomePage.ts +++ b/src/vs/workbench/contrib/welcome/page/browser/welcomePage.ts @@ -72,7 +72,12 @@ export class WelcomePageContribution implements IWorkbenchContribution { // Open the welcome even if we opened a set of default editors if ((!editorService.activeEditor || layoutService.openedDefaultEditors) && !hasBackups) { const startupEditorSetting = configurationService.inspect(configurationKey); - const openWithReadme = startupEditorSetting.userValue === 'readme'; + + // 'readme' should not be set in workspace settings to prevent tracking, + // but it can be set as a default (as in codespaces) or a user setting + const openWithReadme = startupEditorSetting.value === 'readme' && + (startupEditorSetting.userValue === 'readme' || startupEditorSetting.defaultValue === 'readme'); + if (openWithReadme) { return Promise.all(contextService.getWorkspace().folders.map(folder => { const folderUri = folder.uri; From e984154e118fc8e60145bfe32bed74711cede6f3 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 3 Feb 2021 11:33:36 -0800 Subject: [PATCH 07/20] merge stream output in rendering --- .../view/output/transforms/richTransform.ts | 2 +- .../view/output/transforms/streamTransform.ts | 4 +- .../view/output/transforms/textHelper.ts | 48 +++----------- .../browser/view/renderers/cellOutput.ts | 64 +++++++++++++++---- 4 files changed, 65 insertions(+), 53 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts index 88deceb6225..bf27a0f05cb 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts @@ -211,7 +211,7 @@ class RichRenderer implements IOutputTransformContribution { renderPlainText(output: IDisplayOutputViewModel, notebookUri: URI, container: HTMLElement): IRenderOutput { const data = output.model.data['text/plain']; const contentNode = DOM.$('.output-plaintext'); - truncatedArrayOfString(contentNode, isArray(data) ? data : [data], this.openerService, this.textFileService, this.themeService, true); + truncatedArrayOfString(contentNode, isArray(data) ? data : [data], this.openerService, this.textFileService, this.themeService); container.appendChild(contentNode); return { type: RenderOutputType.None, hasDynamicHeight: false }; diff --git a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/streamTransform.ts b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/streamTransform.ts index 5f6f9b7737a..17276e2c6b7 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/streamTransform.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/streamTransform.ts @@ -23,8 +23,8 @@ class StreamRenderer implements IOutputTransformContribution { render(viewModel: IStreamOutputViewModel, container: HTMLElement): IRenderOutput { const output = viewModel.model; - const contentNode = DOM.$('.output-stream'); - truncatedArrayOfString(contentNode, [output.text], this.openerService, this.textFileService, this.themeService, false); + const contentNode = DOM.$('span.output-stream'); + truncatedArrayOfString(contentNode, [output.text], this.openerService, this.textFileService, this.themeService); container.appendChild(contentNode); return { type: RenderOutputType.None, hasDynamicHeight: false }; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts index c8dc7731886..635b079bc84 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts @@ -49,7 +49,7 @@ function generateViewMoreElement(outputs: string[], openerService: IOpenerServic return element; } -export function truncatedArrayOfString(container: HTMLElement, outputs: string[], openerService: IOpenerService, textFileService: ITextFileService, themeService: IThemeService, renderANSI: boolean) { +export function truncatedArrayOfString(container: HTMLElement, outputs: string[], openerService: IOpenerService, textFileService: ITextFileService, themeService: IThemeService) { const fullLen = outputs.reduce((p, c) => { return p + c.length; }, 0); @@ -65,14 +65,7 @@ export function truncatedArrayOfString(container: HTMLElement, outputs: string[] const sizeBufferLimitPosition = buffer.getPositionAt(SIZE_LIMIT); if (sizeBufferLimitPosition.lineNumber < LINES_LIMIT) { const truncatedText = buffer.getValueInRange(new Range(1, 1, sizeBufferLimitPosition.lineNumber, sizeBufferLimitPosition.column), EndOfLinePreference.TextDefined); - if (renderANSI) { - container.appendChild(handleANSIOutput(truncatedText, themeService)); - } else { - const pre = DOM.$('pre'); - pre.innerText = truncatedText; - container.appendChild(pre); - } - + container.appendChild(handleANSIOutput(truncatedText, themeService)); // view more ... container.appendChild(generateViewMoreElement(outputs, openerService, textFileService)); return; @@ -89,42 +82,19 @@ export function truncatedArrayOfString(container: HTMLElement, outputs: string[] if (buffer.getLineCount() < LINES_LIMIT) { const lineCount = buffer.getLineCount(); const fullRange = new Range(1, 1, lineCount, Math.max(1, buffer.getLineLastNonWhitespaceColumn(lineCount))); - - if (renderANSI) { - const pre = DOM.$('pre'); - container.appendChild(pre); - - pre.appendChild(handleANSIOutput(buffer.getValueInRange(fullRange, EndOfLinePreference.TextDefined), themeService)); - } else { - const pre = DOM.$('pre'); - container.appendChild(pre); - pre.innerText = buffer.getValueInRange(fullRange, EndOfLinePreference.TextDefined); - } + container.appendChild(handleANSIOutput(buffer.getValueInRange(fullRange, EndOfLinePreference.TextDefined), themeService)); return; } - if (renderANSI) { - const pre = DOM.$('pre'); - container.appendChild(pre); - pre.appendChild(handleANSIOutput(buffer.getValueInRange(new Range(1, 1, LINES_LIMIT - 5, buffer.getLineLastNonWhitespaceColumn(LINES_LIMIT - 5)), EndOfLinePreference.TextDefined), themeService)); - } else { - const pre = DOM.$('pre'); - pre.innerText = buffer.getValueInRange(new Range(1, 1, LINES_LIMIT - 5, buffer.getLineLastNonWhitespaceColumn(LINES_LIMIT - 5)), EndOfLinePreference.TextDefined); - container.appendChild(pre); - } - + const pre = DOM.$('pre'); + container.appendChild(pre); + pre.appendChild(handleANSIOutput(buffer.getValueInRange(new Range(1, 1, LINES_LIMIT - 5, buffer.getLineLastNonWhitespaceColumn(LINES_LIMIT - 5)), EndOfLinePreference.TextDefined), themeService)); // view more ... container.appendChild(generateViewMoreElement(outputs, openerService, textFileService)); const lineCount = buffer.getLineCount(); - if (renderANSI) { - const pre = DOM.$('div'); - container.appendChild(pre); - pre.appendChild(handleANSIOutput(buffer.getValueInRange(new Range(lineCount - 5, 1, lineCount, buffer.getLineLastNonWhitespaceColumn(lineCount)), EndOfLinePreference.TextDefined), themeService)); - } else { - const post = DOM.$('div'); - post.innerText = buffer.getValueInRange(new Range(lineCount - 5, 1, lineCount, buffer.getLineLastNonWhitespaceColumn(lineCount)), EndOfLinePreference.TextDefined); - container.appendChild(post); - } + const pre2 = DOM.$('div'); + container.appendChild(pre2); + pre2.appendChild(handleANSIOutput(buffer.getValueInRange(new Range(lineCount - 5, 1, lineCount, buffer.getLineLastNonWhitespaceColumn(lineCount)), EndOfLinePreference.TextDefined), themeService)); } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts index bca1187cd94..e56c4367c5f 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts @@ -36,6 +36,16 @@ export class OutputElement extends Disposable { domNode!: HTMLElement; renderResult?: IRenderOutput; + public useDedicatedDOM: boolean = true; + + get domClientHeight() { + if (this.useDedicatedDOM) { + return this.domNode.clientHeight; + } else { + return 0; + } + } + constructor( private notebookEditor: INotebookEditor, private notebookService: INotebookService, @@ -47,6 +57,17 @@ export class OutputElement extends Disposable { super(); } + detach() { + this.domNode.parentElement?.removeChild(this.domNode); + } + + updateDOMTop(top: number) { + if (this.useDedicatedDOM) { + this.domNode.style.top = `${top}px`; + } + } + + render(index: number, beforeElement?: HTMLElement) { if (this.viewCell.metadata.outputCollapsed) { return; @@ -58,10 +79,11 @@ export class OutputElement extends Disposable { const notebookTextModel = this.notebookEditor.viewModel.notebookDocument; - const outputItemDiv = document.createElement('div'); + let outputItemDiv; let result: IRenderOutput | undefined = undefined; if (this.output.isDisplayOutput()) { + outputItemDiv = document.createElement('div'); const [mimeTypes, pick] = this.output.resolveMimeTypes(notebookTextModel); if (mimeTypes.length > 1) { outputItemDiv.style.position = 'relative'; @@ -106,8 +128,27 @@ export class OutputElement extends Disposable { this.output.pickedMimeType = pick; } + } else if (this.output.isStreamOutput()) { + if (!beforeElement && this.outputContainer.lastChild && (this.outputContainer.lastChild).classList.contains('stream-output') && this.output.isStreamOutput()) { + this.useDedicatedDOM = false; + // the previous output and this one are both stream output + outputItemDiv = this.outputContainer.lastChild as HTMLElement; + const innerContainer = outputItemDiv.lastChild && (outputItemDiv.lastChild).classList.contains('output-inner-container') ? outputItemDiv.lastChild as HTMLElement : document.createElement('div'); + outputItemDiv.classList.add('stream-output'); + DOM.append(outputItemDiv, innerContainer); + + result = this.notebookEditor.getOutputRenderer().render(this.output, innerContainer, undefined, this.getNotebookUri(),); + } else { + outputItemDiv = document.createElement('div'); + const innerContainer = DOM.$('.output-inner-container'); + outputItemDiv.classList.add('stream-output'); + DOM.append(outputItemDiv, innerContainer); + + result = this.notebookEditor.getOutputRenderer().render(this.output, innerContainer, undefined, this.getNotebookUri(),); + } } else { // for text and error, there is no mimetype + outputItemDiv = document.createElement('div'); const innerContainer = DOM.$('.output-inner-container'); DOM.append(outputItemDiv, innerContainer); @@ -124,7 +165,7 @@ export class OutputElement extends Disposable { if (beforeElement) { this.outputContainer.insertBefore(outputItemDiv, beforeElement); - } else { + } else if (this.useDedicatedDOM) { this.outputContainer.appendChild(outputItemDiv); } @@ -165,11 +206,13 @@ export class OutputElement extends Disposable { this.resizeListener.add(elementSizeObserver); this.viewCell.updateOutputHeight(index, clientHeight); } else if (result.type === RenderOutputType.None) { // no-op if it's a webview - const clientHeight = Math.ceil(outputItemDiv.clientHeight); - this.viewCell.updateOutputHeight(index, clientHeight); + if (this.useDedicatedDOM) { + const clientHeight = Math.ceil(outputItemDiv.clientHeight); + this.viewCell.updateOutputHeight(index, clientHeight); - const top = this.viewCell.getOutputOffsetInContainer(index); - outputItemDiv.style.top = `${top}px`; + const top = this.viewCell.getOutputOffsetInContainer(index); + outputItemDiv.style.top = `${top}px`; + } } } @@ -268,7 +311,7 @@ export class OutputContainer extends Disposable { const index = viewCell.outputsViewModels.indexOf(key); if (index >= 0) { const top = this.viewCell.getOutputOffsetInContainer(index); - value.domNode.style.top = `${top}px`; + value.updateDOMTop(top); } }); })); @@ -330,8 +373,7 @@ export class OutputContainer extends Disposable { if (renderedOutput.renderResult.type !== RenderOutputType.None) { this.notebookEditor.createInset(this.viewCell, renderedOutput.renderResult as IInsetRenderOutput, this.viewCell.getOutputOffset(index)); } else { - // Anything else, just update the height - this.viewCell.updateOutputHeight(index, renderedOutput.domNode.clientHeight); + this.viewCell.updateOutputHeight(index, renderedOutput.domClientHeight); } } else { // Wasn't previously rendered, render it now @@ -352,7 +394,7 @@ export class OutputContainer extends Disposable { this.viewCell.outputsViewModels.forEach((o, i) => { const renderedOutput = this.outputEntries.get(o); if (renderedOutput && renderedOutput.renderResult && renderedOutput.renderResult.type === RenderOutputType.None && !renderedOutput.renderResult.hasDynamicHeight) { - this.viewCell.updateOutputHeight(i, renderedOutput.domNode.clientHeight); + this.viewCell.updateOutputHeight(i, renderedOutput.domClientHeight); } }); } @@ -420,7 +462,7 @@ export class OutputContainer extends Disposable { // already removed removedKeys.push(key); // remove element from DOM - this.templateData?.outputContainer?.removeChild(value.domNode); + value.detach(); if (key.isDisplayOutput()) { this.notebookEditor.removeInset(key); } From 041f9b975b04e1eecaeb40783547ee00efc21850 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 3 Feb 2021 11:47:46 -0800 Subject: [PATCH 08/20] fix missing total height change event. --- .../contrib/notebook/browser/view/renderers/codeCell.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts index f22e6fea28c..74e072419b0 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts @@ -122,6 +122,12 @@ export class CodeCell extends Disposable { } })); + this._register(viewCell.onDidChangeLayout((e) => { + if (e.totalHeight) { + this.relayoutCell(); + } + })); + this._register(templateData.editor.onDidContentSizeChange((e) => { if (e.contentHeightChanged) { if (this.viewCell.layoutInfo.editorHeight !== e.contentHeight) { From 9ddea6f385e1d95b688f97e3f4a5b1cb37caf201 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Wed, 3 Feb 2021 14:16:12 -0800 Subject: [PATCH 09/20] chore: bump electron@11.2.2 --- .yarnrc | 2 +- cgmanifest.json | 4 ++-- extensions/emmet/yarn.lock | 8 -------- package.json | 4 ++-- yarn.lock | 8 ++++---- 5 files changed, 9 insertions(+), 17 deletions(-) diff --git a/.yarnrc b/.yarnrc index 83146ea6002..9738cd535ab 100644 --- a/.yarnrc +++ b/.yarnrc @@ -1,3 +1,3 @@ disturl "https://electronjs.org/headers" -target "11.2.1" +target "11.2.2" runtime "electron" diff --git a/cgmanifest.json b/cgmanifest.json index a47ba0d6247..f9aa3cd5fb0 100644 --- a/cgmanifest.json +++ b/cgmanifest.json @@ -60,12 +60,12 @@ "git": { "name": "electron", "repositoryUrl": "https://github.com/electron/electron", - "commitHash": "8805b996e0d8cfb6e3921f9b586366bafb125b59" + "commitHash": "805e442ff873e10735a1ea18021f491597afa885" } }, "isOnlyProductionDependency": true, "license": "MIT", - "version": "11.2.1" + "version": "11.2.2" }, { "component": { diff --git a/extensions/emmet/yarn.lock b/extensions/emmet/yarn.lock index d47b466909f..b2b572223e9 100644 --- a/extensions/emmet/yarn.lock +++ b/extensions/emmet/yarn.lock @@ -58,16 +58,8 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-12.19.15.tgz#0de7e978fb43db62da369db18ea088a63673c182" integrity sha512-lowukE3GUI+VSYSu6VcBXl14d61Rp5hA1D+61r16qnwC0lYNSqdxcvRh0pswejorHfS+HgwBasM8jLXz0/aOsw== -"emmet@git+https://github.com/rzhao271/emmet.git#1b2df677d8925ef5ea6da9df8845968403979a0a": - version "2.3.0" - resolved "git+https://github.com/rzhao271/emmet.git#1b2df677d8925ef5ea6da9df8845968403979a0a" - dependencies: - "@emmetio/abbreviation" "^2.2.0" - "@emmetio/css-abbreviation" "^2.1.2" - "emmet@https://github.com/rzhao271/emmet.git#1b2df677d8925ef5ea6da9df8845968403979a0a": version "2.3.0" - uid "1b2df677d8925ef5ea6da9df8845968403979a0a" resolved "https://github.com/rzhao271/emmet.git#1b2df677d8925ef5ea6da9df8845968403979a0a" dependencies: "@emmetio/abbreviation" "^2.2.0" diff --git a/package.json b/package.json index ca26be9f76f..958ca329ab1 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "compile-web": "node --max_old_space_size=4095 ./node_modules/gulp/bin/gulp.js compile-web", "watch-web": "node --max_old_space_size=4095 ./node_modules/gulp/bin/gulp.js watch-web", "eslint": "node build/eslint", - "electron-rebuild": "electron-rebuild --arch=arm64 --force --version=11.2.1", + "electron-rebuild": "electron-rebuild --arch=arm64 --force --version=11.2.2", "playwright-install": "node build/azure-pipelines/common/installPlaywright.js", "compile-build": "node --max_old_space_size=4095 ./node_modules/gulp/bin/gulp.js compile-build", "compile-extensions-build": "node --max_old_space_size=4095 ./node_modules/gulp/bin/gulp.js compile-extensions-build", @@ -125,7 +125,7 @@ "cssnano": "^4.1.10", "debounce": "^1.0.0", "deemon": "^1.4.0", - "electron": "11.2.1", + "electron": "11.2.2", "electron-rebuild": "2.0.3", "eslint": "6.8.0", "eslint-plugin-jsdoc": "^19.1.0", diff --git a/yarn.lock b/yarn.lock index d70f707bcfd..930553d75b0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2914,10 +2914,10 @@ electron-to-chromium@^1.3.634: resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.642.tgz#8b884f50296c2ae2a9997f024d0e3e57facc2b94" integrity sha512-cev+jOrz/Zm1i+Yh334Hed6lQVOkkemk2wRozfMF4MtTR7pxf3r3L5Rbd7uX1zMcEqVJ7alJBnJL7+JffkC6FQ== -electron@11.2.1: - version "11.2.1" - resolved "https://registry.yarnpkg.com/electron/-/electron-11.2.1.tgz#8641dd1a62911a1144e0c73c34fd9f37ccc65c2b" - integrity sha512-Im1y29Bnil+Nzs+FCTq01J1OtLbs+2ZGLLllaqX/9n5GgpdtDmZhS/++JHBsYZ+4+0n7asO+JKQgJD+CqPClzg== +electron@11.2.2: + version "11.2.2" + resolved "https://registry.yarnpkg.com/electron/-/electron-11.2.2.tgz#c2e53eb56bd21ae1dc01bc781f2f6bcbe0c18033" + integrity sha512-+OitkBrnCFwOF5LXAeNnfIJDKhdBm77jboc16WCIpDsCyT+JpGsKK4y6o30nRZq3zC+wZggUm5w6Ujw5n76CGg== dependencies: "@electron/get" "^1.0.1" "@types/node" "^12.0.12" From 55d04df641d0b3a43a55ce13ffdf2ab48f8f7b5b Mon Sep 17 00:00:00 2001 From: Jackson Kearl Date: Wed, 3 Feb 2021 15:45:35 -0800 Subject: [PATCH 10/20] Fix #115316: Getting started back button looks unstyled --- .../gettingStarted/browser/gettingStarted.css | 10 ++++--- .../gettingStarted/browser/gettingStarted.ts | 26 +++++++++++++++++++ .../browser/vs_code_editor_getting_started.ts | 2 +- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.css b/src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.css index 6b442829391..09b136cbc10 100644 --- a/src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.css +++ b/src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.css @@ -238,10 +238,11 @@ outline-style: solid; } -.monaco-workbench .part.editor > .content .walkThroughContent .gettingStartedContainer .prev-button { +.monaco-workbench .part.editor > .content .walkThroughContent .gettingStartedContainer .prev-button.button-link { position: absolute; - left: -2px; - top: -10px; + left: 40px; + top: 5px; + padding: 0 2px 2px; margin: 10px; z-index: 1; } @@ -252,7 +253,8 @@ .monaco-workbench .part.editor > .content .walkThroughContent .gettingStartedContainer .prev-button .codicon { position: relative; - top: 2px; + top: 3px; + left: -4px; } .monaco-workbench .part.editor > .content .walkThroughContent .gettingStartedContainer .gettingStartedSlide .skip { diff --git a/src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.ts b/src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.ts index de64ed7d444..73b78777e48 100644 --- a/src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.ts +++ b/src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.ts @@ -294,8 +294,11 @@ export class GettingStartedPage extends Disposable { } this.buildCategorySlide(container, this.editorInput.selectedCategory, this.editorInput.selectedTask); categoriesSlide.classList.add('prev'); + this.setButtonEnablement(container, 'details'); } else { tasksSlide.classList.add('next'); + this.focusFirstUncompletedCategory(container); + this.setButtonEnablement(container, 'categories'); } setTimeout(() => assertIsDefined(container.querySelector('.gettingStartedContainer')).classList.add('animationReady'), 0); } @@ -339,6 +342,7 @@ export class GettingStartedPage extends Disposable { this.buildCategorySlide(container, categoryID); slides[currentSlide].classList.add('prev'); slides[currentSlide + 1].classList.remove('next'); + this.setButtonEnablement(container, 'details'); } }); } @@ -418,9 +422,30 @@ export class GettingStartedPage extends Disposable { if (currentSlide > 0) { slides[currentSlide].classList.add('next'); assertIsDefined(slides[currentSlide - 1]).classList.remove('prev'); + this.setButtonEnablement(container, 'categories'); } + this.focusFirstUncompletedCategory(container); }); } + + private focusFirstUncompletedCategory(container: HTMLElement) { + let toFocus!: HTMLElement; + container.querySelectorAll('.category-progress').forEach(progress => { + const progressAmount = assertIsDefined(progress.querySelector('.progress-bar-inner') as HTMLDivElement).style.width; + if (!toFocus && progressAmount !== '100%') { toFocus = assertIsDefined(progress.parentElement?.parentElement); } + }); + (toFocus ?? assertIsDefined(container.querySelector('button.skip')) as HTMLButtonElement).focus(); + } + + private setButtonEnablement(container: HTMLElement, toEnable: 'details' | 'categories') { + if (toEnable === 'categories') { + container.querySelector('.gettingStartedSlideDetails')!.querySelectorAll('button').forEach(button => button.disabled = true); + container.querySelector('.gettingStartedSlideCategory')!.querySelectorAll('button').forEach(button => button.disabled = false); + } else { + container.querySelector('.gettingStartedSlideDetails')!.querySelectorAll('button').forEach(button => button.disabled = false); + container.querySelector('.gettingStartedSlideCategory')!.querySelectorAll('button').forEach(button => button.disabled = true); + } + } } export class GettingStartedInputFactory implements IEditorInputFactory { @@ -503,6 +528,7 @@ registerThemingParticipant((theme, collector) => { if (link) { collector.addRule(`.monaco-workbench .part.editor > .content .walkThroughContent .gettingStartedContainer a { color: ${link}; }`); collector.addRule(`.monaco-workbench .part.editor > .content .walkThroughContent .gettingStartedContainer .button-link { color: ${link}; }`); + collector.addRule(`.monaco-workbench .part.editor > .content .walkThroughContent .gettingStartedContainer .button-link * { color: ${link}; }`); } const activeLink = theme.getColor(textLinkActiveForeground); if (activeLink) { diff --git a/src/vs/workbench/contrib/welcome/gettingStarted/browser/vs_code_editor_getting_started.ts b/src/vs/workbench/contrib/welcome/gettingStarted/browser/vs_code_editor_getting_started.ts index 9c09f066d84..84de9db6ac6 100644 --- a/src/vs/workbench/contrib/welcome/gettingStarted/browser/vs_code_editor_getting_started.ts +++ b/src/vs/workbench/contrib/welcome/gettingStarted/browser/vs_code_editor_getting_started.ts @@ -17,7 +17,7 @@ export default () => `
- Back +