Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add tests for terminal suggest widget, fix some bugs #234445

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
5 changes: 5 additions & 0 deletions .vscode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ const extensions = [
workspaceFolder: `extensions/vscode-colorize-tests/test`,
mocha: { timeout: 60_000 }
},
{
label: 'terminal-suggest',
workspaceFolder: path.join(os.tmpdir(), `terminal-suggest-${Math.floor(Math.random() * 100000)}`),
mocha: { timeout: 60_000 }
},
{
label: 'vscode-colorize-perf-tests',
workspaceFolder: `extensions/vscode-colorize-perf-tests/test`,
Expand Down
1 change: 1 addition & 0 deletions build/gulpfile.extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const compilations = [
'extensions/markdown-math/tsconfig.json',
'extensions/media-preview/tsconfig.json',
'extensions/merge-conflict/tsconfig.json',
'extensions/terminal-suggest/tsconfig.json',
'extensions/microsoft-authentication/tsconfig.json',
'extensions/notebook-renderers/tsconfig.json',
'extensions/npm/tsconfig.json',
Expand Down
4 changes: 2 additions & 2 deletions extensions/terminal-suggest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
"terminalCompletionProvider"
],
"scripts": {
"compile": "npx gulp compile-extension:npm",
"watch": "npx gulp watch-extension:npm"
"compile": "npx gulp compile-extension:terminal-suggest",
"watch": "npx gulp watch-extension:terminal-suggest"
},

"main": "./out/terminalSuggestMain",
Expand Down
69 changes: 69 additions & 0 deletions extensions/terminal-suggest/src/terminalSuggestMain.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { deepStrictEqual, strictEqual } from 'assert';
import 'mocha';
import { availableSpecs, getCompletionItemsFromSpecs } from './terminalSuggestMain';

suite('Terminal Suggest', () => {

const availableCommands = ['cd', 'code', 'code-insiders'];
const codeOptions = ['-', '--add', '--category', '--diff', '--disable-extension', '--disable-extensions', '--disable-gpu', '--enable-proposed-api', '--extensions-dir', '--goto', '--help', '--inspect-brk-extensions', '--inspect-extensions', '--install-extension', '--list-extensions', '--locale', '--log', '--max-memory', '--merge', '--new-window', '--pre-release', '--prof-startup', '--profile', '--reuse-window', '--show-versions', '--status', '--sync', '--telemetry', '--uninstall-extension', '--user-data-dir', '--verbose', '--version', '--wait', '-a', '-d', '-g', '-h', '-m', '-n', '-r', '-s', '-v', '-w'];

suite('Cursor at the end of the command line', () => {
createTestCase('|', availableCommands, 'neither', availableSpecs);
createTestCase('c|', availableCommands, 'neither', availableSpecs);
createTestCase('ls && c|', availableCommands, 'neither', availableSpecs);
createTestCase('cd |', ['~', '-'], 'folders', availableSpecs);
createTestCase('code|', ['code-insiders'], 'neither', availableSpecs);
createTestCase('code-insiders|', [], 'neither', availableSpecs);
createTestCase('code |', codeOptions, 'neither', availableSpecs);
createTestCase('code --locale |', ['bg', 'de', 'en', 'es', 'fr', 'hu', 'it', 'ja', 'ko', 'pt-br', 'ru', 'tr', 'zh-CN', 'zh-TW'], 'neither', availableSpecs);
createTestCase('code --diff |', [], 'files', availableSpecs);
createTestCase('code -di|', codeOptions.filter(o => o.startsWith('di')), 'neither', availableSpecs);
createTestCase('code --diff ./file1 |', [], 'files', availableSpecs);
createTestCase('code --merge |', [], 'files', availableSpecs);
createTestCase('code --merge ./file1 ./file2 |', [], 'files', availableSpecs);
createTestCase('code --merge ./file1 ./file2 ./base |', [], 'files', availableSpecs);
createTestCase('code --goto |', [], 'files', availableSpecs);
createTestCase('code --user-data-dir |', [], 'folders', availableSpecs);
createTestCase('code --profile |', [], 'neither', availableSpecs);
createTestCase('code --install-extension |', [], 'neither', availableSpecs);
createTestCase('code --uninstall-extension |', [], 'neither', availableSpecs);
createTestCase('code --log |', ['critical', 'error', 'warn', 'info', 'debug', 'trace', 'off'], 'neither', availableSpecs);
createTestCase('code --sync |', ['on', 'off'], 'neither', availableSpecs);
createTestCase('code --extensions-dir |', [], 'folders', availableSpecs);
createTestCase('code --list-extensions |', codeOptions, 'neither', availableSpecs);
createTestCase('code --show-versions |', codeOptions, 'neither', availableSpecs);
createTestCase('code --category |', ['azure', 'data science', 'debuggers', 'extension packs', 'education', 'formatters', 'keymaps', 'language packs', 'linters', 'machine learning', 'notebooks', 'programming languages', 'scm providers', 'snippets', 'testing', 'themes', 'visualization', 'other'], 'neither', availableSpecs);
createTestCase('code --category a|', ['azure'], 'neither', availableSpecs);
createTestCase('code-insiders --list-extensions |', codeOptions, 'neither', availableSpecs);
createTestCase('code-insiders --show-versions |', codeOptions, 'neither', availableSpecs);
createTestCase('code-insiders --category |', ['azure', 'data science', 'debuggers', 'extension packs', 'education', 'formatters', 'keymaps', 'language packs', 'linters', 'machine learning', 'notebooks', 'programming languages', 'scm providers', 'snippets', 'testing', 'themes', 'visualization', 'other'], 'neither', availableSpecs);
createTestCase('code-insiders --category a|', ['azure'], 'neither', availableSpecs);
createTestCase('code-insiders --category azure |', [], 'neither', availableSpecs);
});
suite('Cursor not at the end of the line', () => {
createTestCase('code | --locale', codeOptions, 'neither', availableSpecs);
createTestCase('code --locale | && ls', ['bg', 'de', 'en', 'es', 'fr', 'hu', 'it', 'ja', 'ko', 'pt-br', 'ru', 'tr', 'zh-CN', 'zh-TW'], 'neither', availableSpecs);
createTestCase('code-insiders | --locale', codeOptions, 'neither', availableSpecs);
createTestCase('code-insiders --locale | && ls', ['bg', 'de', 'en', 'es', 'fr', 'hu', 'it', 'ja', 'ko', 'pt-br', 'ru', 'tr', 'zh-CN', 'zh-TW'], 'neither', availableSpecs);
});

function createTestCase(commandLineWithCursor: string, expectedCompletionLabels: string[], resourcesRequested: 'files' | 'folders' | 'both' | 'neither', availableSpecs: Fig.Spec[]): void {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best to stick expected as the last arg

const commandLine = commandLineWithCursor.split('|')[0];
const cursorPosition = commandLineWithCursor.indexOf('|');
const prefix = commandLine.slice(0, cursorPosition).split(' ').pop() || '';
const filesRequested = resourcesRequested === 'files' || resourcesRequested === 'both';
const foldersRequested = resourcesRequested === 'folders' || resourcesRequested === 'both';
test(commandLineWithCursor, function () {
const result = getCompletionItemsFromSpecs(availableSpecs, { commandLine, cursorPosition }, availableCommands, prefix);
deepStrictEqual(result.items.map(i => i.label).sort(), expectedCompletionLabels.sort());
strictEqual(result.filesRequested, filesRequested);
strictEqual(result.foldersRequested, foldersRequested);
});
}
});

41 changes: 27 additions & 14 deletions extensions/terminal-suggest/src/terminalSuggestMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import cdSpec from './completions/cd';
let cachedAvailableCommands: Set<string> | undefined;
let cachedBuiltinCommands: Map<string, string[]> | undefined;

export const availableSpecs = [codeCompletionSpec, codeInsidersCompletionSpec, cdSpec];

function getBuiltinCommands(shell: string): string[] | undefined {
try {
const shellType = path.basename(shell);
Expand Down Expand Up @@ -89,16 +91,15 @@ export async function activate(context: vscode.ExtensionContext) {
const items: vscode.TerminalCompletionItem[] = [];
const prefix = getPrefix(terminalContext.commandLine, terminalContext.cursorPosition);

const specs = [codeCompletionSpec, codeInsidersCompletionSpec, cdSpec];
const specCompletions = await getCompletionItemsFromSpecs(specs, terminalContext, new Set(commands), prefix, token);
const specCompletions = await getCompletionItemsFromSpecs(availableSpecs, terminalContext, commands, prefix, token);

items.push(...specCompletions.items);
let filesRequested = specCompletions.filesRequested;
let foldersRequested = specCompletions.foldersRequested;

if (!specCompletions.specificSuggestionsProvided) {
for (const command of commands) {
if (command.startsWith(prefix)) {
if (command.startsWith(prefix) && !items.find(item => item.label === command)) {
items.push(createCompletionItem(terminalContext.cursorPosition, prefix, command));
}
}
Expand Down Expand Up @@ -214,7 +215,7 @@ export function asArray<T>(x: T | T[]): T[] {
return Array.isArray(x) ? x : [x];
}

function getCompletionItemsFromSpecs(specs: Fig.Spec[], terminalContext: { commandLine: string; cursorPosition: number }, availableCommands: Set<string>, prefix: string, token: vscode.CancellationToken): { items: vscode.TerminalCompletionItem[]; filesRequested: boolean; foldersRequested: boolean; specificSuggestionsProvided: boolean } {
export function getCompletionItemsFromSpecs(specs: Fig.Spec[], terminalContext: { commandLine: string; cursorPosition: number }, availableCommands: string[], prefix: string, token?: vscode.CancellationToken): { items: vscode.TerminalCompletionItem[]; filesRequested: boolean; foldersRequested: boolean; specificSuggestionsProvided: boolean } {
const items: vscode.TerminalCompletionItem[] = [];
let filesRequested = false;
let foldersRequested = false;
Expand All @@ -224,7 +225,21 @@ function getCompletionItemsFromSpecs(specs: Fig.Spec[], terminalContext: { comma
continue;
}
for (const specLabel of specLabels) {
if (!availableCommands.has(specLabel) || token.isCancellationRequested || !terminalContext.commandLine.startsWith(specLabel)) {
if (!availableCommands.includes(specLabel) || (token && token?.isCancellationRequested)) {
continue;
}
//
if (
// If the prompt is empty
!terminalContext.commandLine
// or the prefix matches the command and the prefix is not equal to the command
|| !!prefix && specLabel.startsWith(prefix) && specLabel !== prefix
) {
// push it to the completion items
items.push(createCompletionItem(terminalContext.cursorPosition, prefix, specLabel));
}
if (!terminalContext.commandLine.startsWith(specLabel)) {
// the spec label is not the first word in the command line, so do not provide options or args
continue;
}
const precedingText = terminalContext.commandLine.slice(0, terminalContext.cursorPosition + 1);
Expand All @@ -235,7 +250,7 @@ function getCompletionItemsFromSpecs(specs: Fig.Spec[], terminalContext: { comma
continue;
}
for (const optionLabel of optionLabels) {
if (optionLabel.startsWith(prefix) || (prefix.length > specLabel.length && prefix.trim() === specLabel)) {
if (!items.find(i => i.label === optionLabel) && optionLabel.startsWith(prefix) || (prefix.length > specLabel.length && prefix.trim() === specLabel)) {
items.push(createCompletionItem(terminalContext.cursorPosition, prefix, optionLabel, option.description, false, vscode.TerminalCompletionItemKind.Flag));
}
const expectedText = `${specLabel} ${optionLabel} `;
Expand All @@ -248,13 +263,8 @@ function getCompletionItemsFromSpecs(specs: Fig.Spec[], terminalContext: { comma
if (!argsCompletions) {
continue;
}
if (argsCompletions.specificSuggestionsProvided) {
// prevents the list from containing a bunch of other stuff
return argsCompletions;
}
items.push(...argsCompletions.items);
filesRequested = filesRequested || argsCompletions.filesRequested;
foldersRequested = foldersRequested || argsCompletions.foldersRequested;
// return early so that we don't show the other completions
return argsCompletions;
}
}
}
Expand Down Expand Up @@ -307,7 +317,10 @@ function getCompletionItemsFromArgs(args: Fig.SingleOrArray<Fig.Arg> | undefined
}

for (const suggestionLabel of suggestionLabels) {
if (suggestionLabel && suggestionLabel.startsWith(currentPrefix.trim())) {
if (items.find(i => i.label === suggestionLabel)) {
continue;
}
if (suggestionLabel && suggestionLabel.startsWith(currentPrefix.trim()) && suggestionLabel !== currentPrefix.trim()) {
const hasSpaceBeforeCursor = terminalContext.commandLine[terminalContext.cursorPosition - 1] === ' ';
// prefix will be '' if there is a space before the cursor
const description = typeof suggestion !== 'string' ? suggestion.description : '';
Expand Down
1 change: 1 addition & 0 deletions extensions/terminal-suggest/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
},
"include": [
"src/**/*",
"src/completions/index.d.ts",
"../../src/vscode-dts/vscode.d.ts",
"../../src/vscode-dts/vscode.proposed.terminalCompletionProvider.d.ts"
]
Expand Down
6 changes: 6 additions & 0 deletions scripts/test-integration.sh
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ echo
npm run test-extension -- -l vscode-colorize-tests
kill_app

echo
echo "### Terminal Suggest tests"
echo
npm run test-extension -- -l terminal-suggest --enable-proposed-api=vscode.vscode-api-tests
kill_app

echo
echo "### TypeScript tests"
echo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
continue;
}

const label = prefix + stat.resource.fsPath.replace(cwd.fsPath, '');
const label = prefix + stat.resource.fsPath.replace(dir.fsPath, '');
resourceCompletions.push({
label,
kind,
Expand Down
Loading