diff --git a/src/renderer/src/components/TerminalSearch.test.tsx b/src/renderer/src/components/TerminalSearch.test.tsx index f004bed4f9a..2cfa3d89051 100644 --- a/src/renderer/src/components/TerminalSearch.test.tsx +++ b/src/renderer/src/components/TerminalSearch.test.tsx @@ -1,15 +1,22 @@ // @vitest-environment happy-dom -import { cleanup, fireEvent, render, waitFor } from '@testing-library/react' +import { act, cleanup, fireEvent, render } from '@testing-library/react' import type { SearchAddon } from '@xterm/addon-search' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { matchSearchNavigate, type SearchState } from '@/components/terminal-pane/keyboard-handlers' import TerminalSearch from './TerminalSearch' vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) -afterEach(cleanup) +beforeEach(() => vi.useFakeTimers()) + +afterEach(() => { + cleanup() + vi.clearAllTimers() + vi.useRealTimers() +}) function createSearchAddon(): SearchAddon { return { @@ -19,68 +26,235 @@ function createSearchAddon(): SearchAddon { } as unknown as SearchAddon } -function renderSearch(searchAddon: SearchAddon): ReturnType { +function clearAddonMocks(addon: SearchAddon): void { + vi.mocked(addon.findNext).mockClear() + vi.mocked(addon.findPrevious).mockClear() + vi.mocked(addon.clearDecorations).mockClear() +} + +function advanceSearchDebounce(): void { + act(() => vi.advanceTimersByTime(75)) +} + +function renderSearch( + searchAddon: SearchAddon, + searchStateRef: React.RefObject = { + current: { query: '', caseSensitive: false, regex: false } + } +): ReturnType { return render( ) } -describe('TerminalSearch cleanup', () => { - it('clears the current addon when the query is erased', async () => { +describe('TerminalSearch debounce', () => { + it('runs one search with the latest query after a typing burst', () => { const addon = createSearchAddon() const view = renderSearch(addon) + const input = view.getByPlaceholderText('Search...') + clearAddonMocks(addon) - fireEvent.change(view.getByPlaceholderText('Search...'), { target: { value: 'needle' } }) - await waitFor(() => expect(addon.findNext).toHaveBeenCalled()) - vi.mocked(addon.clearDecorations).mockClear() - vi.mocked(addon.findNext).mockClear() + fireEvent.change(input, { target: { value: 'n' } }) + act(() => vi.advanceTimersByTime(30)) + fireEvent.change(input, { target: { value: 'nee' } }) + act(() => vi.advanceTimersByTime(30)) + fireEvent.change(input, { target: { value: 'needle' } }) - fireEvent.change(view.getByPlaceholderText('Search...'), { target: { value: '' } }) + act(() => vi.advanceTimersByTime(74)) + expect(addon.findNext).not.toHaveBeenCalled() - await waitFor(() => expect(addon.clearDecorations).toHaveBeenCalledTimes(1)) - expect(addon.findNext).toHaveBeenCalledWith('') + act(() => vi.advanceTimersByTime(1)) + expect(addon.findNext).toHaveBeenCalledTimes(1) + expect(addon.findNext).toHaveBeenCalledWith( + 'needle', + expect.objectContaining({ caseSensitive: false, regex: false, incremental: true }) + ) }) - it('clears the previous addon when the search moves to another pane', async () => { + it('updates shortcut search state on every input change before the debounce', () => { + const addon = createSearchAddon() + const searchStateRef = { + current: { query: '', caseSensitive: false, regex: false } + } + const view = renderSearch(addon, searchStateRef) + const input = view.getByPlaceholderText('Search...') + + fireEvent.change(input, { target: { value: 'n' } }) + expect(searchStateRef.current).toEqual({ query: 'n', caseSensitive: false, regex: false }) + fireEvent.change(input, { target: { value: 'needle' } }) + expect(searchStateRef.current).toEqual({ query: 'needle', caseSensitive: false, regex: false }) + }) + + it.each([ + ['macOS', true, { metaKey: true, ctrlKey: false }], + ['Linux/Windows', false, { metaKey: false, ctrlKey: true }] + ])('exposes the pending query to %s search navigation', (_platform, isMac, modifiers) => { + const addon = createSearchAddon() + const searchStateRef = { + current: { query: '', caseSensitive: false, regex: false } + } + const view = renderSearch(addon, searchStateRef) + fireEvent.change(view.getByPlaceholderText('Search...'), { target: { value: 'needle' } }) + + expect( + matchSearchNavigate( + { key: 'g', shiftKey: false, altKey: false, ...modifiers }, + isMac, + true, + searchStateRef.current + ) + ).toBe('next') + }) + + it('flushes an armed search on Enter and does not rerun its timer', () => { + const addon = createSearchAddon() + const view = renderSearch(addon) + const input = view.getByPlaceholderText('Search...') + clearAddonMocks(addon) + + fireEvent.change(input, { target: { value: 'needle' } }) + fireEvent.keyDown(input, { key: 'Enter' }) + + expect(addon.findNext).toHaveBeenCalledTimes(2) + expect(addon.findNext).toHaveBeenNthCalledWith( + 1, + 'needle', + expect.objectContaining({ incremental: true }) + ) + expect(addon.findNext).toHaveBeenNthCalledWith( + 2, + 'needle', + expect.objectContaining({ incremental: false }) + ) + + advanceSearchDebounce() + expect(addon.findNext).toHaveBeenCalledTimes(2) + }) + + it('flushes before Shift+Enter navigates to the previous match', () => { + const addon = createSearchAddon() + const view = renderSearch(addon) + const input = view.getByPlaceholderText('Search...') + clearAddonMocks(addon) + + fireEvent.change(input, { target: { value: 'needle' } }) + fireEvent.keyDown(input, { key: 'Enter', shiftKey: true }) + + expect(addon.findNext).toHaveBeenCalledTimes(1) + expect(addon.findNext).toHaveBeenCalledWith( + 'needle', + expect.objectContaining({ incremental: true }) + ) + expect(addon.findPrevious).toHaveBeenCalledWith( + 'needle', + expect.objectContaining({ incremental: false }) + ) + }) + + it('restarts an armed search with the latest option values', () => { + const addon = createSearchAddon() + const view = renderSearch(addon) + const input = view.getByPlaceholderText('Search...') + clearAddonMocks(addon) + + fireEvent.change(input, { target: { value: 'needle' } }) + fireEvent.click(view.getByTitle('Case sensitive')) + fireEvent.click(view.getByTitle('Regex')) + advanceSearchDebounce() + + expect(addon.findNext).toHaveBeenCalledTimes(1) + expect(addon.findNext).toHaveBeenCalledWith( + 'needle', + expect.objectContaining({ caseSensitive: true, regex: true, incremental: true }) + ) + }) +}) + +describe('TerminalSearch cleanup', () => { + it('clears immediately when the query is erased and cancels its search', () => { + const addon = createSearchAddon() + const view = renderSearch(addon) + const input = view.getByPlaceholderText('Search...') + clearAddonMocks(addon) + + fireEvent.change(input, { target: { value: 'needle' } }) + fireEvent.change(input, { target: { value: '' } }) + + expect(addon.clearDecorations).toHaveBeenCalledTimes(1) + expect(addon.findNext).toHaveBeenCalledWith('') + advanceSearchDebounce() + expect(addon.findNext).toHaveBeenCalledTimes(1) + }) + + it('clears immediately when the search closes', () => { + const addon = createSearchAddon() + const searchStateRef = { + current: { query: '', caseSensitive: false, regex: false } + } + const view = renderSearch(addon, searchStateRef) + fireEvent.change(view.getByPlaceholderText('Search...'), { target: { value: 'needle' } }) + clearAddonMocks(addon) + + view.rerender( + + ) + + expect(addon.clearDecorations).toHaveBeenCalledTimes(1) + expect(addon.findNext).toHaveBeenCalledWith('') + advanceSearchDebounce() + expect(addon.findNext).toHaveBeenCalledTimes(1) + }) + + it('clears the previous addon and does not run its armed search after a pane swap', () => { const previousAddon = createSearchAddon() const nextAddon = createSearchAddon() - const view = renderSearch(previousAddon) - + const searchStateRef = { + current: { query: '', caseSensitive: false, regex: false } + } + const view = renderSearch(previousAddon, searchStateRef) fireEvent.change(view.getByPlaceholderText('Search...'), { target: { value: 'needle' } }) - await waitFor(() => expect(previousAddon.findNext).toHaveBeenCalled()) - vi.mocked(previousAddon.clearDecorations).mockClear() - vi.mocked(previousAddon.findNext).mockClear() + clearAddonMocks(previousAddon) view.rerender( ) expect(previousAddon.clearDecorations).toHaveBeenCalledTimes(1) expect(previousAddon.findNext).toHaveBeenCalledWith('') + advanceSearchDebounce() + expect(previousAddon.findNext).toHaveBeenCalledTimes(1) + expect(nextAddon.findNext).toHaveBeenCalledWith( + 'needle', + expect.objectContaining({ incremental: true }) + ) }) - it('clears the addon when the search portal unmounts', async () => { + it('clears the addon and cancels the armed search on unmount', () => { const addon = createSearchAddon() const view = renderSearch(addon) - fireEvent.change(view.getByPlaceholderText('Search...'), { target: { value: 'needle' } }) - await waitFor(() => expect(addon.findNext).toHaveBeenCalled()) - vi.mocked(addon.clearDecorations).mockClear() - vi.mocked(addon.findNext).mockClear() + clearAddonMocks(addon) view.unmount() expect(addon.clearDecorations).toHaveBeenCalledTimes(1) expect(addon.findNext).toHaveBeenCalledWith('') + advanceSearchDebounce() + expect(addon.findNext).toHaveBeenCalledTimes(1) }) }) diff --git a/src/renderer/src/components/TerminalSearch.tsx b/src/renderer/src/components/TerminalSearch.tsx index ed8bf1b906a..8f848089e7e 100644 --- a/src/renderer/src/components/TerminalSearch.tsx +++ b/src/renderer/src/components/TerminalSearch.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState, useCallback } from 'react' +import { useEffect, useRef, useState, useCallback } from 'react' import { ChevronUp, ChevronDown, X, CaseSensitive, Regex } from 'lucide-react' import type { SearchAddon } from '@xterm/addon-search' import { Button } from '@/components/ui/button' @@ -14,6 +14,14 @@ type TerminalSearchProps = { searchStateRef: React.RefObject } +const SEARCH_DEBOUNCE_MS = 75 + +type PendingSearch = { + addon: SearchAddon + query: string + options: NonNullable[1]> +} + function clearTerminalSearch(searchAddon: SearchAddon | null): void { if (!searchAddon) { return @@ -32,6 +40,8 @@ export default function TerminalSearch({ const [query, setQuery] = useState('') const [caseSensitive, setCaseSensitive] = useState(false) const [regex, setRegex] = useState(false) + const pendingSearchRef = useRef(null) + const searchTimerRef = useRef(null) const requestQuery = getFindRequestQuery(query) // Why: the default xterm SearchAddon highlights blend into common @@ -81,11 +91,32 @@ export default function TerminalSearch({ input?.focus() }, []) + const cancelPendingSearch = useCallback((): void => { + if (searchTimerRef.current !== null) { + window.clearTimeout(searchTimerRef.current) + searchTimerRef.current = null + } + pendingSearchRef.current = null + }, []) + + const flushPendingSearch = useCallback((): void => { + const pending = pendingSearchRef.current + cancelPendingSearch() + if (pending) { + safeFind( + (term, options) => pending.addon.findNext(term, options), + pending.query, + pending.options + ) + } + }, [cancelPendingSearch]) + useEffect( () => () => { + cancelPendingSearch() clearTerminalSearch(searchAddon) }, - [searchAddon] + [cancelPendingSearch, searchAddon] ) useEffect(() => { @@ -93,6 +124,7 @@ export default function TerminalSearch({ // can read the current search state without lifting it to parent state. searchStateRef.current = { query: requestQuery ?? '', caseSensitive, regex } + cancelPendingSearch() if (!isOpen) { clearTerminalSearch(searchAddon) return @@ -102,13 +134,32 @@ export default function TerminalSearch({ return } if (searchAddon) { - safeFind( - (term, options) => searchAddon.findNext(term, options), - requestQuery, - searchOptions(true) - ) + const pending = { addon: searchAddon, query: requestQuery, options: searchOptions(true) } + pendingSearchRef.current = pending + searchTimerRef.current = window.setTimeout(() => { + if (pendingSearchRef.current !== pending) { + return + } + pendingSearchRef.current = null + searchTimerRef.current = null + safeFind( + (term, options) => pending.addon.findNext(term, options), + pending.query, + pending.options + ) + }, SEARCH_DEBOUNCE_MS) } - }, [requestQuery, searchAddon, isOpen, caseSensitive, regex, searchStateRef, searchOptions]) + return cancelPendingSearch + }, [ + requestQuery, + searchAddon, + isOpen, + caseSensitive, + regex, + searchStateRef, + searchOptions, + cancelPendingSearch + ]) const handleKeyDown = useCallback( (e: React.KeyboardEvent) => { @@ -117,12 +168,26 @@ export default function TerminalSearch({ if (e.key === 'Escape') { onClose() } else if (e.key === 'Enter' && e.shiftKey) { + flushPendingSearch() findPrevious() } else if (e.key === 'Enter') { + flushPendingSearch() findNext() } }, - [onClose, findNext, findPrevious] + [onClose, findNext, findPrevious, flushPendingSearch] + ) + + const handleQueryChange = useCallback( + (nextQuery: string): void => { + searchStateRef.current = { + query: getFindRequestQuery(nextQuery) ?? '', + caseSensitive, + regex + } + setQuery(nextQuery) + }, + [caseSensitive, regex, searchStateRef] ) if (!isOpen) { @@ -140,7 +205,7 @@ export default function TerminalSearch({ ref={handleInputRef} type="text" value={query} - onChange={(e) => setQuery(e.target.value)} + onChange={(e) => handleQueryChange(e.target.value)} placeholder={translate('auto.components.TerminalSearch.e07012f26e', 'Search...')} className="min-w-0 flex-1 border-none bg-transparent text-sm text-white outline-none placeholder:text-zinc-500" />