file-search: fix not ignoring .git

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Change Log
 
+## v1.8.0
+
+<a name="breaking_changes_1.8.0">[Breaking Changes:](#breaking_changes_1.8.0)</a>
+
+- [file-search] Deprecate dependency on `@theia/process` and replaced its usage by node's `child_process` api.
+
 ## v1.7.0 - 29/10/2020
 
 <a name="release_milestone_1.7.0">[1.7.0 Release Milestone](https://github.com/eclipse-theia/theia/milestone/12?closed=1)</a>

packages/file-search/src/browser/quick-file-open.ts

@@ -29,6 +29,7 @@ import { Command } from '@theia/core/lib/common';
 import { NavigationLocationService } from '@theia/editor/lib/browser/navigation/navigation-location-service';
 import * as fuzzy from 'fuzzy';
 import { MessageService } from '@theia/core/lib/common/message-service';
+import { FileSystemPreferences } from '@theia/filesystem/lib/browser';
 
 export const quickFileOpen: Command = {
     id: 'file-search.openFile',
@@ -55,6 +56,8 @@ export class QuickFileOpenService implements QuickOpenModel, QuickOpenHandler {
     protected readonly navigationLocationService: NavigationLocationService;
     @inject(MessageService)
     protected readonly messageService: MessageService;
+    @inject(FileSystemPreferences)
+    protected readonly fsPreferences: FileSystemPreferences;
 
     /**
      * Whether to hide .gitignored (and other ignored) files.
@@ -198,7 +201,9 @@ export class QuickFileOpenService implements QuickOpenModel, QuickOpenHandler {
                 fuzzyMatch: true,
                 limit: 200,
                 useGitIgnore: this.hideIgnoredFiles,
-                excludePatterns: ['*.git*']
+                excludePatterns: this.hideIgnoredFiles
+                    ? Object.keys(this.fsPreferences['files.exclude'])
+                    : undefined,
             }, token).then(handler);
         } else {
             if (roots.length !== 0) {

packages/file-search/src/node/file-search-service-impl.spec.ts

@@ -22,16 +22,18 @@ import { FileUri } from '@theia/core/lib/node';
 import { Container, ContainerModule } from 'inversify';
 import { CancellationTokenSource } from '@theia/core';
 import { bindLogger } from '@theia/core/lib/node/logger-backend-module';
-import processBackendModule from '@theia/process/lib/node/process-backend-module';
 import URI from '@theia/core/lib/common/uri';
 import { FileSearchService } from '../common/file-search-service';
+import { RawProcessFactory } from '@theia/process/lib/node';
 
 /* eslint-disable no-unused-expressions */
 
 const testContainer = new Container();
 
 bindLogger(testContainer.bind.bind(testContainer));
-testContainer.load(processBackendModule);
+testContainer.bind(RawProcessFactory).toConstantValue(() => {
+    throw new Error('should not be used anymore');
+});
 testContainer.load(new ContainerModule(bind => {
     bind(FileSearchServiceImpl).toSelf().inSingletonScope();
 }));
@@ -52,16 +54,16 @@ describe('search-service', function (): void {
         // eslint-disable-next-line deprecation/deprecation
         const expectedFile = FileUri.create(__filename).displayName;
         const testFile = matches.find(e => e.endsWith(expectedFile));
-        expect(testFile).to.be.not.undefined;
+        expect(testFile).to.not.be.undefined;
     });
 
     it.skip('should respect nested .gitignore', async () => {
         const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources')).toString();
         const matches = await service.find('foo', { rootUris: [rootUri], fuzzyMatch: false });
 
         expect(matches.find(match => match.endsWith('subdir1/sub-bar/foo.txt'))).to.be.undefined;
-        expect(matches.find(match => match.endsWith('subdir1/sub2/foo.txt'))).to.be.not.undefined;
-        expect(matches.find(match => match.endsWith('subdir1/foo.txt'))).to.be.not.undefined;
+        expect(matches.find(match => match.endsWith('subdir1/sub2/foo.txt'))).to.not.be.undefined;
+        expect(matches.find(match => match.endsWith('subdir1/foo.txt'))).to.not.be.undefined;
     });
 
     it('should cancel searches', async () => {
@@ -78,7 +80,7 @@ describe('search-service', function (): void {
         const dirB = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1/sub2')).toString();
 
         const matches = await service.find('foo', { rootUris: [dirA, dirB] });
-        expect(matches).to.be.not.undefined;
+        expect(matches).to.not.be.undefined;
         expect(matches.length).to.eq(2);
     });
 
@@ -87,19 +89,19 @@ describe('search-service', function (): void {
             const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1/sub2')).toString();
 
             const matches = await service.find('', { rootUris: [rootUri], includePatterns: ['**/*oo.*'] });
-            expect(matches).to.be.not.undefined;
+            expect(matches).to.not.be.undefined;
             expect(matches.length).to.eq(1);
         });
 
         it('should NOT support file searches with globs without the prefixed or trailing star (*)', async () => {
             const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1/sub2')).toString();
 
             const trailingMatches = await service.find('', { rootUris: [rootUri], includePatterns: ['*oo'] });
-            expect(trailingMatches).to.be.not.undefined;
+            expect(trailingMatches).to.not.be.undefined;
             expect(trailingMatches.length).to.eq(0);
 
             const prefixedMatches = await service.find('', { rootUris: [rootUri], includePatterns: ['oo*'] });
-            expect(prefixedMatches).to.be.not.undefined;
+            expect(prefixedMatches).to.not.be.undefined;
             expect(prefixedMatches.length).to.eq(0);
         });
     });
@@ -109,7 +111,7 @@ describe('search-service', function (): void {
             const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1/sub2')).toString();
 
             const matches = await service.find('', { rootUris: [rootUri], includePatterns: ['**/*oo.*'], excludePatterns: ['foo'] });
-            expect(matches).to.be.not.undefined;
+            expect(matches).to.not.be.undefined;
             expect(matches.length).to.eq(1);
         });
 
@@ -153,7 +155,7 @@ describe('search-service', function (): void {
 
         async function assertIgnoreGlobs(options: FileSearchService.Options): Promise<void> {
             const matches = await service.find('', options);
-            expect(matches).to.be.not.undefined;
+            expect(matches).to.not.be.undefined;
             expect(matches.length).to.eq(0);
         }
     });
@@ -180,11 +182,17 @@ describe('search-service', function (): void {
                 const relativeMatch = relativeUri!.toString();
                 let position = 0;
                 for (const ch of 'shell') {
-                    position = relativeMatch.indexOf(ch, position);
-                    assert.notStrictEqual(position, -1, relativeMatch);
+                    position = relativeMatch.toLowerCase().indexOf(ch, position);
+                    assert.notStrictEqual(position, -1, `character "${ch}" not found in "${relativeMatch}"`);
                 }
             }
         });
+
+        it('should not look into .git', async () => {
+            const matches = await service.find('master', { rootUris: [rootUri.toString()], fuzzyMatch: false, useGitIgnore: true, limit: 200 });
+            // `**/.git/refs/remotes/*/master` files should not be picked up
+            assert.deepStrictEqual([], matches);
+        });
     });
 
 });

packages/file-search/src/node/file-search-service-impl.ts

@@ -14,6 +14,7 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
+import * as cp from 'child_process';
 import * as fuzzy from 'fuzzy';
 import * as readline from 'readline';
 import { rgPath } from 'vscode-ripgrep';
@@ -30,7 +31,9 @@ export class FileSearchServiceImpl implements FileSearchService {
 
     constructor(
         @inject(ILogger) protected readonly logger: ILogger,
-        @inject(RawProcessFactory) protected readonly rawProcessFactory: RawProcessFactory) { }
+        /** @deprecated since 1.7.0 */
+        @inject(RawProcessFactory) protected readonly rawProcessFactory: RawProcessFactory,
+    ) { }
 
     async find(searchPattern: string, options: FileSearchService.Options, clientToken?: CancellationToken): Promise<string[]> {
         const cancellationSource = new CancellationTokenSource();
@@ -111,37 +114,38 @@ export class FileSearchServiceImpl implements FileSearchService {
         return [...exactMatches, ...fuzzyMatches].slice(0, opts.limit);
     }
 
-    private doFind(rootUri: URI, options: FileSearchService.BaseOptions,
-        accept: (fileUri: string) => void, token: CancellationToken): Promise<void> {
+    private doFind(rootUri: URI, options: FileSearchService.BaseOptions, accept: (fileUri: string) => void, token: CancellationToken): Promise<void> {
         return new Promise((resolve, reject) => {
-            try {
-                const cwd = FileUri.fsPath(rootUri);
-                const args = this.getSearchArgs(options);
-                // TODO: why not just child_process.spawn, theia process are supposed to be used for user processes like tasks and terminals, not internal
-                const process = this.rawProcessFactory({ command: rgPath, args, options: { cwd } });
-                process.onError(reject);
-                process.outputStream.on('close', resolve);
-                token.onCancellationRequested(() => process.kill());
-
-                const lineReader = readline.createInterface({
-                    input: process.outputStream,
-                    output: process.inputStream
-                });
-                lineReader.on('line', line => {
-                    if (token.isCancellationRequested) {
-                        process.kill();
-                    } else {
-                        accept(line);
-                    }
-                });
-            } catch (e) {
-                reject(e);
-            }
+            const cwd = FileUri.fsPath(rootUri);
+            const args = this.getSearchArgs(options);
+            const ripgrep = cp.spawn(rgPath, args, { cwd, stdio: ['pipe', 'pipe', 'inherit'] });
+            ripgrep.on('error', reject);
+            ripgrep.on('exit', (code, signal) => {
+                if (typeof code === 'number' && code !== 0) {
+                    reject(new Error(`"${rgPath}" exited with code: ${code}`));
+                } else if (typeof signal === 'string') {
+                    reject(new Error(`"${rgPath}" was terminated by signal: ${signal}`));
+                }
+            });
+            token.onCancellationRequested(() => {
+                ripgrep.kill(); // most likely sends a signal.
+                resolve(); // avoid rejecting for no good reason.
+            });
+            const lineReader = readline.createInterface({
+                input: ripgrep.stdout,
+                crlfDelay: Infinity,
+            });
+            lineReader.on('line', line => {
+                if (!token.isCancellationRequested) {
+                    accept(line);
+                }
+            });
+            lineReader.on('close', () => resolve());
         });
     }
 
     private getSearchArgs(options: FileSearchService.BaseOptions): string[] {
-        const args = ['--files', '--hidden', '--case-sensitive'];
+        const args = ['--files', '--hidden'];
         if (options.includePatterns) {
             for (const includePattern of options.includePatterns) {
                 if (includePattern) {
@@ -156,8 +160,11 @@ export class FileSearchServiceImpl implements FileSearchService {
                 }
             }
         }
-        if (!options.useGitIgnore) {
-            args.push('-uu');
+        if (options.useGitIgnore) {
+            // ripgrep follows `.gitignore` by default, but it doesn't exclude `.git`:
+            args.push('--glob', '!.git');
+        } else {
+            args.push('--no-ignore');
         }
         return args;
     }