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

Initial commit for support vscode.workspace.findFiles feature #3128

Merged
merged 1 commit into from
Oct 16, 2018

Conversation

vparfonov
Copy link
Contributor

Signed-off-by: Vitalii Parfonov vparfonov@redhat.com

Find files across all workspace folders in the workspace.
More details https://code.visualstudio.com/docs/extensionAPI/vscode-api#_workspace 'findFiles'

@evidolob evidolob added plug-in system issues related to the plug-in system Team: Che-Plugins issues related to the che-plugins team labels Oct 10, 2018
let j = 0;
const promises: Promise<any>[] = new Array();
for (const root of this.roots) {
console.log('******** >> ' + root.uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

hello, I think it needs to be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

promises[j++] = this.fileSearchService.find(includePattern, {rootUri: root.uri}).then(value => {
const paths: string[] = new Array();
let i = 0;
value.forEach(function (item) {
Copy link
Contributor

Choose a reason for hiding this comment

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

arrow functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -39,6 +39,7 @@ import {
DefinitionLink,
DocumentLink
} from './model';
import {CancellationToken} from '@theia/plugin';
Copy link
Contributor

Choose a reason for hiding this comment

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

there is an import

 import * as theia from '@theia/plugin';

so you should use theia.CancellationToken (without adding this import)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -77,6 +77,8 @@ import { TerminalServiceExtImpl } from './terminal-ext';
import { LanguagesExtImpl, score } from './languages';
import { fromDocumentSelector } from './type-converters';
import { DialogsExtImpl } from './dialogs';
import { GlobPattern} from '@theia/plugin';
Copy link
Contributor

Choose a reason for hiding this comment

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

there is already an import

import * as theia from '@theia/plugin';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -19,24 +19,29 @@ import { WorkspaceExt, MAIN_RPC_CONTEXT, WorkspaceMain, WorkspaceFolderPickOptio
import { RPCProtocol } from '../../api/rpc-protocol';
import { WorkspaceService } from '@theia/workspace/lib/browser';
import Uri from 'vscode-uri';
import { WorkspaceFoldersChangeEvent, WorkspaceFolder } from '@theia/plugin';
import { UriComponents } from '../../common/uri-components';
import { WorkspaceFoldersChangeEvent, WorkspaceFolder, CancellationToken } from '@theia/plugin';
Copy link
Contributor

Choose a reason for hiding this comment

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

please use instead

import * as theia from '@theia/plugin';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}

let excludePatternOrDisregardExcludes: string | false;
if (exclude === null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have somtimes null ? AFAIK in theia we mostly have undefined or values

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we should not use null at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}

let excludePatternOrDisregardExcludes: string | false;
if (exclude === null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we should not use null at all

return Promise.all(promises).then(value => {
let i = 0;
value.forEach(function (path) {
// let path: string;
Copy link
Contributor

@evidolob evidolob Oct 11, 2018

Choose a reason for hiding this comment

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

Commented code should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -14,12 +14,13 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { WorkspaceFolder, WorkspaceFoldersChangeEvent, WorkspaceFolderPickOptions, GlobPattern, FileSystemWatcher } from '@theia/plugin';
import { WorkspaceFolder, WorkspaceFoldersChangeEvent, WorkspaceFolderPickOptions, GlobPattern, FileSystemWatcher, Uri } from '@theia/plugin';
Copy link
Contributor

Choose a reason for hiding this comment

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

You do not need Uri from '@theia/plugin', you can use URI that you imported below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@benoitf
Copy link
Contributor

benoitf commented Oct 16, 2018

@vparfonov FYI you need to use rebase only instead of "merge with master" in your branch, as it's the only way of merging PR on Theia repo

@@ -125,4 +130,29 @@ export class WorkspaceMainImpl implements WorkspaceMain {
});
}

$startFileSearch(includePattern: string, excludePatternOrDisregardExcludes?: string | false,
maxResults?: number, token?: theia.CancellationToken): Promise<UriComponents[]> {
const uris: UriComponents[] = new Array();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can use

const uris: UriComponents[] = [];

Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plug-in system issues related to the plug-in system Team: Che-Plugins issues related to the che-plugins team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants