Browse Source

fix: grep tool Windows support, case sensitivity and unit tests (#73)

* fix: fix rg lookup path on Windows

* fix: fix Windows rg support

* fix: fix Windows support

* fix: ensure case insensitive search works correctly in rg wrapper

* test: add unit tests for grep tool

* fix: fix style
⁢4.435km/s 2 months ago
parent
commit
b03ae03b58
3 changed files with 224 additions and 20 deletions
  1. 10 4
      src/tools/grep/cli.ts
  2. 5 16
      src/tools/grep/constants.ts
  3. 209 0
      src/tools/grep/grep.test.ts

+ 10 - 4
src/tools/grep/cli.ts

@@ -8,8 +8,8 @@ import {
   DEFAULT_TIMEOUT_MS,
   DEFAULT_TIMEOUT_MS,
   GREP_SAFETY_FLAGS,
   GREP_SAFETY_FLAGS,
   type GrepBackend,
   type GrepBackend,
-  RG_SAFETY_FLAGS,
   resolveGrepCli,
   resolveGrepCli,
+  RG_SAFETY_FLAGS,
 } from './constants';
 } from './constants';
 import type { CountResult, GrepMatch, GrepOptions, GrepResult } from './types';
 import type { CountResult, GrepMatch, GrepOptions, GrepResult } from './types';
 
 
@@ -26,7 +26,11 @@ function buildRgArgs(options: GrepOptions): string[] {
     args.push(`-C${Math.min(options.context, 10)}`);
     args.push(`-C${Math.min(options.context, 10)}`);
   }
   }
 
 
-  if (options.caseSensitive) args.push('--case-sensitive');
+  if (options.caseSensitive) {
+    args.push('--case-sensitive');
+  } else {
+    args.push('-i');
+  }
   if (options.wholeWord) args.push('-w');
   if (options.wholeWord) args.push('-w');
   if (options.fixedStrings) args.push('-F');
   if (options.fixedStrings) args.push('-F');
   if (options.multiline) args.push('-U');
   if (options.multiline) args.push('-U');
@@ -90,7 +94,8 @@ function parseOutput(output: string): GrepMatch[] {
   if (!output.trim()) return [];
   if (!output.trim()) return [];
 
 
   const matches: GrepMatch[] = [];
   const matches: GrepMatch[] = [];
-  const lines = output.split('\n');
+  // Handle both Unix (\n) and Windows (\r\n) line endings
+  const lines = output.trim().split(/\r?\n/);
 
 
   for (const line of lines) {
   for (const line of lines) {
     if (!line.trim()) continue;
     if (!line.trim()) continue;
@@ -112,7 +117,8 @@ function parseCountOutput(output: string): CountResult[] {
   if (!output.trim()) return [];
   if (!output.trim()) return [];
 
 
   const results: CountResult[] = [];
   const results: CountResult[] = [];
-  const lines = output.split('\n');
+  // Handle both Unix (\n) and Windows (\r\n) line endings
+  const lines = output.trim().split(/\r?\n/);
 
 
   for (const line of lines) {
   for (const line of lines) {
     if (!line.trim()) continue;
     if (!line.trim()) continue;

+ 5 - 16
src/tools/grep/constants.ts

@@ -5,6 +5,8 @@ import {
   downloadAndInstallRipgrep,
   downloadAndInstallRipgrep,
   getInstalledRipgrepPath,
   getInstalledRipgrepPath,
 } from './downloader';
 } from './downloader';
+import { homedir } from 'os';
+
 
 
 export type GrepBackend = 'rg' | 'grep';
 export type GrepBackend = 'rg' | 'grep';
 
 
@@ -23,7 +25,8 @@ function findExecutable(name: string): string | null {
   try {
   try {
     const result = spawnSync(cmd, [name], { encoding: 'utf-8', timeout: 5000 });
     const result = spawnSync(cmd, [name], { encoding: 'utf-8', timeout: 5000 });
     if (result.status === 0 && result.stdout.trim()) {
     if (result.status === 0 && result.stdout.trim()) {
-      return result.stdout.trim().split('\n')[0];
+      // Handle both Unix (\n) and Windows (\r\n) line endings
+      return result.stdout.trim().split(/\r?\n/)[0];
     }
     }
   } catch {
   } catch {
     // Command execution failed
     // Command execution failed
@@ -31,20 +34,6 @@ function findExecutable(name: string): string | null {
   return null;
   return null;
 }
 }
 
 
-function getDataDir(): string {
-  if (process.platform === 'win32') {
-    return (
-      process.env.LOCALAPPDATA ||
-      process.env.APPDATA ||
-      join(process.env.USERPROFILE || '.', 'AppData', 'Local')
-    );
-  }
-  return (
-    process.env.XDG_DATA_HOME ||
-    join(process.env.HOME || '.', '.local', 'share')
-  );
-}
-
 function getOpenCodeBundledRg(): string | null {
 function getOpenCodeBundledRg(): string | null {
   const execPath = process.execPath;
   const execPath = process.execPath;
   const execDir = dirname(execPath);
   const execDir = dirname(execPath);
@@ -54,7 +43,7 @@ function getOpenCodeBundledRg(): string | null {
 
 
   const candidates = [
   const candidates = [
     // OpenCode XDG data path (highest priority - where OpenCode installs rg)
     // OpenCode XDG data path (highest priority - where OpenCode installs rg)
-    join(getDataDir(), 'opencode', 'bin', rgName),
+    join(homedir(), '.opencode', 'bin', rgName),
     // Legacy paths relative to execPath
     // Legacy paths relative to execPath
     join(execDir, rgName),
     join(execDir, rgName),
     join(execDir, 'bin', rgName),
     join(execDir, 'bin', rgName),

+ 209 - 0
src/tools/grep/grep.test.ts

@@ -0,0 +1,209 @@
+import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
+import { join } from 'node:path';
+import { mkdir, rm, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { runRg, runRgCount } from './cli';
+import { formatGrepResult } from './utils';
+import { grep } from './tools';
+
+describe('grep tool', () => {
+  const testDir = join(tmpdir(), `grep-test-${Date.now()}`);
+  const testFile1 = join(testDir, 'test1.txt');
+  const testFile2 = join(testDir, 'test2.ts');
+
+  beforeAll(async () => {
+    await mkdir(testDir, { recursive: true });
+    await writeFile(testFile1, 'Hello world\nThis is a test file\nAnother line with match');
+    await writeFile(testFile2, 'const x = \'Hello world\';\nconsole.log(\'test\');');
+  });
+
+  afterAll(async () => {
+    await rm(testDir, { recursive: true, force: true });
+  });
+
+  describe('formatGrepResult', () => {
+    test('formats empty results', () => {
+      const result = {
+        matches: [],
+        totalMatches: 0,
+        filesSearched: 10,
+        truncated: false,
+      };
+      expect(formatGrepResult(result)).toBe('No matches found.');
+    });
+
+    test('formats error results', () => {
+      const result = {
+        matches: [],
+        totalMatches: 0,
+        filesSearched: 0,
+        truncated: false,
+        error: 'Something went wrong',
+      };
+      expect(formatGrepResult(result)).toBe('Error: Something went wrong');
+    });
+
+    test('formats matches correctly', () => {
+      const result = {
+        matches: [
+          { file: 'file1.ts', line: 10, text: 'const foo = \'bar\'' },
+          { file: 'file1.ts', line: 15, text: 'console.log(foo)' },
+          { file: 'file2.ts', line: 5, text: 'import { foo } from \'./file1\'' },
+        ],
+        totalMatches: 3,
+        filesSearched: 2,
+        truncated: false,
+      };
+
+      const output = formatGrepResult(result);
+      expect(output).toContain('file1.ts:');
+      expect(output).toContain('  10: const foo = \'bar\'');
+      expect(output).toContain('  15: console.log(foo)');
+      expect(output).toContain('file2.ts:');
+      expect(output).toContain('  5: import { foo } from \'./file1\'');
+      expect(output).toContain('Found 3 matches in 2 files');
+    });
+
+    test('indicates truncation', () => {
+      const result = {
+        matches: [{ file: 'foo.txt', line: 1, text: 'bar' }],
+        totalMatches: 100,
+        filesSearched: 50,
+        truncated: true,
+      };
+      expect(formatGrepResult(result)).toContain('(output truncated)');
+    });
+  });
+
+  describe('runRg', () => {
+    test('finds matches in files', async () => {
+      const result = await runRg({
+        pattern: 'Hello',
+        paths: [testDir],
+      });
+
+      expect(result.totalMatches).toBeGreaterThanOrEqual(2);
+      expect(result.matches.some((m) => m.file.includes('test1.txt') && m.text.includes('Hello'))).toBe(true);
+      expect(result.matches.some((m) => m.file.includes('test2.ts') && m.text.includes('Hello'))).toBe(true);
+    });
+
+    test('respects file inclusion patterns', async () => {
+      const result = await runRg({
+        pattern: 'Hello',
+        paths: [testDir],
+        globs: ['*.txt'],
+      });
+
+      expect(result.matches.some((m) => m.file.includes('test1.txt'))).toBe(true);
+      expect(result.matches.some((m) => m.file.includes('test2.ts'))).toBe(false);
+    });
+
+    test('handles no matches', async () => {
+      const result = await runRg({
+        pattern: 'NonExistentString12345',
+        paths: [testDir],
+      });
+
+      expect(result.totalMatches).toBe(0);
+      expect(result.matches).toHaveLength(0);
+    });
+
+    test('respects case sensitivity', async () => {
+      // Default is case insensitive (smart case usually, but wrapper might default differently, let's check implementation)
+      // Looking at cli.ts: if (!options.caseSensitive) args.push("-i")
+      // So default is case insensitive.
+
+      const resultInsensitive = await runRg({
+        pattern: 'hello',
+        paths: [testDir],
+        caseSensitive: false
+      });
+      expect(resultInsensitive.totalMatches).toBeGreaterThan(0);
+
+      const resultSensitive = await runRg({
+        pattern: 'hello', // File has "Hello"
+        paths: [testDir],
+        caseSensitive: true
+      });
+      expect(resultSensitive.totalMatches).toBe(0);
+    });
+
+    test('respects whole word match', async () => {
+      const resultPartial = await runRg({
+        pattern: 'Hell',
+        paths: [testDir],
+        wholeWord: false
+      });
+      expect(resultPartial.totalMatches).toBeGreaterThan(0);
+
+      const resultWhole = await runRg({
+        pattern: 'Hell',
+        paths: [testDir],
+        wholeWord: true
+      });
+      expect(resultWhole.totalMatches).toBe(0);
+    });
+
+    test('respects max count', async () => {
+      const result = await runRg({
+        pattern: 'Hello',
+        paths: [testDir],
+        maxCount: 1
+      });
+      // maxCount is per file
+      expect(result.matches.filter(m => m.file.includes('test1.txt')).length).toBeLessThanOrEqual(1);
+    });
+  });
+
+  describe('runRgCount', () => {
+    test('counts matches correctly', async () => {
+      const results = await runRgCount({
+        pattern: 'Hello',
+        paths: [testDir]
+      });
+
+      expect(results.length).toBeGreaterThan(0);
+      const file1Result = results.find(r => r.file.includes('test1.txt'));
+      expect(file1Result).toBeDefined();
+      expect(file1Result?.count).toBe(1);
+    });
+  });
+
+  describe('grep tool execute', () => {
+    test('executes successfully', async () => {
+      // @ts-ignore
+      const result = await grep.execute({
+        pattern: 'Hello',
+        path: testDir,
+      });
+
+      expect(typeof result).toBe('string');
+      expect(result).toContain('Found');
+      expect(result).toContain('matches');
+    });
+
+    test('handles errors gracefully', async () => {
+      // @ts-ignore
+      const result = await grep.execute({
+        pattern: 'Hello',
+        path: '/non/existent/path/12345',
+      });
+
+      // Depending on implementation, it might return "No matches found" or an error string
+      // But it should not throw
+      expect(typeof result).toBe('string');
+    });
+
+    test('respects include pattern in execute', async () => {
+      // @ts-ignore
+      const result = await grep.execute({
+        pattern: 'Hello',
+        path: testDir,
+        include: '*.txt'
+      });
+
+      expect(result).toContain('test1.txt');
+      expect(result).not.toContain('test2.ts');
+    });
+  });
+});