Skip to content

Commit 2cd1bf6

Browse files
TyriarTyriar
authored
Merge pull request microsoft#153204 from babakks/fix-repeated-cwd-for-new-terminal-picker
🐛 Fix repeated CWD entries when creating new terminal in multi-root workspace
2 parents a7b9ce8 + ce4aed5 commit 2cd1bf6

File tree

2 files changed

+169
-21
lines changed

2 files changed

+169
-21
lines changed

‎src/vs/workbench/contrib/terminal/browser/terminalActions.ts‎

Lines changed: 86 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,27 @@ import { ITerminalQuickPickItem } from 'vs/workbench/contrib/terminal/browser/te
5252
import { IThemeService } from 'vs/platform/theme/common/themeService';
5353
import { getIconId, getColorClass, getUriClasses } from 'vs/workbench/contrib/terminal/browser/terminalIcon';
5454
import { clearShellFileHistory, getCommandHistory } from 'vs/workbench/contrib/terminal/common/history';
55+
import { IModelService } from 'vs/editor/common/services/model';
56+
import { ILanguageService } from 'vs/editor/common/languages/language';
57+
import { CancellationToken } from 'vs/base/common/cancellation';
58+
import { dirname } from 'vs/base/common/resources';
59+
import { getIconClasses } from 'vs/editor/common/services/getIconClasses';
60+
import { FileKind, IFileService } from 'vs/platform/files/common/files';
5561
import { Categories } from 'vs/platform/action/common/actionCommonCategories';
5662
import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
5763
import { TerminalCapability } from 'vs/platform/terminal/common/capabilities/capabilities';
58-
import { IFileService } from 'vs/platform/files/common/files';
5964
import { VSBuffer } from 'vs/base/common/buffer';
6065

6166
export const switchTerminalActionViewItemSeparator = '\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500';
6267
export const switchTerminalShowTabsTitle = localize('showTerminalTabs', "Show Tabs");
6368

69+
export interface WorkspaceFolderCwdPair {
70+
folder: IWorkspaceFolder;
71+
cwd: URI;
72+
isAbsolute: boolean;
73+
isOverridden: boolean;
74+
}
75+
6476
export async function getCwdForSplit(configHelper: ITerminalConfigHelper, instance: ITerminalInstance, folders?: IWorkspaceFolder[], commandService?: ICommandService): Promise<string | URI | undefined> {
6577
switch (configHelper.config.splitCwd) {
6678
case 'workspaceRoot':
@@ -1897,8 +1909,6 @@ export function registerTerminalActions() {
18971909
const terminalGroupService = accessor.get(ITerminalGroupService);
18981910
const workspaceContextService = accessor.get(IWorkspaceContextService);
18991911
const commandService = accessor.get(ICommandService);
1900-
const configurationService = accessor.get(IConfigurationService);
1901-
const configurationResolverService = accessor.get(IConfigurationResolverService);
19021912
const folders = workspaceContextService.getWorkspace().folders;
19031913
if (eventOrOptions && eventOrOptions instanceof MouseEvent && (eventOrOptions.altKey || eventOrOptions.ctrlKey)) {
19041914
await terminalService.createTerminal({ location: { splitActiveTerminal: true } });
@@ -1914,27 +1924,12 @@ export function registerTerminalActions() {
19141924
// single root
19151925
instance = await terminalService.createTerminal(eventOrOptions);
19161926
} else {
1917-
const options: IPickOptions<IQuickPickItem> = {
1918-
placeHolder: localize('workbench.action.terminal.newWorkspacePlaceholder', "Select current working directory for new terminal")
1919-
};
1920-
const workspace = await commandService.executeCommand<IWorkspaceFolder>(PICK_WORKSPACE_FOLDER_COMMAND_ID, [options]);
1921-
if (!workspace) {
1927+
const cwd = (await pickTerminalCwd(accessor))?.cwd;
1928+
if (!cwd) {
19221929
// Don't create the instance if the workspace picker was canceled
19231930
return;
19241931
}
1925-
eventOrOptions.cwd = workspace.uri;
1926-
const cwdConfig = configurationService.getValue(TerminalSettingId.Cwd, { resource: workspace.uri });
1927-
if (typeof cwdConfig === 'string' && cwdConfig.length > 0) {
1928-
const resolvedCwdConfig = await configurationResolverService.resolveAsync(workspace, cwdConfig);
1929-
if (isAbsolute(resolvedCwdConfig) || resolvedCwdConfig.startsWith(AbstractVariableResolverService.VARIABLE_LHS)) {
1930-
eventOrOptions.cwd = URI.from({
1931-
scheme: workspace.uri.scheme,
1932-
path: resolvedCwdConfig
1933-
});
1934-
} else {
1935-
eventOrOptions.cwd = URI.joinPath(workspace.uri, resolvedCwdConfig);
1936-
}
1937-
}
1932+
eventOrOptions.cwd = cwd;
19381933
instance = await terminalService.createTerminal(eventOrOptions);
19391934
}
19401935
terminalService.setActiveInstance(instance);
@@ -2603,3 +2598,73 @@ function getActiveInstance(accessor: ServicesAccessor, resource: unknown): ITerm
26032598
const instance = terminalService.getInstanceFromResource(castedResource) || terminalService.activeInstance;
26042599
return instance;
26052600
}
2601+
2602+
async function pickTerminalCwd(accessor: ServicesAccessor, cancel?: CancellationToken): Promise<WorkspaceFolderCwdPair | undefined> {
2603+
const quickInputService = accessor.get(IQuickInputService);
2604+
const labelService = accessor.get(ILabelService);
2605+
const contextService = accessor.get(IWorkspaceContextService);
2606+
const modelService = accessor.get(IModelService);
2607+
const languageService = accessor.get(ILanguageService);
2608+
const configurationService = accessor.get(IConfigurationService);
2609+
const configurationResolverService = accessor.get(IConfigurationResolverService);
2610+
2611+
const folders = contextService.getWorkspace().folders;
2612+
if (!folders.length) {
2613+
return;
2614+
}
2615+
2616+
const folderCwdPairs = await Promise.all(folders.map(x => resolveWorkspaceFolderCwd(x, configurationService, configurationResolverService)));
2617+
const shrinkedPairs = shrinkWorkspaceFolderCwdPairs(folderCwdPairs);
2618+
2619+
if (shrinkedPairs.length === 1) {
2620+
return shrinkedPairs[0];
2621+
}
2622+
2623+
type Item = IQuickPickItem & { pair: WorkspaceFolderCwdPair };
2624+
const folderPicks: Item[] = shrinkedPairs.map(pair => ({
2625+
label: pair.folder.name,
2626+
description: pair.isOverridden
2627+
? localize('workbench.action.terminal.overriddenCwdDescription', "(Overriden) {0}", labelService.getUriLabel(pair.cwd, { relative: !pair.isAbsolute }))
2628+
: labelService.getUriLabel(dirname(pair.cwd), { relative: true }),
2629+
pair: pair,
2630+
iconClasses: getIconClasses(modelService, languageService, pair.cwd, FileKind.ROOT_FOLDER)
2631+
}));
2632+
const options: IPickOptions<Item> = {
2633+
placeHolder: localize('workbench.action.terminal.newWorkspacePlaceholder', "Select current working directory for new terminal"),
2634+
matchOnDescription: true,
2635+
canPickMany: false,
2636+
};
2637+
2638+
const token: CancellationToken = cancel || CancellationToken.None;
2639+
const pick = await quickInputService.pick<Item>(folderPicks, options, token);
2640+
return pick?.pair;
2641+
}
2642+
2643+
async function resolveWorkspaceFolderCwd(folder: IWorkspaceFolder, configurationService: IConfigurationService, configurationResolverService: IConfigurationResolverService): Promise<WorkspaceFolderCwdPair> {
2644+
const cwdConfig = configurationService.getValue(TerminalSettingId.Cwd, { resource: folder.uri });
2645+
if (typeof cwdConfig !== 'string' || cwdConfig.length === 0) {
2646+
return { folder, cwd: folder.uri, isAbsolute: false, isOverridden: false };
2647+
}
2648+
2649+
const resolvedCwdConfig = await configurationResolverService.resolveAsync(folder, cwdConfig);
2650+
return isAbsolute(resolvedCwdConfig) || resolvedCwdConfig.startsWith(AbstractVariableResolverService.VARIABLE_LHS)
2651+
? { folder, isAbsolute: true, isOverridden: true, cwd: URI.from({ scheme: folder.uri.scheme, path: resolvedCwdConfig }) }
2652+
: { folder, isAbsolute: false, isOverridden: true, cwd: URI.joinPath(folder.uri, resolvedCwdConfig) };
2653+
}
2654+
2655+
/**
2656+
* Drops repeated CWDs, if any, by keeping the one which best matches the workspace folder. It also preserves the original order.
2657+
*/
2658+
export function shrinkWorkspaceFolderCwdPairs(pairs: WorkspaceFolderCwdPair[]): WorkspaceFolderCwdPair[] {
2659+
const map = new Map<string, WorkspaceFolderCwdPair>();
2660+
for (const pair of pairs) {
2661+
const key = pair.cwd.toString();
2662+
const value = map.get(key);
2663+
if (!value || key === pair.folder.uri.toString()) {
2664+
map.set(key, pair);
2665+
}
2666+
}
2667+
const selectedPairs = new Set(map.values());
2668+
const selectedPairsInOrder = pairs.filter(x => selectedPairs.has(x));
2669+
return selectedPairsInOrder;
2670+
}

‎src/vs/workbench/contrib/terminal/test/browser/terminalActions.test.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { deepStrictEqual } from 'assert';
7+
import { URI } from 'vs/base/common/uri';
8+
import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';
9+
import { WorkspaceFolderCwdPair, shrinkWorkspaceFolderCwdPairs } from 'vs/workbench/contrib/terminal/browser/terminalActions';
10+
11+
function makeFakeFolder(name: string, uri: URI): IWorkspaceFolder {
12+
return {
13+
name,
14+
uri,
15+
index: 0,
16+
toResource: () => uri,
17+
};
18+
}
19+
20+
function makePair(folder: IWorkspaceFolder, cwd?: URI | IWorkspaceFolder, isAbsolute?: boolean): WorkspaceFolderCwdPair {
21+
return {
22+
folder,
23+
cwd: !cwd ? folder.uri : (cwd instanceof URI ? cwd : cwd.uri),
24+
isAbsolute: !!isAbsolute,
25+
isOverridden: !!cwd && cwd.toString() !== folder.uri.toString(),
26+
};
27+
}
28+
29+
suite('terminalActions', () => {
30+
const root: URI = URI.file('/some-root');
31+
const a = makeFakeFolder('a', URI.joinPath(root, 'a'));
32+
const b = makeFakeFolder('b', URI.joinPath(root, 'b'));
33+
const c = makeFakeFolder('c', URI.joinPath(root, 'c'));
34+
const d = makeFakeFolder('d', URI.joinPath(root, 'd'));
35+
36+
suite('shrinkWorkspaceFolderCwdPairs', () => {
37+
test('should return empty when given array is empty', () => {
38+
deepStrictEqual(shrinkWorkspaceFolderCwdPairs([]), []);
39+
});
40+
41+
test('should return the only single pair when given argument is a single element array', () => {
42+
const pairs = [makePair(a)];
43+
deepStrictEqual(shrinkWorkspaceFolderCwdPairs(pairs), pairs);
44+
});
45+
46+
test('should return all pairs when no repeated cwds', () => {
47+
const pairs = [makePair(a), makePair(b), makePair(c)];
48+
deepStrictEqual(shrinkWorkspaceFolderCwdPairs(pairs), pairs);
49+
});
50+
51+
suite('should select the pair that has the same URI when repeated cwds exist', () => {
52+
test('all repeated', () => {
53+
const pairA = makePair(a);
54+
const pairB = makePair(b, a); // CWD points to A
55+
const pairC = makePair(c, a); // CWD points to A
56+
deepStrictEqual(shrinkWorkspaceFolderCwdPairs([pairA, pairB, pairC]), [pairA]);
57+
});
58+
59+
test('two repeated + one different', () => {
60+
const pairA = makePair(a);
61+
const pairB = makePair(b, a); // CWD points to A
62+
const pairC = makePair(c);
63+
deepStrictEqual(shrinkWorkspaceFolderCwdPairs([pairA, pairB, pairC]), [pairA, pairC]);
64+
});
65+
66+
test('two repeated + two repeated', () => {
67+
const pairA = makePair(a);
68+
const pairB = makePair(b, a); // CWD points to A
69+
const pairC = makePair(c);
70+
const pairD = makePair(d, c);
71+
deepStrictEqual(shrinkWorkspaceFolderCwdPairs([pairA, pairB, pairC, pairD]), [pairA, pairC]);
72+
});
73+
74+
test('two repeated + two repeated (reverse order)', () => {
75+
const pairB = makePair(b, a); // CWD points to A
76+
const pairA = makePair(a);
77+
const pairD = makePair(d, c);
78+
const pairC = makePair(c);
79+
deepStrictEqual(shrinkWorkspaceFolderCwdPairs([pairA, pairB, pairC, pairD]), [pairA, pairC]);
80+
});
81+
});
82+
});
83+
});

0 commit comments

Comments
 (0)