From d115bb2a9773a2ef0e1b2f81fb758b35ccfcfcb0 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Wed, 24 Mar 2021 14:28:30 -0700 Subject: [PATCH] clean up statProvider, use existsFile, fix #119807 --- .../contrib/terminal/node/terminalProfiles.ts | 66 +++++++------------ .../test/node/terminalProfiles.test.ts | 13 +--- 2 files changed, 24 insertions(+), 55 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/node/terminalProfiles.ts b/src/vs/workbench/contrib/terminal/node/terminalProfiles.ts index a1d12c68129..525b7b72777 100644 --- a/src/vs/workbench/contrib/terminal/node/terminalProfiles.ts +++ b/src/vs/workbench/contrib/terminal/node/terminalProfiles.ts @@ -13,15 +13,15 @@ import * as cp from 'child_process'; import { ExtHostVariableResolverService } from 'vs/workbench/api/common/extHostDebugService'; import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; import { ILogService } from 'vs/platform/log/common/log'; +import * as pfs from 'vs/base/node/pfs'; let profileSources: Map | undefined; export function detectAvailableProfiles(quickLaunchOnly: boolean, logService?: ILogService, config?: ITerminalConfiguration, variableResolver?: ExtHostVariableResolverService, workspaceFolder?: IWorkspaceFolder, statProvider?: IStatProvider, testPaths?: string[]): Promise { - const provider = statProvider ? statProvider : fs.promises; - return platform.isWindows ? detectAvailableWindowsProfiles(quickLaunchOnly, provider, logService, config?.showQuickLaunchWslProfiles, config?.profiles.windows, variableResolver, workspaceFolder) : detectAvailableUnixProfiles(provider, logService, quickLaunchOnly, platform.isMacintosh ? config?.profiles.osx : config?.profiles.linux, testPaths, variableResolver, workspaceFolder); + return platform.isWindows ? detectAvailableWindowsProfiles(quickLaunchOnly, statProvider, logService, config?.showQuickLaunchWslProfiles, config?.profiles.windows, variableResolver, workspaceFolder) : detectAvailableUnixProfiles(statProvider, logService, quickLaunchOnly, platform.isMacintosh ? config?.profiles.osx : config?.profiles.linux, testPaths, variableResolver, workspaceFolder); } -async function detectAvailableWindowsProfiles(quickLaunchOnly: boolean, statProvider: IStatProvider, logService?: ILogService, showQuickLaunchWslProfiles?: boolean, configProfiles?: { [key: string]: ITerminalProfileObject }, variableResolver?: ExtHostVariableResolverService, workspaceFolder?: IWorkspaceFolder): Promise { +async function detectAvailableWindowsProfiles(quickLaunchOnly: boolean, statProvider?: IStatProvider, logService?: ILogService, showQuickLaunchWslProfiles?: boolean, configProfiles?: { [key: string]: ITerminalProfileObject }, variableResolver?: ExtHostVariableResolverService, workspaceFolder?: IWorkspaceFolder): Promise { // Determine the correct System32 path. We want to point to Sysnative // when the 32-bit version of VS Code is running on a 64-bit machine. // The reason for this is because PowerShell's important PSReadline @@ -61,7 +61,7 @@ async function detectAvailableWindowsProfiles(quickLaunchOnly: boolean, statProv applyConfigProfilesToMap(configProfiles, detectedProfiles); - const resultProfiles: ITerminalProfile[] = await transformToTerminalProfiles(detectedProfiles.entries(), statProvider, logService, variableResolver, workspaceFolder); + const resultProfiles: ITerminalProfile[] = await transformToTerminalProfiles(detectedProfiles.entries(), logService, statProvider, variableResolver, workspaceFolder); if (!quickLaunchOnly || (quickLaunchOnly && showQuickLaunchWslProfiles)) { resultProfiles.push(... await getWslProfiles(`${system32Path}\\${useWSLexe ? 'wsl.exe' : 'bash.exe'}`, showQuickLaunchWslProfiles)); @@ -70,7 +70,7 @@ async function detectAvailableWindowsProfiles(quickLaunchOnly: boolean, statProv return resultProfiles; } -async function transformToTerminalProfiles(entries: IterableIterator<[string, ITerminalProfileObject]>, statProvider: IStatProvider, logService?: ILogService, variableResolver?: ExtHostVariableResolverService, workspaceFolder?: IWorkspaceFolder): Promise { +async function transformToTerminalProfiles(entries: IterableIterator<[string, ITerminalProfileObject]>, logService?: ILogService, statProvider?: IStatProvider, variableResolver?: ExtHostVariableResolverService, workspaceFolder?: IWorkspaceFolder): Promise { const resultProfiles: ITerminalProfile[] = []; for (const [profileName, profile] of entries) { if (profile === null) { continue; } @@ -190,22 +190,29 @@ async function getWslProfiles(wslPath: string, showQuickLaunchWslProfiles?: bool return []; } -async function detectAvailableUnixProfiles(statProvider: IStatProvider, logService?: ILogService, quickLaunchOnly?: boolean, configProfiles?: { [key: string]: ITerminalProfileObject }, testPaths?: string[], variableResolver?: ExtHostVariableResolverService, workspaceFolder?: IWorkspaceFolder): Promise { +async function detectAvailableUnixProfiles(statProvider?: IStatProvider, logService?: ILogService, quickLaunchOnly?: boolean, configProfiles?: { [key: string]: ITerminalProfileObject }, testPaths?: string[], variableResolver?: ExtHostVariableResolverService, workspaceFolder?: IWorkspaceFolder): Promise { const detectedProfiles: Map = new Map(); // Add non-quick launch profiles if (!quickLaunchOnly) { const contents = await fs.promises.readFile('/etc/shells', 'utf8'); const profiles = testPaths || contents.split('\n').filter(e => e.trim().indexOf('#') !== 0 && e.trim().length > 0); + const counts: Map = new Map(); for (const profile of profiles) { - const profileName = basename(profile); + let profileName = basename(profile); + let count = counts.get(profileName) || 0; + count++; + if (count > 1) { + profileName = `${profileName} (${count})`; + } + counts.set(profileName, count); detectedProfiles.set(profileName, { path: profile, isAutoDetected: true }); } } applyConfigProfilesToMap(configProfiles, detectedProfiles); - return await transformToTerminalProfiles(detectedProfiles.entries(), statProvider, logService, variableResolver, workspaceFolder); + return await transformToTerminalProfiles(detectedProfiles.entries(), logService, statProvider, variableResolver, workspaceFolder); } function applyConfigProfilesToMap(configProfiles: { [key: string]: ITerminalProfileObject } | undefined, profilesMap: Map) { @@ -221,7 +228,7 @@ function applyConfigProfilesToMap(configProfiles: { [key: string]: ITerminalProf } } -async function validateProfilePaths(profileName: string, potentialPaths: string[], statProvider: IStatProvider, args?: string[] | string, overrideName?: string, isAutoDetected?: boolean, logService?: ILogService): Promise { +async function validateProfilePaths(profileName: string, potentialPaths: string[], statProvider?: IStatProvider, args?: string[] | string, overrideName?: string, isAutoDetected?: boolean, logService?: ILogService): Promise { if (potentialPaths.length === 0) { return Promise.resolve(undefined); } @@ -230,51 +237,22 @@ async function validateProfilePaths(profileName: string, potentialPaths: string[ return validateProfilePaths(profileName, potentialPaths, statProvider, args, overrideName, isAutoDetected); } - const profile = { - profileName, - path, - args, - overrideName, - isAutoDetected - }; + const profile = { profileName, path, args, overrideName, isAutoDetected }; if (basename(path) === path) { return profile; } - try { - const result = await fs.promises.stat(normalize(path)); - if (result.isFile() || result.isSymbolicLink()) { - return profile; - } - } catch (e) { - // Also try using lstat as some symbolic links on Windows - // throw 'permission denied' using 'stat' but don't throw - // using 'lstat' - try { - const result = await fs.promises.lstat(normalize(path)); - if (result.isFile() || result.isSymbolicLink()) { - { - return profile; - } - } - } - catch (e) { - // noop - } + const result = statProvider ? await statProvider.existsFile(path) : await pfs.SymlinkSupport.existsFile(normalize(path)); + if (result) { + return profile; } + return validateProfilePaths(profileName, potentialPaths, statProvider, args, overrideName, isAutoDetected); } export interface IStatProvider { - stat(path: string): Promise<{ - isFile(): boolean; - isSymbolicLink(): boolean; - }>, - lstat(path: string): Promise<{ - isFile(): boolean; - isSymbolicLink(): boolean; - }> + existsFile(path: string): Promise, } interface IPotentialTerminalProfile { diff --git a/src/vs/workbench/contrib/terminal/test/node/terminalProfiles.test.ts b/src/vs/workbench/contrib/terminal/test/node/terminalProfiles.test.ts index 9243c1ac6b8..7a4cb47feca 100644 --- a/src/vs/workbench/contrib/terminal/test/node/terminalProfiles.test.ts +++ b/src/vs/workbench/contrib/terminal/test/node/terminalProfiles.test.ts @@ -114,17 +114,8 @@ suite('Workbench - TerminalProfiles', () => { function createStatProvider(expectedPaths: string[]): IStatProvider { const provider = { - async stat(path: string) { - return { - isFile: () => expectedPaths.includes(path), - isSymbolicLink: () => false - }; - }, - async lstat(path: string) { - return { - isFile: () => expectedPaths.includes(path), - isSymbolicLink: () => false - }; + async existsFile(path: string): Promise { + return expectedPaths.includes(path); } }; return provider;