diff options
| author | Evan Senter <[email protected]> | 2025-04-18 18:29:27 +0100 |
|---|---|---|
| committer | Evan Senter <[email protected]> | 2025-04-18 18:36:33 +0100 |
| commit | dbf4c3a37c55b8e14c9fefd1f839d3555072e17b (patch) | |
| tree | 8ae6e7136eaec78573cd86fb982e4a725079bfe7 /packages/cli/src | |
| parent | f330a87e50fb6e8e4f1949851f8f6b6cf53d7776 (diff) | |
Revert "Including a test harness for it, and making sure the cursor is always at the end."
This reverts commit 97db77997fd6369031d2f1cf750051999fb0b5b5.
Diffstat (limited to 'packages/cli/src')
| -rw-r--r-- | packages/cli/src/config/globalConfig.ts | 2 | ||||
| -rw-r--r-- | packages/cli/src/core/gemini-client.ts | 2 | ||||
| -rw-r--r-- | packages/cli/src/tools/terminal.tool.ts | 5 | ||||
| -rw-r--r-- | packages/cli/src/ui/App.test.tsx | 121 | ||||
| -rw-r--r-- | packages/cli/src/ui/App.tsx | 96 | ||||
| -rw-r--r-- | packages/cli/src/ui/components/InputPrompt.tsx | 3 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/useInputHistory.test.ts | 177 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/useInputHistory.ts | 92 |
8 files changed, 65 insertions, 433 deletions
diff --git a/packages/cli/src/config/globalConfig.ts b/packages/cli/src/config/globalConfig.ts index 89705ed6..2b6ad518 100644 --- a/packages/cli/src/config/globalConfig.ts +++ b/packages/cli/src/config/globalConfig.ts @@ -47,4 +47,4 @@ export function getConfig(): GlobalConfig { */ export function getModel(): string { return getConfig().model; -} +}
\ No newline at end of file diff --git a/packages/cli/src/core/gemini-client.ts b/packages/cli/src/core/gemini-client.ts index 0b79a2ad..6ce89816 100644 --- a/packages/cli/src/core/gemini-client.ts +++ b/packages/cli/src/core/gemini-client.ts @@ -9,7 +9,7 @@ import { Content, } from '@google/genai'; import { getApiKey } from '../config/env.js'; -import { getModel } from '../config/globalConfig.js'; +import { getModel } from '../config/globalConfig.js'; import { CoreSystemPrompt } from './prompts.js'; import { type ToolCallEvent, diff --git a/packages/cli/src/tools/terminal.tool.ts b/packages/cli/src/tools/terminal.tool.ts index ce524fa5..73f4af7d 100644 --- a/packages/cli/src/tools/terminal.tool.ts +++ b/packages/cli/src/tools/terminal.tool.ts @@ -142,7 +142,10 @@ export class TerminalTool extends BaseTool< private rejectShellReady: ((reason?: any) => void) | undefined; // Definite assignment assertion private readonly backgroundTerminalAnalyzer: BackgroundTerminalAnalyzer; - constructor(rootDirectory: string, outputLimit: number = MAX_OUTPUT_LENGTH) { + constructor( + rootDirectory: string, + outputLimit: number = MAX_OUTPUT_LENGTH, + ) { const toolDisplayName = 'Terminal'; // --- LLM-Facing Description --- // Updated description for background tasks to mention polling and LLM analysis diff --git a/packages/cli/src/ui/App.test.tsx b/packages/cli/src/ui/App.test.tsx deleted file mode 100644 index a13b4997..00000000 --- a/packages/cli/src/ui/App.test.tsx +++ /dev/null @@ -1,121 +0,0 @@ -// packages/cli/src/ui/App.test.tsx -import React from 'react'; -import { render, cleanup } from 'ink-testing-library'; -import { vi, describe, test, expect, beforeEach, afterEach } from 'vitest'; -import App from './App.js'; -import { useInputHistory } from './hooks/useInputHistory.js'; -import { useGeminiStream } from './hooks/useGeminiStream.js'; -import { StreamingState } from '../core/gemini-stream.js'; -import type { HistoryItem } from './types.js'; -import fs from 'fs'; -import path from 'path'; -import os from 'os'; -import { initializeConfig } from '../config/globalConfig.js'; - -// --- Mocks --- - -// Mock the useGeminiStream hook -vi.mock('./hooks/useGeminiStream.js', () => ({ - useGeminiStream: vi.fn(), -})); - -// Mock the useInputHistory hook -vi.mock('./hooks/useInputHistory.js', () => ({ - useInputHistory: vi.fn(), -})); - -// Mock fs/path/os used for warnings check -vi.mock('fs', () => ({ - default: { - existsSync: vi.fn().mockReturnValue(false), - readFileSync: vi.fn(), - unlinkSync: vi.fn(), - writeFileSync: vi.fn(), - }, -})); -vi.mock('path', async (importOriginal) => { - const originalPath = await importOriginal<typeof import('path')>(); - return { - ...originalPath, - default: originalPath, - join: originalPath.join, - resolve: originalPath.resolve, - relative: originalPath.relative, - }; -}); -vi.mock('os', async (importOriginal) => { - const originalOs = await importOriginal<typeof import('os')>(); - return { - ...originalOs, - default: originalOs, - tmpdir: vi.fn().mockReturnValue('/tmp'), - }; -}); - -// --- Test Suite --- -describe('App Component Rendering', () => { - // Define mock return values for the hooks - let mockSetQuery: ReturnType<typeof vi.fn>; - let mockResetHistoryNav: ReturnType<typeof vi.fn>; - let mockSubmitQuery: ReturnType<typeof vi.fn>; - - beforeEach(() => { - // Reset mocks - vi.clearAllMocks(); - (fs.existsSync as ReturnType<typeof vi.fn>).mockReturnValue(false); - - // Initialize global config - initializeConfig({ model: 'test-model-v1' }); - - // Setup mock return values for hooks - mockSetQuery = vi.fn(); - mockResetHistoryNav = vi.fn(); - mockSubmitQuery = vi.fn().mockResolvedValue(undefined); - - (useInputHistory as ReturnType<typeof vi.fn>).mockReturnValue({ - query: '', - setQuery: mockSetQuery, - resetHistoryNav: mockResetHistoryNav, - inputKey: 0, - }); - - (useGeminiStream as ReturnType<typeof vi.fn>).mockReturnValue({ - streamingState: StreamingState.Idle, - submitQuery: mockSubmitQuery, - initError: null, - }); - }); - - afterEach(() => { - cleanup(); - vi.resetModules(); - }); - - // Helper function to render App - const renderApp = (initialHookQuery = '') => { - (useInputHistory as ReturnType<typeof vi.fn>).mockReturnValue({ - query: initialHookQuery, - setQuery: mockSetQuery, - resetHistoryNav: mockResetHistoryNav, - inputKey: 0, - }); - - return render(<App directory="/test/dir" />); - }; - - // --- Tests --- - test('should render initial placeholder with model', () => { - const { lastFrame } = renderApp(); - expect(lastFrame()).toContain('Ask Gemini (test-model-v1)'); - }); - - test('should pass query from useInputHistory to InputPrompt', () => { - const { lastFrame } = renderApp('test query from hook'); - expect(lastFrame()).toContain('> test query from hook'); - }); - - // Add more tests here for App's behavior, like: - // - Displaying startup warnings when the mocked fs.existsSync returns true - // - Displaying initError from useGeminiStream when it's not null - // - Ensuring handleInputSubmit calls the correct functions from the hooks -}); diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index 5a88c90b..cda748f4 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -1,12 +1,11 @@ import React, { useState, useEffect, useMemo } from 'react'; -import { Box, Text } from 'ink'; +import { Box, Text, useInput } from 'ink'; import fs from 'fs'; import path from 'path'; import os from 'os'; import type { HistoryItem } from './types.js'; import { useGeminiStream } from './hooks/useGeminiStream.js'; import { useLoadingIndicator } from './hooks/useLoadingIndicator.js'; -import { useInputHistory } from './hooks/useInputHistory.js'; import Header from './components/Header.js'; import Tips from './components/Tips.js'; import HistoryDisplay from './components/HistoryDisplay.js'; @@ -23,8 +22,11 @@ interface AppProps { } const App = ({ directory }: AppProps) => { + const [query, setQuery] = useState(''); const [history, setHistory] = useState<HistoryItem[]>([]); const [startupWarnings, setStartupWarnings] = useState<string[]>([]); + const [historyIndex, setHistoryIndex] = useState<number>(-1); + const [originalQueryBeforeNav, setOriginalQueryBeforeNav] = useState<string>(''); const { streamingState, submitQuery, initError } = useGeminiStream(setHistory); const { elapsedTime, currentLoadingPhrase } = @@ -32,54 +34,34 @@ const App = ({ directory }: AppProps) => { const userMessages = useMemo(() => { return history - .filter( - (item): item is HistoryItem & { type: 'user'; text: string } => - item.type === 'user' && - typeof item.text === 'string' && - item.text.trim() !== '', - ) - .map((item) => item.text); + .filter((item): item is HistoryItem & { type: 'user'; text: string } => + item.type === 'user' && typeof item.text === 'string' && item.text.trim() !== '' + ) + .map(item => item.text); }, [history]); useEffect(() => { try { if (fs.existsSync(warningsFilePath)) { + console.log('[App] Found warnings file:', warningsFilePath); const warningsContent = fs.readFileSync(warningsFilePath, 'utf-8'); - setStartupWarnings( - warningsContent.split('\n').filter((line) => line.trim() !== ''), - ); + setStartupWarnings(warningsContent.split('\n').filter(line => line.trim() !== '')); try { - fs.unlinkSync(warningsFilePath); + fs.unlinkSync(warningsFilePath); } catch (unlinkErr: any) { - console.warn( - `[App] Warning: Could not delete warnings file: ${unlinkErr.message}`, - ); + console.warn(`[App] Warning: Could not delete warnings file: ${unlinkErr.message}`); } + } else { + console.log('[App] No warnings file found.'); } } catch (err: any) { - console.error( - `[App] Error checking/reading warnings file: ${err.message}`, - ); + console.error(`[App] Error checking/reading warnings file: ${err.message}`); } }, []); - const isWaitingForToolConfirmation = history.some( - (item) => - item.type === 'tool_group' && - item.tools.some((tool) => tool.confirmationDetails !== undefined), - ); - const isInputActive = - streamingState === StreamingState.Idle && - !initError && - !isWaitingForToolConfirmation; - - const { query, setQuery, resetHistoryNav, inputKey } = useInputHistory({ - userMessages, - isActive: isInputActive, - }); - const handleInputSubmit = (value: PartListUnion) => { - resetHistoryNav(); + setHistoryIndex(-1); + setOriginalQueryBeforeNav(''); submitQuery(value) .then(() => { setQuery(''); @@ -107,6 +89,47 @@ const App = ({ directory }: AppProps) => { } }, [initError, history]); + const isWaitingForToolConfirmation = history.some( + (item) => + item.type === 'tool_group' && + item.tools.some((tool) => tool.confirmationDetails !== undefined), + ); + const isInputActive = streamingState === StreamingState.Idle && !initError; + + useInput((input, key) => { + if (!isInputActive || isWaitingForToolConfirmation) { + return; + } + + if (key.upArrow) { + if (userMessages.length === 0) return; + if (historyIndex === -1) { + setOriginalQueryBeforeNav(query); + } + const nextIndex = Math.min(historyIndex + 1, userMessages.length - 1); + if (nextIndex !== historyIndex) { + setHistoryIndex(nextIndex); + setQuery(userMessages[userMessages.length - 1 - nextIndex]); + } + } else if (key.downArrow) { + if (historyIndex < 0) return; + const nextIndex = Math.max(historyIndex - 1, -1); + setHistoryIndex(nextIndex); + if (nextIndex === -1) { + setQuery(originalQueryBeforeNav); + } else { + setQuery(userMessages[userMessages.length - 1 - nextIndex]); + } + } else { + if (input || key.backspace || key.delete || key.leftArrow || key.rightArrow) { + if (historyIndex !== -1) { + setHistoryIndex(-1); + setOriginalQueryBeforeNav(''); + } + } + } + }, { isActive: isInputActive }); + return ( <Box flexDirection="column" padding={1} marginBottom={1} width="100%"> <Header cwd={directory} /> @@ -170,13 +193,12 @@ const App = ({ directory }: AppProps) => { /> </Box> - {!isWaitingForToolConfirmation && ( + {!isWaitingForToolConfirmation && isInputActive && ( <InputPrompt query={query} setQuery={setQuery} onSubmit={handleInputSubmit} isActive={isInputActive} - forceKey={inputKey} /> )} diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 1119926f..f79aeaa3 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -8,14 +8,12 @@ interface InputPromptProps { setQuery: (value: string) => void; onSubmit: (value: string) => void; isActive: boolean; - forceKey: number; } const InputPrompt: React.FC<InputPromptProps> = ({ query, setQuery, onSubmit, - forceKey, }) => { const model = getModel(); @@ -24,7 +22,6 @@ const InputPrompt: React.FC<InputPromptProps> = ({ <Text color={'white'}>> </Text> <Box flexGrow={1}> <TextInput - key={forceKey} value={query} onChange={setQuery} onSubmit={onSubmit} diff --git a/packages/cli/src/ui/hooks/useInputHistory.test.ts b/packages/cli/src/ui/hooks/useInputHistory.test.ts deleted file mode 100644 index 34af31b2..00000000 --- a/packages/cli/src/ui/hooks/useInputHistory.test.ts +++ /dev/null @@ -1,177 +0,0 @@ -// packages/cli/src/ui/hooks/useInputHistory.test.ts -import { renderHook, act } from '@testing-library/react'; -import { useInput } from 'ink'; -import { vi, describe, test, expect, beforeEach, Mock } from 'vitest'; -import { useInputHistory } from './useInputHistory.js'; - -// Mock the useInput hook from Ink -vi.mock('ink', async (importOriginal) => { - const originalInk = await importOriginal<typeof import('ink')>(); - return { - ...originalInk, // Keep other exports - useInput: vi.fn(), // Mock useInput - }; -}); - -// Helper type for the mocked useInput callback -type UseInputCallback = (input: string, key: any) => void; - -describe('useInputHistory Hook', () => { - let mockUseInputCallback: UseInputCallback | undefined; - const mockUserMessages = ['msg1', 'msg2', 'msg3']; // Sample history - - beforeEach(() => { - // Reset the mock before each test and capture the callback - (useInput as Mock).mockImplementation((callback, options) => { - // Only store the callback if the hook is active in the test - if (options?.isActive !== false) { - mockUseInputCallback = callback; - } else { - mockUseInputCallback = undefined; - } - }); - }); - - // Helper function to simulate key press by invoking the captured callback - const simulateKeyPress = (key: object, input: string = '') => { - act(() => { - if (mockUseInputCallback) { - mockUseInputCallback(input, key); - } else { - // Optionally throw an error if trying to press key when inactive - // console.warn('Simulated key press while useInput was inactive'); - } - }); - }; - - test('should initialize with empty query', () => { - const { result } = renderHook(() => - useInputHistory({ userMessages: [], isActive: true }), - ); - expect(result.current.query).toBe(''); - }); - - test('up arrow should do nothing if history is empty', () => { - const { result } = renderHook(() => - useInputHistory({ userMessages: [], isActive: true }), - ); - simulateKeyPress({ upArrow: true }); - expect(result.current.query).toBe(''); - }); - - test('up arrow should recall the last message', () => { - const { result } = renderHook(() => - useInputHistory({ userMessages: mockUserMessages, isActive: true }), - ); - simulateKeyPress({ upArrow: true }); - expect(result.current.query).toBe('msg3'); // Last message - }); - - test('repeated up arrows should navigate history', () => { - const { result } = renderHook(() => - useInputHistory({ userMessages: mockUserMessages, isActive: true }), - ); - simulateKeyPress({ upArrow: true }); // -> msg3 - simulateKeyPress({ upArrow: true }); // -> msg2 - expect(result.current.query).toBe('msg2'); - simulateKeyPress({ upArrow: true }); // -> msg1 - expect(result.current.query).toBe('msg1'); - simulateKeyPress({ upArrow: true }); // -> stays on msg1 - expect(result.current.query).toBe('msg1'); - }); - - test('down arrow should navigate history forward', () => { - const { result } = renderHook(() => - useInputHistory({ userMessages: mockUserMessages, isActive: true }), - ); - simulateKeyPress({ upArrow: true }); // -> msg3 - simulateKeyPress({ upArrow: true }); // -> msg2 - simulateKeyPress({ upArrow: true }); // -> msg1 - expect(result.current.query).toBe('msg1'); - - simulateKeyPress({ downArrow: true }); // -> msg2 - expect(result.current.query).toBe('msg2'); - simulateKeyPress({ downArrow: true }); // -> msg3 - expect(result.current.query).toBe('msg3'); - }); - - test('down arrow should restore original query', () => { - const { result } = renderHook(() => - useInputHistory({ userMessages: mockUserMessages, isActive: true }), - ); - - // Set initial query - act(() => { - result.current.setQuery('original typing'); - }); - expect(result.current.query).toBe('original typing'); - - simulateKeyPress({ upArrow: true }); // -> msg3 - expect(result.current.query).toBe('msg3'); - - simulateKeyPress({ downArrow: true }); // -> original typing - expect(result.current.query).toBe('original typing'); - - // Pressing down again should do nothing - simulateKeyPress({ downArrow: true }); - expect(result.current.query).toBe('original typing'); - }); - - test('typing should reset navigation', () => { - const { result } = renderHook(() => - useInputHistory({ userMessages: mockUserMessages, isActive: true }), - ); - - simulateKeyPress({ upArrow: true }); // -> msg3 - expect(result.current.query).toBe('msg3'); - - // Simulate typing 'x' (Note: we manually call setQuery here, as useInput is mocked) - act(() => { - result.current.setQuery(result.current.query + 'x'); - }); - // Also simulate the input event that would trigger the reset - simulateKeyPress({}, 'x'); - expect(result.current.query).toBe('msg3x'); - - simulateKeyPress({ upArrow: true }); // Should restart navigation -> msg3 - expect(result.current.query).toBe('msg3'); - }); - - test('calling resetHistoryNav should clear navigation state', () => { - const { result } = renderHook(() => - useInputHistory({ userMessages: mockUserMessages, isActive: true }), - ); - - // Set initial query and navigate - act(() => { - result.current.setQuery('original'); - }); - simulateKeyPress({ upArrow: true }); // -> msg3 - expect(result.current.query).toBe('msg3'); - - // Reset - act(() => { - result.current.resetHistoryNav(); - }); - - // Press down - should restore original query ('original') because nav was reset - // However, our current resetHistoryNav also clears originalQueryBeforeNav. - // Let's test that down does nothing because historyIndex is -1 - simulateKeyPress({ downArrow: true }); - expect(result.current.query).toBe('msg3'); // Stays msg3 because downArrow doesn't run when index is -1 - - // Press up - should start nav again from the top - simulateKeyPress({ upArrow: true }); - expect(result.current.query).toBe('msg3'); - }); - - test('should not trigger callback if isActive is false', () => { - renderHook(() => - useInputHistory({ userMessages: mockUserMessages, isActive: false }), - ); - // mockUseInputCallback should be undefined because isActive was false - expect(mockUseInputCallback).toBeUndefined(); - // Attempting to simulate should not throw error (or check internal state if possible) - expect(() => simulateKeyPress({ upArrow: true })).not.toThrow(); - }); -}); diff --git a/packages/cli/src/ui/hooks/useInputHistory.ts b/packages/cli/src/ui/hooks/useInputHistory.ts deleted file mode 100644 index bdd02e36..00000000 --- a/packages/cli/src/ui/hooks/useInputHistory.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { useState, useCallback } from 'react'; -import { useInput } from 'ink'; - -interface UseInputHistoryProps { - userMessages: readonly string[]; // Use readonly string[] instead - isActive: boolean; // To enable/disable the useInput hook -} - -interface UseInputHistoryReturn { - query: string; - setQuery: React.Dispatch<React.SetStateAction<string>>; - resetHistoryNav: () => void; - inputKey: number; // Key to force input reset -} - -export function useInputHistory({ - userMessages, - isActive, -}: UseInputHistoryProps): UseInputHistoryReturn { - const [query, setQuery] = useState(''); - const [historyIndex, setHistoryIndex] = useState<number>(-1); // -1 means current query - const [originalQueryBeforeNav, setOriginalQueryBeforeNav] = - useState<string>(''); - const [inputKey, setInputKey] = useState<number>(0); // Add key state - - const resetHistoryNav = useCallback(() => { - setHistoryIndex(-1); - setOriginalQueryBeforeNav(''); - // Don't reset inputKey here, only on explicit nav actions - }, []); - - useInput( - (input, key) => { - // Do nothing if the hook is not active - if (!isActive) { - return; - } - - if (key.upArrow) { - if (userMessages.length === 0) return; - // Store current query if starting navigation - if (historyIndex === -1) { - setOriginalQueryBeforeNav(query); - } - const nextIndex = Math.min(historyIndex + 1, userMessages.length - 1); - if (nextIndex !== historyIndex) { - setHistoryIndex(nextIndex); - const newValue = userMessages[userMessages.length - 1 - nextIndex]; - setQuery(newValue); - setInputKey(k => k + 1); // Increment key on navigation change - } - } else if (key.downArrow) { - if (historyIndex < 0) return; // Already at the bottom - const nextIndex = Math.max(historyIndex - 1, -1); - setHistoryIndex(nextIndex); - if (nextIndex === -1) { - // Restore original query - setQuery(originalQueryBeforeNav); - setInputKey(k => k + 1); // Increment key on navigation change - } else { - // Set query based on reversed index - const newValue = userMessages[userMessages.length - 1 - nextIndex]; - setQuery(newValue); - setInputKey(k => k + 1); // Increment key on navigation change - } - } else { - // If user types anything other than arrows, reset history navigation state - // This check might be too broad, adjust if handling more special keys - if ( - input || - key.backspace || - key.delete || - key.leftArrow || - key.rightArrow - ) { - if (historyIndex !== -1) { - resetHistoryNav(); - // Note: The actual input change is handled by the component using setQuery/onChange - } - } - } - }, - { isActive }, // Pass isActive to useInput - ); - - return { - query, - setQuery, // Return setQuery directly for flexibility - resetHistoryNav, - inputKey, // Return the key - }; -} |
