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

update FindFiles2 to make options clearer #211494

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -604,13 +604,13 @@ suite('vscode API - workspace', () => {
});

test('findFiles2 - null exclude', async () => {
await vscode.workspace.findFiles2('**/file.txt', { useDefaultExcludes: true, useDefaultSearchExcludes: false }).then((res) => {
await vscode.workspace.findFiles2('**/file.txt', { excludeSettings: vscode.ExcludeSettingOptions.filesExclude }).then((res) => {
// file.exclude folder is still searched, search.exclude folder is not
assert.strictEqual(res.length, 1);
assert.strictEqual(basename(vscode.workspace.asRelativePath(res[0])), 'file.txt');
});

await vscode.workspace.findFiles2('**/file.txt', { useDefaultExcludes: false, useDefaultSearchExcludes: false }).then((res) => {
await vscode.workspace.findFiles2('**/file.txt', { excludeSettings: vscode.ExcludeSettingOptions.none }).then((res) => {
// search.exclude and files.exclude folders are both searched
assert.strictEqual(res.length, 2);
assert.strictEqual(basename(vscode.workspace.asRelativePath(res[0])), 'file.txt');
Expand Down
4 changes: 3 additions & 1 deletion src/vs/workbench/api/common/extHost.api.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ import { ExtensionDescriptionRegistry } from 'vs/workbench/services/extensions/c
import { UIKind } from 'vs/workbench/services/extensions/common/extensionHostProtocol';
import { checkProposedApiEnabled, isProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions';
import { ProxyIdentifier } from 'vs/workbench/services/extensions/common/proxyIdentifier';
import { TextSearchCompleteMessageType } from 'vs/workbench/services/search/common/searchExtTypes';
import { ExcludeSettingOptions, SearchIgnoreOptions, TextSearchCompleteMessageType } from 'vs/workbench/services/search/common/searchExtTypes';
import type * as vscode from 'vscode';

export interface IExtensionRegistries {
Expand Down Expand Up @@ -1725,6 +1725,8 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
NewSymbolNameTriggerKind: extHostTypes.NewSymbolNameTriggerKind,
InlineEdit: extHostTypes.InlineEdit,
InlineEditTriggerKind: extHostTypes.InlineEditTriggerKind,
SearchIgnoreOptions: SearchIgnoreOptions,
ExcludeSettingOptions: ExcludeSettingOptions,
};
};
}
107 changes: 99 additions & 8 deletions src/vs/workbench/api/common/extHostWorkspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { IRawFileMatch2, ITextSearchResult, resultIsMatch } from 'vs/workbench/s
import * as vscode from 'vscode';
import { ExtHostWorkspaceShape, IRelativePatternDto, IWorkspaceData, MainContext, MainThreadMessageOptions, MainThreadMessageServiceShape, MainThreadWorkspaceShape } from './extHost.protocol';
import { revive } from 'vs/base/common/marshalling';
import { ExcludeSettingOptions, SearchIgnoreOptions } from 'vs/workbench/services/search/common/searchExtTypes';

export interface IExtHostWorkspaceProvider {
getWorkspaceFolder2(uri: vscode.Uri, resolveParent?: boolean): Promise<vscode.WorkspaceFolder | undefined>;
Expand Down Expand Up @@ -460,9 +461,8 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape, IExtHostWorkspac
return this._findFilesImpl(include, undefined, {
exclude: excludeString,
maxResults,
useDefaultExcludes: useFileExcludes,
useDefaultSearchExcludes: false,
useIgnoreFiles: false
excludeSettings: useFileExcludes ? ExcludeSettingOptions.filesExclude : ExcludeSettingOptions.none,
ignoreFilesToUse: SearchIgnoreOptions.none
}, token);
}

Expand All @@ -474,6 +474,94 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape, IExtHostWorkspac
return this._findFilesImpl(undefined, filePattern, options, token);
}

private _parseIgnoreFileSettings(options: vscode.FindFiles2Options): {
disregardLocalIgnoreFiles: boolean | undefined; // in this case, undefined means to use the value from settings.
disregardParentIgnoreFiles: boolean | undefined;
disregardGlobalIgnoreFiles: boolean | undefined;
} {

let disregardLocalIgnoreFiles: boolean | undefined = undefined;
let disregardParentIgnoreFiles: boolean | undefined = undefined;
let disregardGlobalIgnoreFiles: boolean | undefined = undefined;

if (options.ignoreFilesToUse) {
switch (options.ignoreFilesToUse) {
case SearchIgnoreOptions.none:
disregardLocalIgnoreFiles = true;
disregardParentIgnoreFiles = true;
disregardGlobalIgnoreFiles = true;
break;
case SearchIgnoreOptions.localOnly:
disregardLocalIgnoreFiles = false;
disregardParentIgnoreFiles = true;
disregardGlobalIgnoreFiles = true;
break;
case SearchIgnoreOptions.localAndFollowSettings:
disregardLocalIgnoreFiles = false;
break;
case SearchIgnoreOptions.localAndParent:
disregardLocalIgnoreFiles = false;
disregardParentIgnoreFiles = false;
disregardGlobalIgnoreFiles = true;
break;
case SearchIgnoreOptions.localAndGlobal:
disregardLocalIgnoreFiles = false;
disregardParentIgnoreFiles = true;
disregardGlobalIgnoreFiles = false;
break;
case SearchIgnoreOptions.all:
disregardLocalIgnoreFiles = false;
disregardParentIgnoreFiles = false;
disregardGlobalIgnoreFiles = false;
break;
default:
// SearchIgnoreOptions.followSettings also follows this
break;
}
} else {
// todo: remove once copilot chat moves off of deprecated options for ~1 month
disregardLocalIgnoreFiles = typeof options.useIgnoreFiles === 'boolean' ? !options.useIgnoreFiles : undefined;
disregardGlobalIgnoreFiles = typeof options.useGlobalIgnoreFiles === 'boolean' ? !options.useGlobalIgnoreFiles : undefined;
disregardParentIgnoreFiles = typeof options.useParentIgnoreFiles === 'boolean' ? !options.useParentIgnoreFiles : undefined;
}

return {
disregardLocalIgnoreFiles,
disregardParentIgnoreFiles,
disregardGlobalIgnoreFiles
};
}

private _parseShouldUseExcludeSetting(options: vscode.FindFiles2Options): {
disregardExcludeSettings: boolean;
disregardSearchExcludeSettings: boolean;
} {

let disregardExcludeSettings: boolean = false;
let disregardSearchExcludeSettings: boolean = false;

if (options.excludeSettings) {
switch (options.excludeSettings) {
case ExcludeSettingOptions.none:
disregardExcludeSettings = true;
disregardSearchExcludeSettings = true;
break;
case ExcludeSettingOptions.filesExclude:
disregardSearchExcludeSettings = true;
break;
default:
// includes the case for ExcludeSettingOptions.searchAndFilesExclude
break;
}
} else {
// todo: remove once copilot chat moves off of deprecated options for ~1 month
disregardExcludeSettings = typeof options.useDefaultExcludes === 'boolean' ? !options.useDefaultExcludes : false;
disregardSearchExcludeSettings = typeof options.useDefaultSearchExcludes === 'boolean' ? !options.useDefaultSearchExcludes : false;
}

return { disregardExcludeSettings, disregardSearchExcludeSettings };
}

private async _findFilesImpl(
// the old `findFiles` used `include` to query, but the new `findFiles2` uses `filePattern` to query.
// `filePattern` is the proper way to handle this, since it takes less precedence than the ignore files.
Expand All @@ -488,13 +576,16 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape, IExtHostWorkspac
const excludePattern = (typeof options.exclude === 'string') ? options.exclude :
options.exclude ? options.exclude.pattern : undefined;

const shouldUseExcludeSetting = this._parseShouldUseExcludeSetting(options);
const ignoreFileSettings = this._parseIgnoreFileSettings(options);

const fileQueries = <IFileQueryBuilderOptions>{
ignoreSymlinks: typeof options.followSymlinks === 'boolean' ? !options.followSymlinks : undefined,
disregardIgnoreFiles: typeof options.useIgnoreFiles === 'boolean' ? !options.useIgnoreFiles : undefined,
disregardGlobalIgnoreFiles: typeof options.useGlobalIgnoreFiles === 'boolean' ? !options.useGlobalIgnoreFiles : undefined,
disregardParentIgnoreFiles: typeof options.useParentIgnoreFiles === 'boolean' ? !options.useParentIgnoreFiles : undefined,
disregardExcludeSettings: typeof options.useDefaultExcludes === 'boolean' ? !options.useDefaultExcludes : false,
disregardSearchExcludeSettings: typeof options.useDefaultSearchExcludes === 'boolean' ? !options.useDefaultSearchExcludes : false,
disregardIgnoreFiles: ignoreFileSettings.disregardLocalIgnoreFiles,
disregardGlobalIgnoreFiles: ignoreFileSettings.disregardGlobalIgnoreFiles,
disregardParentIgnoreFiles: ignoreFileSettings.disregardParentIgnoreFiles,
disregardExcludeSettings: shouldUseExcludeSetting.disregardExcludeSettings,
disregardSearchExcludeSettings: shouldUseExcludeSetting.disregardSearchExcludeSettings,
maxResults: options.maxResults,
excludePattern: excludePattern,
shouldGlobSearch: typeof options.fuzzy === 'boolean' ? !options.fuzzy : true,
Expand Down
5 changes: 3 additions & 2 deletions src/vs/workbench/api/test/browser/extHostWorkspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { FileSystemProviderCapabilities } from 'vs/platform/files/common/files';
import { nullExtensionDescription as extensionDescriptor } from 'vs/workbench/services/extensions/common/extensions';
import { IURITransformerService } from 'vs/workbench/api/common/extHostUriTransformerService';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { ExcludeSettingOptions, SearchIgnoreOptions } from 'vs/workbench/services/search/common/searchExtTypes';

function createExtHostWorkspace(mainContext: IMainContext, data: IWorkspaceData, logService: ILogService): ExtHostWorkspace {
const result = new ExtHostWorkspace(
Expand Down Expand Up @@ -710,7 +711,7 @@ suite('ExtHostWorkspace', function () {
});

const ws = createExtHostWorkspace(rpcProtocol, { id: 'foo', folders: [aWorkspaceFolderData(URI.file(root), 0)], name: 'Test' }, new NullLogService());
return ws.findFiles2('foo', { maxResults: 10, useDefaultExcludes: true }, new ExtensionIdentifier('test')).then(() => {
return ws.findFiles2('foo', { maxResults: 10, excludeSettings: ExcludeSettingOptions.searchAndFilesExclude }, new ExtensionIdentifier('test')).then(() => {
assert(mainThreadCalled, 'mainThreadCalled');
});
});
Expand Down Expand Up @@ -825,7 +826,7 @@ suite('ExtHostWorkspace', function () {
});

const ws = createExtHostWorkspace(rpcProtocol, { id: 'foo', folders: [aWorkspaceFolderData(URI.file(root), 0)], name: 'Test' }, new NullLogService());
return ws.findFiles2('', { useIgnoreFiles: true, useParentIgnoreFiles: true, useGlobalIgnoreFiles: true }, new ExtensionIdentifier('test')).then(() => {
return ws.findFiles2('', { ignoreFilesToUse: SearchIgnoreOptions.all }, new ExtensionIdentifier('test')).then(() => {
assert(mainThreadCalled, 'mainThreadCalled');
});
});
Expand Down
22 changes: 22 additions & 0 deletions src/vs/workbench/services/search/common/searchExtTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,3 +498,25 @@ export interface FindTextInFilesOptions {
*/
afterContext?: number;
}

/*
* Options for following search.exclude and files.exclude settings.
*/
export enum ExcludeSettingOptions {
none = 1,
filesExclude = 2,
searchAndFilesExclude = 3,
}

/*
* Which locations of .gitignore and .ignore files to follow when searching.
*/
export enum SearchIgnoreOptions {
none = 1,
localOnly = 2,
localAndFollowSettings = 3,
Copy link
Contributor

Choose a reason for hiding this comment

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

Which settings? does followSettings means to follow what's set in excludeSettingOptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This means to follow the settings for global and parent ignore files-following. So using those settings to dictate whether to use them.

Copy link
Contributor

@rzhao271 rzhao271 May 1, 2024

Choose a reason for hiding this comment

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

Could the option be changed to followIgnoreSettings or a note be added to the docstring then? Otherwise, I think it's really easy to mix up these followSettings keys with the exclude enum options and/or exclude settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think that option 2 in this comment makes anything more clear? #205692 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although this PR has the enum approach, I've been brainstorming better ways to go about this.

localAndParent = 4,
localAndGlobal = 5,
all = 6,
followSettings = 7,
}
113 changes: 98 additions & 15 deletions src/vscode-dts/vscode.proposed.findFiles2.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,51 @@ declare module 'vscode' {
exclude?: GlobPattern;

/**
* Whether to use the values for files.exclude. Defaults to true.
* Which settings to follow when searching for files. Defaults to {@link ExcludeSettingOptions.searchAndFilesExclude}.
*/
useDefaultExcludes?: boolean;
excludeSettings?: ExcludeSettingOptions;

/**
* Whether to use the values for search.exclude. Defaults to true. Will not be followed if `useDefaultExcludes` is set to `false`.
* The maximum number of results to search for
*/
useDefaultSearchExcludes?: boolean;
maxResults?: number;

/**
* The maximum number of results to search for
* Which file locations we should look for ignore (.gitignore or .ignore) files to follow.
* Defaults to {@link SearchIgnoreOptions.followSettings}.
*/
maxResults?: number;
ignoreFilesToUse?: SearchIgnoreOptions;

/**
* Whether symlinks should be followed while searching.
* Defaults to the value for `search.followSymlinks` in settings.
* For more info, see the setting listed above.
*/
followSymlinks?: boolean;

/**
* If set to true, the `filePattern` arg will be fuzzy-searched instead of glob-searched.
* If `filePattern` is a {@link RelativePattern relative pattern}, then the fuzzy search will act on the `pattern` of the {@link RelativePattern RelativePattern}
*/
fuzzy?: boolean;

/**
* Whether to use the values for files.exclude. Defaults to true.
* @deprecated please use {@link FindFiles2Options.excludeSettings} instead.
* */
useDefaultExcludes?: boolean;

/**
* Whether to use the values for search.exclude. Defaults to true. Will not be followed if `useDefaultExcludes` is set to `false`.
* @deprecated please use {@link FindFiles2Options.excludeSettings} instead.
*/
useDefaultSearchExcludes?: boolean;

/**
* Whether external files that exclude files, like .gitignore, should be respected.
* Defaults to the value for `search.useIgnoreFiles` in settings.
* For more info, see the setting listed above.
* @deprecated please use {@link FindFiles2Options.ignoreFilesToUse} instead.
*/
useIgnoreFiles?: boolean;

Expand All @@ -41,6 +68,7 @@ declare module 'vscode' {
* Must set `useIgnoreFiles` to `true` to use this.
* Defaults to the value for `search.useGlobalIgnoreFiles` in settings.
* For more info, see the setting listed above.
* @deprecated please use {@link FindFiles2Options.ignoreFilesToUse} instead.
*/
useGlobalIgnoreFiles?: boolean;

Expand All @@ -49,21 +77,76 @@ declare module 'vscode' {
* Must set `useIgnoreFiles` to `true` to use this.
* Defaults to the value for `search.useParentIgnoreFiles` in settings.
* For more info, see the setting listed above.
* @deprecated please use {@link FindFiles2Options.ignoreFilesToUse} instead.
*/
useParentIgnoreFiles?: boolean;
}

/**
* Whether symlinks should be followed while searching.
* Defaults to the value for `search.followSymlinks` in settings.
* For more info, see the setting listed above.
/*
* Options for following search.exclude and files.exclude settings.
*/
export enum ExcludeSettingOptions {
/*
* Don't use any exclude settings.
*/
followSymlinks?: boolean;
none = 1,
/*
* Use:
* - files.exclude setting
*/
filesExclude = 2,
/*
* Use:
* - files.exclude setting
* - search.exclude setting
*/
searchAndFilesExclude = 3
}

/**
* If set to true, the `filePattern` arg will be fuzzy-searched instead of glob-searched.
* If `filePattern` is a {@link RelativePattern relative pattern}, then the fuzzy search will act on the `pattern` of the {@link RelativePattern RelativePattern}
/*
* Which locations of .gitignore and .ignore files to follow when searching.
*/
export enum SearchIgnoreOptions {
/*
* Don't use ignore files.
*/
fuzzy?: boolean;
none = 1,
/*
* Use:
* - ignore files at the workspace root.
*/
localOnly = 2,
/*
* Use:
* - ignore files at the workspace root.
*
* Follow `search.useParentIgnoreFiles`and `search.useGlobalIgnoreFiles` in settings
* to dictate whether to use parent and global ignore files.
*/
localAndFollowSettings = 3,
/*
* Use:
* - ignore files at the workspace root.
* - ignore files directly under parent folder(s) of the workspace root.
*/
localAndParent = 4,
/*
* Use:
* - ignore files at the workspace root.
* - global ignore files (e.g. $HOME/.config/git/ignore).
*/
localAndGlobal = 5,
/*
* Use:
* - ignore files at the workspace root.
* - ignore files directly under parent folder(s) of the workspace root.
* - global ignore files (e.g. $HOME/.config/git/ignore).
*/
all = 6,
/*
* Follow `search.useIgnoreFiles`, `search.useParentIgnoreFiles`, and `search.useGlobalIgnoreFiles` in settings.
*/
followSettings = 7,
}

/**
Expand Down