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 . and ../ completions #237836

Merged
merged 41 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
1879f85
fix #234351
meganrogge Jan 13, 2025
a1c66f4
tweak
meganrogge Jan 13, 2025
90413df
consolidate
meganrogge Jan 13, 2025
789a154
move . and .. to top
meganrogge Jan 13, 2025
8b6dbad
Push WIP for fixing edge cases
Tyriar Jan 13, 2025
68151b2
update test expectations, get tests to pass
meganrogge Jan 14, 2025
b7e0f32
use provider, not builtin
meganrogge Jan 14, 2025
48613cd
provide paths as details
meganrogge Jan 14, 2025
4624f36
add test case for and fix bug
meganrogge Jan 14, 2025
c26f1b5
fix tests, some bugs
meganrogge Jan 14, 2025
8e798af
Refactor, clean up the code
meganrogge Jan 14, 2025
99de7b6
further simplify
meganrogge Jan 14, 2025
8bb2b4a
add a comment
meganrogge Jan 14, 2025
2dc39b9
better name
meganrogge Jan 14, 2025
7a93217
sort by label alphabetically
meganrogge Jan 14, 2025
2f4dee8
Add comment
meganrogge Jan 14, 2025
6f77cab
refactor tests
meganrogge Jan 14, 2025
f29b6b8
move score part into conditional to not override it
meganrogge Jan 14, 2025
a84759c
add test for alphabetical order
meganrogge Jan 14, 2025
76aa350
Add more tests
meganrogge Jan 14, 2025
db1757f
add more tests
meganrogge Jan 14, 2025
5c0724b
rm commas
meganrogge Jan 14, 2025
9fb7a4a
rename
meganrogge Jan 14, 2025
3389bdb
also assert last item
meganrogge Jan 15, 2025
1bd065c
add isWindows and opposite test case
meganrogge Jan 15, 2025
2086f15
normalize prefix
meganrogge Jan 15, 2025
74ff875
pass in should normalize prefix
meganrogge Jan 15, 2025
d4e0228
check if absolute path before adding children
meganrogge Jan 15, 2025
9010261
fix a test
meganrogge Jan 15, 2025
2932776
rm a test
meganrogge Jan 15, 2025
b7a74a7
alter test
meganrogge Jan 15, 2025
33ca570
add test for absolute paths on windows
meganrogge Jan 15, 2025
9b95e66
Fix filling of children dirs on Windows, fix unit tests on Windows
Tyriar Jan 15, 2025
c9b2a97
Remove most recent completion feature
Tyriar Jan 15, 2025
013e59c
Show absolute path in completion details
Tyriar Jan 15, 2025
0f33b3d
fix test expectations
meganrogge Jan 15, 2025
c1ab0e7
fix test expectations
meganrogge Jan 15, 2025
44d5bbd
don't provide parent dir for absolute path
meganrogge Jan 15, 2025
4450eab
set as var
meganrogge Jan 15, 2025
a031ef3
use path sep in else
meganrogge Jan 15, 2025
6237400
add comment, todo
meganrogge Jan 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { CancellationToken } from '../../../../../base/common/cancellation.js';
import { Disposable, IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js';
import { Schemas } from '../../../../../base/common/network.js';
import { basename } from '../../../../../base/common/path.js';
import { isWindows } from '../../../../../base/common/platform.js';
import { URI } from '../../../../../base/common/uri.js';
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
import { IFileService } from '../../../../../platform/files/common/files.js';
Expand Down Expand Up @@ -62,6 +64,7 @@ export interface TerminalResourceRequestConfig {
foldersRequested?: boolean;
cwd?: URI;
pathSeparator: string;
shouldNormalizePrefix?: boolean;
}


Expand Down Expand Up @@ -183,6 +186,10 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
}

async resolveResources(resourceRequestConfig: TerminalResourceRequestConfig, promptValue: string, cursorPosition: number, provider: string): Promise<ITerminalCompletion[] | undefined> {
if (resourceRequestConfig.shouldNormalizePrefix) {
// for tests, make sure the right path separator is used
promptValue = promptValue.replaceAll(/[\\/]/g, resourceRequestConfig.pathSeparator);
}
meganrogge marked this conversation as resolved.
Show resolved Hide resolved
const cwd = URI.revive(resourceRequestConfig.cwd);
const foldersRequested = resourceRequestConfig.foldersRequested ?? false;
const filesRequested = resourceRequestConfig.filesRequested ?? false;
Expand All @@ -197,46 +204,128 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo

const resourceCompletions: ITerminalCompletion[] = [];
const cursorPrefix = promptValue.substring(0, cursorPosition);
const endsWithSpace = cursorPrefix.endsWith(' ');
const lastWord = endsWithSpace ? '' : cursorPrefix.split(' ').at(-1) ?? '';

for (const stat of fileStat.children) {
let kind: TerminalCompletionItemKind | undefined;
if (foldersRequested && stat.isDirectory) {
kind = TerminalCompletionItemKind.Folder;
}
if (filesRequested && !stat.isDirectory && (stat.isFile || stat.resource.scheme === Schemas.file)) {
kind = TerminalCompletionItemKind.File;
}
if (kind === undefined) {
continue;
}
const isDirectory = kind === TerminalCompletionItemKind.Folder;
const fileName = basename(stat.resource.fsPath);

let label;
if (!lastWord.startsWith('.' + resourceRequestConfig.pathSeparator) && !lastWord.startsWith('..' + resourceRequestConfig.pathSeparator)) {
// add a dot to the beginning of the label if it doesn't already have one
label = `.${resourceRequestConfig.pathSeparator}${fileName}`;
} else {
if (lastWord.endsWith(resourceRequestConfig.pathSeparator)) {
label = `${lastWord}${fileName}`;
} else {
label = `${lastWord}${resourceRequestConfig.pathSeparator}${fileName}`;
const useForwardSlash = !resourceRequestConfig.shouldNormalizePrefix && isWindows;

// The last word (or argument). When the cursor is following a space it will be the empty
// string
const lastWord = cursorPrefix.endsWith(' ') ? '' : cursorPrefix.split(' ').at(-1) ?? '';

// Get the nearest folder path from the prefix. This ignores everything after the `/` as
// they are what triggers changes in the directory.
let lastSlashIndex: number;
if (useForwardSlash) {
lastSlashIndex = Math.max(lastWord.lastIndexOf('\\'), lastWord.lastIndexOf('/'));
} else {
lastSlashIndex = lastWord.lastIndexOf(resourceRequestConfig.pathSeparator);
}

// The _complete_ folder of the last word. For example if the last word is `./src/file`,
// this will be `./src/`. This also always ends in the path separator if it is not the empty
// string and path separators are normalized on Windows.
let lastWordFolder = lastSlashIndex === -1 ? '' : lastWord.slice(0, lastSlashIndex + 1);
if (isWindows) {
lastWordFolder = lastWordFolder.replaceAll('/', '\\');
}

const lastWordFolderHasDotPrefix = lastWordFolder.match(/^\.\.?[\\\/]/);

// Add current directory. This should be shown at the top because it will be an exact match
// and therefore highlight the detail, plus it improves the experience when runOnEnter is
// used.
//
// For example:
// - `|` -> `.`, this does not have the trailing `/` intentionally as it's common to
// complete the current working directory and we do not want to complete `./` when
// `runOnEnter` is used.
// - `./src/|` -> `./src/`
if (foldersRequested) {
resourceCompletions.push({
label: lastWordFolder.length === 0 ? '.' : lastWordFolder,
provider,
kind: TerminalCompletionItemKind.Folder,
isDirectory: true,
isFile: false,
detail: getFriendlyFolderPath(cwd, resourceRequestConfig.pathSeparator),
replacementIndex: cursorPosition - lastWord.length,
replacementLength: lastWord.length
});
}

// Handle absolute paths differently to avoid adding `./` prefixes
// TODO: Deal with git bash case
const isAbsolutePath = useForwardSlash
? /^[a-zA-Z]:\\/.test(lastWord)
: lastWord.startsWith(resourceRequestConfig.pathSeparator) && lastWord.endsWith(resourceRequestConfig.pathSeparator);

// Add all direct children files or folders
//
// For example:
// - `cd ./src/` -> `cd ./src/folder1`, ...
if (!isAbsolutePath) {
for (const stat of fileStat.children) {
let kind: TerminalCompletionItemKind | undefined;
if (foldersRequested && stat.isDirectory) {
kind = TerminalCompletionItemKind.Folder;
}
if (lastWord.length && lastWord.at(-1) !== resourceRequestConfig.pathSeparator && lastWord.at(-1) !== '.') {
label = `.${resourceRequestConfig.pathSeparator}${fileName}`;
if (filesRequested && !stat.isDirectory && (stat.isFile || stat.resource.scheme === Schemas.file)) {
kind = TerminalCompletionItemKind.File;
}
if (kind === undefined) {
continue;
}
const isDirectory = kind === TerminalCompletionItemKind.Folder;
const resourceName = basename(stat.resource.fsPath);

let label = `${lastWordFolder}${resourceName}`;

// Normalize suggestion to add a ./ prefix to the start of the path if there isn't
// one already. We may want to change this behavior in the future to go with
// whatever format the user has
if (!lastWordFolderHasDotPrefix) {
label = `.${resourceRequestConfig.pathSeparator}${label}`;
}

// Ensure directories end with a path separator
if (isDirectory && !label.endsWith(resourceRequestConfig.pathSeparator)) {
label = `${label}${resourceRequestConfig.pathSeparator}`;
}

// Normalize path separator to `\` on Windows. It should act the exact same as `/` but
// suggestions should all use `\`
if (useForwardSlash) {
label = label.replaceAll('/', '\\');
}

resourceCompletions.push({
label,
provider,
kind,
isDirectory,
isFile: kind === TerminalCompletionItemKind.File,
replacementIndex: cursorPosition - lastWord.length,
replacementLength: lastWord.length
});
}
if (isDirectory && !label.endsWith(resourceRequestConfig.pathSeparator)) {
label = label + resourceRequestConfig.pathSeparator;
}
}

// Add parent directory to the bottom of the list because it's not as useful as other suggestions
//
// For example:
// - `|` -> `../`
// - `./src/|` -> `./src/../`
//
// On Windows, the path seprators are normalized to `\`:
// - `./src/|` -> `.\src\..\`
if (!isAbsolutePath && foldersRequested) {
const parentDir = URI.joinPath(cwd, '..' + resourceRequestConfig.pathSeparator);
resourceCompletions.push({
label,
label: lastWordFolder + '..' + resourceRequestConfig.pathSeparator,
provider,
kind,
isDirectory,
isFile: kind === TerminalCompletionItemKind.File,
kind: TerminalCompletionItemKind.Folder,
detail: getFriendlyFolderPath(parentDir, resourceRequestConfig.pathSeparator),
isDirectory: true,
isFile: false,
replacementIndex: cursorPosition - lastWord.length,
replacementLength: lastWord.length
});
Expand All @@ -245,3 +334,16 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
return resourceCompletions.length ? resourceCompletions : undefined;
}
}

function getFriendlyFolderPath(uri: URI, pathSeparator: string): string {
let path = uri.fsPath;
// Ensure folders end with the path separator to differentiate presentation from files
if (!path.endsWith(pathSeparator)) {
path += pathSeparator;
}
// Ensure drive is capitalized on Windows
if (pathSeparator === '\\' && path.match(/^[a-zA-Z]:\\/)) {
path = `${path[0].toUpperCase()}:${path.slice(2)}`;
}
return path;
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { SimpleCompletionItem } from '../../../../services/suggest/browser/simpl
import { LineContext, SimpleCompletionModel } from '../../../../services/suggest/browser/simpleCompletionModel.js';
import { ISimpleSelectedSuggestion, SimpleSuggestWidget } from '../../../../services/suggest/browser/simpleSuggestWidget.js';
import type { ISimpleSuggestWidgetFontInfo } from '../../../../services/suggest/browser/simpleSuggestWidgetRenderer.js';
import { ITerminalCompletion, ITerminalCompletionService, TerminalCompletionItemKind } from './terminalCompletionService.js';
import { ITerminalCompletionService, TerminalCompletionItemKind } from './terminalCompletionService.js';
import { TerminalShellType } from '../../../../../platform/terminal/common/terminal.js';
import { CancellationToken, CancellationTokenSource } from '../../../../../base/common/cancellation.js';
import { IExtensionService } from '../../../../services/extensions/common/extensions.js';
Expand Down Expand Up @@ -58,7 +58,6 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
private _enableWidget: boolean = true;
private _pathSeparator: string = sep;
private _isFilteringDirectories: boolean = false;
private _mostRecentCompletion?: ITerminalCompletion;

// TODO: Remove these in favor of prompt input state
private _leadingLineContent?: string;
Expand Down Expand Up @@ -193,12 +192,6 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
this._leadingLineContent = this._promptInputModel.prefix;
}

if (this._mostRecentCompletion?.isDirectory && completions.every(e => e.isDirectory)) {
completions.push(this._mostRecentCompletion);
}
this._mostRecentCompletion = undefined;


let normalizedLeadingLineContent = this._leadingLineContent;

// If there is a single directory in the completions:
Expand Down Expand Up @@ -504,8 +497,6 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
SuggestAddon.lastAcceptedCompletionTimestamp = 0;
}

this._mostRecentCompletion = completion;

const commonPrefixLen = commonPrefixLength(replacementText, completionText);
const commonPrefix = replacementText.substring(replacementText.length - 1 - commonPrefixLen, replacementText.length - 1);
const completionSuffix = completionText.substring(commonPrefixLen);
Expand Down
Loading
Loading