Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Improve terminal find behavior when there are more than 1000 results #182917

Merged
merged 3 commits into from May 19, 2023
Merged

Fix Improve terminal find behavior when there are more than 1000 results #182917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
393d160
Fixes #178017
jeanp413 May 18, 2023
3640b74
Merge branch 'main' into fix-178017
Tyriar May 19, 2023
59041c8
Use an option over a protected override
Tyriar May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.css
View file
Open in desktop
Expand Up @@ -34,6 +34,7 @@
border: 1px solid var(--vscode-contrastBorder);
border-bottom-left-radius: 4px;
border-bottom-right-radius: 4px;
font-size: 12px;
}

.monaco-workbench.reduce-motion .monaco-editor .find-widget {
Expand All @@ -57,9 +58,9 @@
}

.monaco-workbench .simple-find-part .matchesCount {
width: 68px;
max-width: 68px;
min-width: 68px;
width: 73px;
max-width: 73px;
min-width: 73px;
padding-left: 5px;
}

Expand Down
28 changes: 19 additions & 9 deletions src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts
View file
Open in desktop
Expand Up @@ -37,11 +37,12 @@ interface IFindOptions {
appendCaseSensitiveLabel?: string;
appendRegexLabel?: string;
appendWholeWordsLabel?: string;
matchesLimit?: number;
type?: 'Terminal' | 'Webview';
}

const SIMPLE_FIND_WIDGET_INITIAL_WIDTH = 310;
const MATCHES_COUNT_WIDTH = 68;
const MATCHES_COUNT_WIDTH = 73;

export abstract class SimpleFindWidget extends Widget {
private readonly _findInput: FindInput;
Expand All @@ -52,6 +53,7 @@ export abstract class SimpleFindWidget extends Widget {
private readonly _updateHistoryDelayer: Delayer<void>;
private readonly prevBtn: SimpleButton;
private readonly nextBtn: SimpleButton;
private readonly _matchesLimit: number;
private _matchesCount: HTMLElement | undefined;

private _isVisible: boolean = false;
Expand All @@ -68,6 +70,8 @@ export abstract class SimpleFindWidget extends Widget {
) {
super();

this._matchesLimit = options.matchesLimit ?? Number.MAX_SAFE_INTEGER;

this._findInput = this._register(new ContextScopedFindInput(null, contextViewService, {
label: NLS_FIND_INPUT_LABEL,
placeholder: NLS_FIND_INPUT_PLACEHOLDER,
Expand Down Expand Up @@ -251,7 +255,7 @@ export abstract class SimpleFindWidget extends Widget {
}

this._isVisible = true;
this.updateButtons(this._foundMatch);
this.updateResultCount();
this.layout();

setTimeout(() => {
Expand Down Expand Up @@ -354,15 +358,21 @@ export abstract class SimpleFindWidget extends Widget {

const count = await this._getResultCount();
this._matchesCount.innerText = '';
const showRedOutline = (this.inputValue.length > 0 && count?.resultCount === 0);
this._matchesCount.classList.toggle('no-results', showRedOutline);
let label = '';
this._matchesCount.classList.toggle('no-results', false);
if (count?.resultCount !== undefined && count?.resultCount === 0) {
label = NLS_NO_RESULTS;
if (!!this.inputValue) {
this._matchesCount.classList.toggle('no-results', true);
if (count?.resultCount) {
let matchesCount: string = String(count.resultCount);
if (count.resultCount >= this._matchesLimit) {
matchesCount += '+';
}
} else if (count?.resultCount) {
label = strings.format(NLS_MATCHES_LOCATION, count.resultIndex + 1, count?.resultCount);
let matchesPosition: string = String(count.resultIndex + 1);
if (matchesPosition === '0') {
matchesPosition = '?';
}
label = strings.format(NLS_MATCHES_LOCATION, matchesPosition, matchesCount);
} else {
label = NLS_NO_RESULTS;
}
alertFn(this._announceSearchResults(label, this.inputValue));
this._matchesCount.appendChild(document.createTextNode(label));
Expand Down
6 changes: 5 additions & 1 deletion src/vs/workbench/contrib/terminal/browser/terminal.ts
View file
Open in desktop
Expand Up @@ -955,6 +955,10 @@ export interface ITerminalChildElement {
xtermReady?(xterm: IXtermTerminal): void;
}

export const enum XtermTerminalConstants {
SearchHighlightLimit = 1000
}

export interface IXtermTerminal {
/**
* An object that tracks when commands are run and enables navigating and selecting between
Expand All @@ -968,7 +972,7 @@ export interface IXtermTerminal {
readonly shellIntegration: IShellIntegration;

readonly onDidChangeSelection: Event<void>;
readonly onDidChangeFindResults: Event<{ resultIndex: number; resultCount: number } | undefined>;
readonly onDidChangeFindResults: Event<{ resultIndex: number; resultCount: number }>;

/**
* Gets a view of the current texture atlas used by the renderers.
Expand Down
8 changes: 4 additions & 4 deletions src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@ import { IEditorOptions } from 'vs/editor/common/config/editorOptions';
import { IShellIntegration, TerminalSettingId } from 'vs/platform/terminal/common/terminal';
import { ITerminalFont } from 'vs/workbench/contrib/terminal/common/terminal';
import { isSafari } from 'vs/base/browser/browser';
import { IMarkTracker, IInternalXtermTerminal, IXtermTerminal, ISuggestController, IXtermColorProvider } from 'vs/workbench/contrib/terminal/browser/terminal';
import { IMarkTracker, IInternalXtermTerminal, IXtermTerminal, ISuggestController, IXtermColorProvider, XtermTerminalConstants } from 'vs/workbench/contrib/terminal/browser/terminal';
import { ILogService } from 'vs/platform/log/common/log';
import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
import { TerminalStorageKeys } from 'vs/workbench/contrib/terminal/common/terminalStorageKeys';
Expand Down Expand Up @@ -152,7 +152,7 @@ export class XtermTerminal extends DisposableStore implements IXtermTerminal, II
readonly onDidRequestSendText = this._onDidRequestSendText.event;
private readonly _onDidRequestFreePort = new Emitter<string>();
readonly onDidRequestFreePort = this._onDidRequestFreePort.event;
private readonly _onDidChangeFindResults = new Emitter<{ resultIndex: number; resultCount: number } | undefined>();
private readonly _onDidChangeFindResults = new Emitter<{ resultIndex: number; resultCount: number }>();
readonly onDidChangeFindResults = this._onDidChangeFindResults.event;
private readonly _onDidChangeSelection = new Emitter<void>();
readonly onDidChangeSelection = this._onDidChangeSelection.event;
Expand Down Expand Up @@ -409,9 +409,9 @@ export class XtermTerminal extends DisposableStore implements IXtermTerminal, II
return this._searchAddon;
}
const AddonCtor = await this._getSearchAddonConstructor();
this._searchAddon = new AddonCtor();
this._searchAddon = new AddonCtor({ highlightLimit: XtermTerminalConstants.SearchHighlightLimit });
this.raw.loadAddon(this._searchAddon);
this._searchAddon.onDidChangeResults((results: { resultIndex: number; resultCount: number } | undefined) => {
this._searchAddon.onDidChangeResults((results: { resultIndex: number; resultCount: number }) => {
this._lastFindResult = results;
this._onDidChangeFindResults.fire(results);
});
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget.ts
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@
import { SimpleFindWidget } from 'vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget';
import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
import { IContextKeyService, IContextKey } from 'vs/platform/contextkey/common/contextkey';
import { ITerminalInstance, IXtermTerminal } from 'vs/workbench/contrib/terminal/browser/terminal';
import { ITerminalInstance, IXtermTerminal, XtermTerminalConstants } from 'vs/workbench/contrib/terminal/browser/terminal';
import { TerminalContextKeys } from 'vs/workbench/contrib/terminal/common/terminalContextKey';
import { IThemeService } from 'vs/platform/theme/common/themeService';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
Expand All @@ -27,7 +27,7 @@ export class TerminalFindWidget extends SimpleFindWidget {
@IThemeService private readonly _themeService: IThemeService,
@IConfigurationService private readonly _configurationService: IConfigurationService
) {
super({ showCommonFindToggles: true, checkImeCompletionState: true, showResultCount: true, type: 'Terminal' }, _contextViewService, _contextKeyService, keybindingService);
super({ showCommonFindToggles: true, checkImeCompletionState: true, showResultCount: true, type: 'Terminal', matchesLimit: XtermTerminalConstants.SearchHighlightLimit }, _contextViewService, _contextKeyService, keybindingService);

this._register(this.state.onFindReplaceStateChange(() => {
this.show();
Expand Down