Skip to content

Commit 585eba7

Browse files
authored
fix terminal suggest regression (#287610)
fix terminal suggest regression (#287187) fixes #287161
1 parent 7b6e372 commit 585eba7

File tree

7 files changed

+44
-8
lines changed

7 files changed

+44
-8
lines changed

extensions/terminal-suggest/src/terminalSuggestMain.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,13 @@ export async function resolveCwdFromCurrentCommandString(currentCommandString: s
413413
}
414414
const relativeFolder = lastSlashIndex === -1 ? '' : prefix.slice(0, lastSlashIndex);
415415

416+
// Don't pre-resolve paths with .. segments - let the completion service handle those
417+
// to avoid double-navigation (e.g., typing ../ would resolve cwd to parent here,
418+
// then completion service would navigate up again from the already-parent cwd)
419+
if (relativeFolder.includes('..')) {
420+
return undefined;
421+
}
422+
416423
// Use vscode.Uri.joinPath for path resolution
417424
const resolvedUri = vscode.Uri.joinPath(currentCwd, relativeFolder);
418425

extensions/terminal-suggest/src/test/completions/cd.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ export const cdTestSuiteSpec: ISuiteSpec = {
3737

3838
// Relative directories (changes cwd due to /)
3939
{ input: 'cd child/|', expectedCompletions, expectedResourceRequests: { type: 'folders', cwd: testPaths.cwdChild } },
40-
{ input: 'cd ../|', expectedCompletions, expectedResourceRequests: { type: 'folders', cwd: testPaths.cwdParent } },
41-
{ input: 'cd ../sibling|', expectedCompletions, expectedResourceRequests: { type: 'folders', cwd: testPaths.cwdParent } },
40+
// Paths with .. are handled by the completion service to avoid double-navigation (no cwd resolution)
41+
{ input: 'cd ../|', expectedCompletions, expectedResourceRequests: { type: 'folders' } },
42+
{ input: 'cd ../sibling|', expectedCompletions, expectedResourceRequests: { type: 'folders' } },
4243
]
4344
};

extensions/terminal-suggest/src/test/completions/upstream/ls.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,9 @@ export const lsTestSuiteSpec: ISuiteSpec = {
8484

8585
// Relative directories (changes cwd due to /)
8686
{ input: 'ls child/|', expectedCompletions: allOptions, expectedResourceRequests: { type: 'both', cwd: testPaths.cwdChild } },
87-
{ input: 'ls ../|', expectedCompletions: allOptions, expectedResourceRequests: { type: 'both', cwd: testPaths.cwdParent } },
88-
{ input: 'ls ../sibling|', expectedCompletions: allOptions, expectedResourceRequests: { type: 'both', cwd: testPaths.cwdParent } },
87+
// Paths with .. are handled by the completion service to avoid double-navigation (no cwd resolution)
88+
{ input: 'ls ../|', expectedCompletions: allOptions, expectedResourceRequests: { type: 'both' } },
89+
{ input: 'ls ../sibling|', expectedCompletions: allOptions, expectedResourceRequests: { type: 'both' } },
8990
]
9091
};
9192

extensions/terminal-suggest/src/test/helpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export interface ITestSpec {
2121
input: string;
2222
expectedResourceRequests?: {
2323
type: 'files' | 'folders' | 'both';
24-
cwd: Uri;
24+
cwd?: Uri;
2525
};
2626
expectedCompletions?: (string | ICompletionResource)[];
2727
}

extensions/terminal-suggest/src/test/terminalSuggestMain.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ suite('Terminal Suggest', () => {
8585
let expectedString = testSpec.expectedCompletions ? `[${testSpec.expectedCompletions.map(e => `'${e}'`).join(', ')}]` : '[]';
8686
if (testSpec.expectedResourceRequests) {
8787
expectedString += ` + ${testSpec.expectedResourceRequests.type}`;
88-
if (testSpec.expectedResourceRequests.cwd.fsPath !== testPaths.cwd.fsPath) {
88+
if (testSpec.expectedResourceRequests.cwd && testSpec.expectedResourceRequests.cwd.fsPath !== testPaths.cwd.fsPath) {
8989
expectedString += ` @ ${basename(testSpec.expectedResourceRequests.cwd.fsPath)}/`;
9090
}
9191
}

src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalCompletionService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
591591
if (lastWordFolder.length > 0) {
592592
label = addPathRelativePrefix(lastWordFolder + label, resourceOptions, lastWordFolderHasDotPrefix);
593593
}
594-
const parentDir = URI.joinPath(cwd, '..' + resourceOptions.pathSeparator);
594+
const parentDir = URI.joinPath(lastWordFolderResource, '..' + resourceOptions.pathSeparator);
595595
resourceCompletions.push({
596596
label,
597597
provider,

src/vs/workbench/contrib/terminalContrib/suggest/test/browser/terminalCompletionService.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,32 @@ suite('TerminalCompletionService', () => {
198198
], { replacementRange: [1, 3] });
199199
});
200200

201+
test('../| should return parent folder completions', async () => {
202+
// Scenario: cwd is /parent/folder1, sibling is /parent/folder2
203+
// When typing ../, should see contents of /parent/ (folder1 and folder2)
204+
validResources = [
205+
URI.parse('file:///parent/folder1'),
206+
URI.parse('file:///parent'),
207+
];
208+
childResources = [
209+
{ resource: URI.parse('file:///parent/folder1/'), isDirectory: true },
210+
{ resource: URI.parse('file:///parent/folder2/'), isDirectory: true },
211+
];
212+
const resourceOptions: TerminalCompletionResourceOptions = {
213+
cwd: URI.parse('file:///parent/folder1'),
214+
showDirectories: true,
215+
pathSeparator
216+
};
217+
const result = await terminalCompletionService.resolveResources(resourceOptions, '../', 3, provider, capabilities);
218+
219+
assertCompletions(result, [
220+
{ label: '../', detail: '/parent/' },
221+
{ label: '../folder1/', detail: '/parent/folder1/' },
222+
{ label: '../folder2/', detail: '/parent/folder2/' },
223+
{ label: '../../', detail: '/' },
224+
], { replacementRange: [0, 3] });
225+
});
226+
201227
test('cd ./| should return folder completions', async () => {
202228
const resourceOptions: TerminalCompletionResourceOptions = {
203229
cwd: URI.parse('file:///test'),
@@ -564,7 +590,8 @@ suite('TerminalCompletionService', () => {
564590
assertCompletions(result, [
565591
{ label: './test/', detail: '/test/test/' },
566592
{ label: './test/inner/', detail: '/test/test/inner/' },
567-
{ label: './test/../', detail: '/' }
593+
// ../` from the viewed folder (/test/test/) goes to /test/, not /
594+
{ label: './test/../', detail: '/test/' }
568595
], { replacementRange: [0, 5] });
569596
});
570597
test('test/| should normalize current and parent folders', async () => {

0 commit comments

Comments
 (0)