fix(addon-search): harden search state and lifecycle
Normalize and snapshot search options, clear stale decorations when decorations are disabled, use try/finally for search events, and refresh highlights without re-entering public navigation APIs. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -70,22 +70,38 @@ export class SearchAddon extends Disposable implements ITerminalAddon, ISearchAp
|
||||
|
||||
private _updateMatches(): void {
|
||||
this._highlightTimeout.clear();
|
||||
if (this._state.cachedSearchTerm && this._state.lastSearchOptions?.decorations) {
|
||||
const lastOptions = this._state.lastSearchOptions;
|
||||
if (this._state.cachedSearchTerm && lastOptions?.decorations) {
|
||||
this._highlightTimeout.value = disposableTimeout(() => {
|
||||
const term = this._state.cachedSearchTerm;
|
||||
this._state.clearCachedTerm();
|
||||
this.findPrevious(term!, { ...this._state.lastSearchOptions, incremental: true }, { noScroll: true });
|
||||
const term = this._state.cachedSearchTerm!;
|
||||
this._refreshHighlights(term, lastOptions);
|
||||
}, 200);
|
||||
}
|
||||
}
|
||||
|
||||
private _refreshHighlights(term: string, searchOptions: ISearchOptions): void {
|
||||
if (!this._terminal || !this._engine || !this._decorationManager) {
|
||||
return;
|
||||
}
|
||||
if (!this._state.isValidSearchTerm(term) || !searchOptions.decorations) {
|
||||
return;
|
||||
}
|
||||
this._highlightAllMatches(term, searchOptions);
|
||||
this._fireResults(searchOptions);
|
||||
}
|
||||
|
||||
public clearDecorations(retainCachedSearchTerm?: boolean): void {
|
||||
this._highlightTimeout.clear();
|
||||
const hadResults = this._resultTracker.searchResults.length > 0;
|
||||
this._resultTracker.clearSelectedDecoration();
|
||||
this._decorationManager?.clearHighlightDecorations();
|
||||
this._resultTracker.clearResults();
|
||||
if (!retainCachedSearchTerm) {
|
||||
this._state.clearCachedTerm();
|
||||
this._state.lastSearchOptions = undefined;
|
||||
}
|
||||
if (hadResults && this._state.hadDecorations) {
|
||||
this._resultTracker.fireResultsChanged(true);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -105,19 +121,25 @@ export class SearchAddon extends Disposable implements ITerminalAddon, ISearchAp
|
||||
throw new Error('Cannot use addon until it has been loaded');
|
||||
}
|
||||
|
||||
this._highlightTimeout.clear();
|
||||
this._onBeforeSearch.fire();
|
||||
let found = false;
|
||||
try {
|
||||
if (this._state.shouldClearDecorations(searchOptions)) {
|
||||
this.clearDecorations(true);
|
||||
}
|
||||
if (this._state.shouldUpdateHighlighting(term, searchOptions)) {
|
||||
this._highlightAllMatches(term, searchOptions!);
|
||||
}
|
||||
|
||||
if (this._state.shouldUpdateHighlighting(term, searchOptions)) {
|
||||
this._highlightAllMatches(term, searchOptions!);
|
||||
found = this._findNextAndSelect(term, searchOptions, internalSearchOptions);
|
||||
this._state.cachedSearchTerm = term;
|
||||
this._state.lastSearchOptions = searchOptions;
|
||||
this._fireResults(searchOptions);
|
||||
} finally {
|
||||
this._onAfterSearch.fire();
|
||||
}
|
||||
|
||||
const found = this._findNextAndSelect(term, searchOptions, internalSearchOptions);
|
||||
this._fireResults(searchOptions);
|
||||
this._state.cachedSearchTerm = term;
|
||||
this._state.lastSearchOptions = searchOptions;
|
||||
|
||||
this._onAfterSearch.fire();
|
||||
|
||||
return found;
|
||||
}
|
||||
|
||||
@@ -167,19 +189,25 @@ export class SearchAddon extends Disposable implements ITerminalAddon, ISearchAp
|
||||
throw new Error('Cannot use addon until it has been loaded');
|
||||
}
|
||||
|
||||
this._highlightTimeout.clear();
|
||||
this._onBeforeSearch.fire();
|
||||
let found = false;
|
||||
try {
|
||||
if (this._state.shouldClearDecorations(searchOptions)) {
|
||||
this.clearDecorations(true);
|
||||
}
|
||||
if (this._state.shouldUpdateHighlighting(term, searchOptions)) {
|
||||
this._highlightAllMatches(term, searchOptions!);
|
||||
}
|
||||
|
||||
if (this._state.shouldUpdateHighlighting(term, searchOptions)) {
|
||||
this._highlightAllMatches(term, searchOptions!);
|
||||
found = this._findPreviousAndSelect(term, searchOptions, internalSearchOptions);
|
||||
this._state.cachedSearchTerm = term;
|
||||
this._state.lastSearchOptions = searchOptions;
|
||||
this._fireResults(searchOptions);
|
||||
} finally {
|
||||
this._onAfterSearch.fire();
|
||||
}
|
||||
|
||||
const found = this._findPreviousAndSelect(term, searchOptions, internalSearchOptions);
|
||||
this._fireResults(searchOptions);
|
||||
this._state.cachedSearchTerm = term;
|
||||
this._state.lastSearchOptions = searchOptions;
|
||||
|
||||
this._onAfterSearch.fire();
|
||||
|
||||
return found;
|
||||
}
|
||||
|
||||
|
||||
@@ -43,6 +43,23 @@ describe('SearchState', () => {
|
||||
state.lastSearchOptions = options;
|
||||
assert.strictEqual(state.didOptionsChange({ caseSensitive: false, regex: false, wholeWord: false }), false);
|
||||
});
|
||||
|
||||
it('should treat omitted flags as equivalent to false', () => {
|
||||
state.lastSearchOptions = { caseSensitive: false, regex: false, wholeWord: false };
|
||||
assert.strictEqual(state.didOptionsChange({}), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldClearDecorations', () => {
|
||||
it('should clear when decorations are disabled after being enabled', () => {
|
||||
state.lastSearchOptions = DECORATIONS_A;
|
||||
assert.strictEqual(state.shouldClearDecorations({ caseSensitive: false }), true);
|
||||
});
|
||||
|
||||
it('should not clear when decorations remain enabled', () => {
|
||||
state.lastSearchOptions = DECORATIONS_A;
|
||||
assert.strictEqual(state.shouldClearDecorations(DECORATIONS_A), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldUpdateHighlighting', () => {
|
||||
|
||||
@@ -5,13 +5,22 @@
|
||||
|
||||
import type { ISearchDecorationOptions, ISearchOptions } from '@xterm/addon-search';
|
||||
|
||||
interface INormalizedSearchOptions {
|
||||
caseSensitive: boolean;
|
||||
regex: boolean;
|
||||
wholeWord: boolean;
|
||||
incremental: boolean;
|
||||
decorations?: ISearchDecorationOptions;
|
||||
}
|
||||
|
||||
/**
|
||||
* Manages search state including cached search terms, options tracking, and validation.
|
||||
* This class provides a centralized way to handle search state consistency and option changes.
|
||||
*/
|
||||
export class SearchState {
|
||||
private _cachedSearchTerm: string | undefined;
|
||||
private _lastSearchOptions: ISearchOptions | undefined;
|
||||
private _lastSearchOptions: INormalizedSearchOptions | undefined;
|
||||
private _hadDecorations: boolean = false;
|
||||
|
||||
/**
|
||||
* Gets the currently cached search term.
|
||||
@@ -31,14 +40,22 @@ export class SearchState {
|
||||
* Gets the last search options used.
|
||||
*/
|
||||
public get lastSearchOptions(): ISearchOptions | undefined {
|
||||
return this._lastSearchOptions;
|
||||
return this._toPublicOptions(this._lastSearchOptions);
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the last search options used.
|
||||
*/
|
||||
public set lastSearchOptions(options: ISearchOptions | undefined) {
|
||||
this._lastSearchOptions = options;
|
||||
this._lastSearchOptions = options ? this._normalizeOptions(options) : undefined;
|
||||
this._hadDecorations = !!this._lastSearchOptions?.decorations;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the previous search used decorations.
|
||||
*/
|
||||
public get hadDecorations(): boolean {
|
||||
return this._hadDecorations;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -62,21 +79,45 @@ export class SearchState {
|
||||
if (!newOptions) {
|
||||
return false;
|
||||
}
|
||||
if (this._lastSearchOptions.caseSensitive !== newOptions.caseSensitive) {
|
||||
const normalized = this._normalizeOptions(newOptions);
|
||||
if (this._lastSearchOptions.caseSensitive !== normalized.caseSensitive) {
|
||||
return true;
|
||||
}
|
||||
if (this._lastSearchOptions.regex !== newOptions.regex) {
|
||||
if (this._lastSearchOptions.regex !== normalized.regex) {
|
||||
return true;
|
||||
}
|
||||
if (this._lastSearchOptions.wholeWord !== newOptions.wholeWord) {
|
||||
if (this._lastSearchOptions.wholeWord !== normalized.wholeWord) {
|
||||
return true;
|
||||
}
|
||||
if (this._didDecorationOptionsChange(this._lastSearchOptions.decorations, newOptions.decorations)) {
|
||||
if (this._didDecorationOptionsChange(this._lastSearchOptions.decorations, normalized.decorations)) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private _normalizeOptions(options: ISearchOptions): INormalizedSearchOptions {
|
||||
return {
|
||||
caseSensitive: !!options.caseSensitive,
|
||||
regex: !!options.regex,
|
||||
wholeWord: !!options.wholeWord,
|
||||
incremental: !!options.incremental,
|
||||
decorations: options.decorations ? { ...options.decorations } : undefined
|
||||
};
|
||||
}
|
||||
|
||||
private _toPublicOptions(options?: INormalizedSearchOptions): ISearchOptions | undefined {
|
||||
if (!options) {
|
||||
return undefined;
|
||||
}
|
||||
return {
|
||||
caseSensitive: options.caseSensitive,
|
||||
regex: options.regex,
|
||||
wholeWord: options.wholeWord,
|
||||
incremental: options.incremental,
|
||||
decorations: options.decorations ? { ...options.decorations } : undefined
|
||||
};
|
||||
}
|
||||
|
||||
private _didDecorationOptionsChange(previous?: ISearchDecorationOptions, current?: ISearchDecorationOptions): boolean {
|
||||
if (!previous && !current) {
|
||||
return false;
|
||||
@@ -107,6 +148,13 @@ export class SearchState {
|
||||
this.didOptionsChange(options);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether decorations were disabled after previously being enabled.
|
||||
*/
|
||||
public shouldClearDecorations(options?: ISearchOptions): boolean {
|
||||
return this._hadDecorations && !options?.decorations;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clears the cached search term.
|
||||
*/
|
||||
@@ -120,5 +168,6 @@ export class SearchState {
|
||||
public reset(): void {
|
||||
this._cachedSearchTerm = undefined;
|
||||
this._lastSearchOptions = undefined;
|
||||
this._hadDecorations = false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user