Skip to content

Commit fba29a9

Browse files
andreamahandreamah
authored
remove experimental flag for notebook search for open notebooks (microsoft#182669)
Fixes microsoft#182565
1 parent c982e24 commit fba29a9

File tree

9 files changed

+36
-75
lines changed

9 files changed

+36
-75
lines changed

‎src/vs/workbench/contrib/search/browser/search.contribution.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import 'vs/workbench/contrib/search/browser/searchActionsNav';
4141
import 'vs/workbench/contrib/search/browser/searchActionsRemoveReplace';
4242
import 'vs/workbench/contrib/search/browser/searchActionsSymbol';
4343
import 'vs/workbench/contrib/search/browser/searchActionsTopBar';
44-
import product from 'vs/platform/product/common/product';
4544

4645
registerSingleton(ISearchWorkbenchService, SearchWorkbenchService, InstantiationType.Delayed);
4746
registerSingleton(ISearchHistoryService, SearchHistoryService, InstantiationType.Delayed);
@@ -350,12 +349,7 @@ configurationRegistry.registerConfiguration({
350349
nls.localize('scm.defaultViewMode.list', "Shows search results as a list.")
351350
],
352351
'description': nls.localize('search.defaultViewMode', "Controls the default search result view mode.")
353-
},
354-
'search.experimental.notebookSearch': {
355-
type: 'boolean',
356-
description: nls.localize('search.experimental.notebookSearch', "Controls whether to use the experimental notebook search in the global search. Please reload your VS Code instance for changes to this setting to take effect."),
357-
default: typeof product.quality === 'string' && product.quality !== 'stable', // only enable as default in insiders
358-
},
352+
}
359353
}
360354
});
361355

‎src/vs/workbench/contrib/search/browser/searchModel.ts

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,6 @@ export class FileMatch extends Disposable implements IFileMatch {
393393
@IReplaceService private readonly replaceService: IReplaceService,
394394
@ILabelService readonly labelService: ILabelService,
395395
@INotebookEditorService private readonly notebookEditorService: INotebookEditorService,
396-
@IConfigurationService private readonly configurationService: IConfigurationService,
397396
) {
398397
super();
399398
this._resource = this.rawMatch.resource;
@@ -445,8 +444,7 @@ export class FileMatch extends Disposable implements IFileMatch {
445444
this.bindModel(model);
446445
this.updateMatchesForModel();
447446
} else {
448-
const experimentalNotebooksEnabled = this.configurationService.getValue<ISearchConfigurationProperties>('search').experimental.notebookSearch;
449-
const notebookEditorWidgetBorrow = experimentalNotebooksEnabled ? this.notebookEditorService.retrieveExistingWidgetFromURI(this.resource) : undefined;
447+
const notebookEditorWidgetBorrow = this.notebookEditorService.retrieveExistingWidgetFromURI(this.resource);
450448

451449
if (notebookEditorWidgetBorrow?.value) {
452450
this.bindNotebookEditorWidget(notebookEditorWidgetBorrow.value);
@@ -1542,21 +1540,17 @@ export class SearchResult extends Disposable {
15421540
@IModelService private readonly modelService: IModelService,
15431541
@IUriIdentityService private readonly uriIdentityService: IUriIdentityService,
15441542
@INotebookEditorService private readonly notebookEditorService: INotebookEditorService,
1545-
@IConfigurationService private readonly configurationService: IConfigurationService,
15461543
) {
15471544
super();
15481545
this._rangeHighlightDecorations = this.instantiationService.createInstance(RangeHighlightDecorations);
15491546

15501547
this._register(this.modelService.onModelAdded(model => this.onModelAdded(model)));
15511548

1552-
const experimentalNotebooksEnabled = this.configurationService.getValue<ISearchConfigurationProperties>('search').experimental.notebookSearch;
1553-
if (experimentalNotebooksEnabled) {
1554-
this._register(this.notebookEditorService.onDidAddNotebookEditor(widget => {
1555-
if (widget instanceof NotebookEditorWidget) {
1556-
this.onDidAddNotebookEditorWidget(<NotebookEditorWidget>widget);
1557-
}
1558-
}));
1559-
}
1549+
this._register(this.notebookEditorService.onDidAddNotebookEditor(widget => {
1550+
if (widget instanceof NotebookEditorWidget) {
1551+
this.onDidAddNotebookEditorWidget(<NotebookEditorWidget>widget);
1552+
}
1553+
}));
15601554

15611555
this._register(this.onChange(e => {
15621556
if (e.removed) {
@@ -1662,11 +1656,6 @@ export class SearchResult extends Disposable {
16621656
}
16631657

16641658
private onDidAddNotebookEditorWidget(widget: NotebookEditorWidget): void {
1665-
const experimentalNotebooksEnabled = this.configurationService.getValue<ISearchConfigurationProperties>('search').experimental.notebookSearch;
1666-
1667-
if (!experimentalNotebooksEnabled) {
1668-
return;
1669-
}
16701659

16711660
this._onWillChangeModelListener?.dispose();
16721661
this._onWillChangeModelListener = widget.onWillChangeModel(
@@ -2049,9 +2038,7 @@ export class SearchModel extends Disposable {
20492038

20502039
onProgress?.(p);
20512040
};
2052-
const experimentalNotebooksEnabled = this.configurationService.getValue<ISearchConfigurationProperties>('search').experimental.notebookSearch;
2053-
2054-
const notebookResult = experimentalNotebooksEnabled ? await this.notebookSearch(query, this.currentCancelTokenSource.token, onProgressCall) : undefined;
2041+
const notebookResult = await this.notebookSearch(query, this.currentCancelTokenSource.token, onProgressCall);
20552042
const currentResult = await this.searchService.textSearch(
20562043
searchQuery,
20572044
this.currentCancelTokenSource.token, onProgressCall,

‎src/vs/workbench/contrib/search/browser/searchView.ts

Lines changed: 21 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ import { IPatternInfo, ISearchComplete, ISearchConfiguration, ISearchConfigurati
8080
import { TextSearchCompleteMessage } from 'vs/workbench/services/search/common/searchExtTypes';
8181
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
8282
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
83-
import { NotebookFindContrib } from 'vs/workbench/contrib/notebook/browser/contrib/find/notebookFindWidget';
8483
import { ILogService } from 'vs/platform/log/common/log';
8584

8685
const $ = dom.$;
@@ -851,7 +850,6 @@ export class SearchView extends ViewPane {
851850
this.lastFocusState = 'tree';
852851
}
853852

854-
// we don't need to check experimental flag here because NotebookMatches only exist when the flag is enabled
855853
let editable = false;
856854
if (focus instanceof MatchInNotebook) {
857855
editable = !focus.isWebviewMatch();
@@ -1818,8 +1816,6 @@ export class SearchView extends ViewPane {
18181816
private shouldOpenInNotebookEditor(match: Match, uri: URI): boolean {
18191817
// Untitled files will return a false positive for getContributedNotebookTypes.
18201818
// Since untitled files are already open, then untitled notebooks should return NotebookMatch results.
1821-
1822-
// notebookMatch are only created when search.experimental.notebookSearch is enabled, so this should never return true if experimental flag is disabled.
18231819
return match instanceof MatchInNotebook || (uri.scheme !== network.Schemas.untitled && this.notebookService.getContributedNotebookTypes(uri).length > 0);
18241820
}
18251821

@@ -1864,41 +1860,32 @@ export class SearchView extends ViewPane {
18641860

18651861
if (editor instanceof NotebookEditor) {
18661862
const elemParent = element.parent() as FileMatch;
1867-
const experimentalNotebooksEnabled = this.configurationService.getValue<ISearchConfigurationProperties>('search').experimental.notebookSearch;
1868-
if (experimentalNotebooksEnabled) {
1869-
if (element instanceof Match) {
1870-
if (element instanceof MatchInNotebook) {
1871-
element.parent().showMatch(element);
1872-
} else {
1873-
const editorWidget = editor.getControl();
1874-
if (editorWidget) {
1875-
// Ensure that the editor widget is binded. If if is, then this should return immediately.
1876-
// Otherwise, it will bind the widget.
1877-
elemParent.bindNotebookEditorWidget(editorWidget);
1878-
await elemParent.updateMatchesForEditorWidget();
1879-
1880-
const matchIndex = oldParentMatches.findIndex(e => e.id() === element.id());
1881-
const matches = element.parent().matches();
1882-
const match = matchIndex >= matches.length ? matches[matches.length - 1] : matches[matchIndex];
1883-
1884-
if (match instanceof MatchInNotebook) {
1885-
elemParent.showMatch(match);
1886-
}
1887-
1888-
if (!this.tree.getFocus().includes(match) || !this.tree.getSelection().includes(match)) {
1889-
this.tree.setSelection([match], getSelectionKeyboardEvent());
1890-
this.tree.setFocus([match]);
1891-
}
1863+
if (element instanceof Match) {
1864+
if (element instanceof MatchInNotebook) {
1865+
element.parent().showMatch(element);
1866+
} else {
1867+
const editorWidget = editor.getControl();
1868+
if (editorWidget) {
1869+
// Ensure that the editor widget is binded. If if is, then this should return immediately.
1870+
// Otherwise, it will bind the widget.
1871+
elemParent.bindNotebookEditorWidget(editorWidget);
1872+
await elemParent.updateMatchesForEditorWidget();
1873+
1874+
const matchIndex = oldParentMatches.findIndex(e => e.id() === element.id());
1875+
const matches = element.parent().matches();
1876+
const match = matchIndex >= matches.length ? matches[matches.length - 1] : matches[matchIndex];
1877+
1878+
if (match instanceof MatchInNotebook) {
1879+
elemParent.showMatch(match);
18921880
}
18931881

1882+
if (!this.tree.getFocus().includes(match) || !this.tree.getSelection().includes(match)) {
1883+
this.tree.setSelection([match], getSelectionKeyboardEvent());
1884+
this.tree.setFocus([match]);
1885+
}
18941886
}
18951887
}
1896-
} else {
1897-
const controller = editor.getControl()?.getContribution<NotebookFindContrib>(NotebookFindContrib.id);
1898-
const matchIndex = element instanceof Match ? element.parent().matches().findIndex(e => e.id() === element.id()) : undefined;
1899-
controller?.show(this.searchWidget.searchInput?.getValue(), { matchIndex, focus: false });
19001888
}
1901-
19021889
}
19031890
}
19041891

‎src/vs/workbench/contrib/search/browser/searchWidget.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
2525
import { KeybindingsRegistry, KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
2626
import { ISearchConfigurationProperties } from 'vs/workbench/services/search/common/search';
2727
import { ThemeIcon } from 'vs/base/common/themables';
28-
import { ContextScopedFindInput, ContextScopedReplaceInput } from 'vs/platform/history/browser/contextScopedHistoryWidget';
28+
import { ContextScopedReplaceInput } from 'vs/platform/history/browser/contextScopedHistoryWidget';
2929
import { appendKeyBindingLabel, isSearchViewFocused, getSearchView } from 'vs/workbench/contrib/search/browser/searchActionsBase';
3030
import * as Constants from 'vs/workbench/contrib/search/common/constants';
3131
import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility';
@@ -385,12 +385,8 @@ export class SearchWidget extends Widget {
385385

386386
const searchInputContainer = dom.append(parent, dom.$('.search-container.input-box'));
387387

388-
const experimentalNotebooksEnabled = this.configurationService.getValue<ISearchConfigurationProperties>('search').experimental.notebookSearch;
389-
if (experimentalNotebooksEnabled) {
390-
this.searchInput = this._register(new SearchFindInput(searchInputContainer, this.contextViewService, inputOptions, this.contextKeyService, this.contextMenuService, this.instantiationService, this._notebookFilters, this._hasNotebookOpen()));
391-
} else {
392-
this.searchInput = this._register(new ContextScopedFindInput(searchInputContainer, this.contextViewService, inputOptions, this.contextKeyService));
393-
}
388+
this.searchInput = this._register(new SearchFindInput(searchInputContainer, this.contextViewService, inputOptions, this.contextKeyService, this.contextMenuService, this.instantiationService, this._notebookFilters, this._hasNotebookOpen()));
389+
394390
this.searchInput.onKeyDown((keyboardEvent: IKeyboardEvent) => this.onSearchInputKeyDown(keyboardEvent));
395391
this.searchInput.setValue(options.value || '');
396392
this.searchInput.setRegex(!!options.isRegex);

‎src/vs/workbench/contrib/search/test/browser/searchModel.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ suite('SearchModel', () => {
500500
function stubModelService(instantiationService: TestInstantiationService): IModelService {
501501
instantiationService.stub(IThemeService, new TestThemeService());
502502
const config = new TestConfigurationService();
503-
config.setUserConfiguration('search', { searchOnType: true, experimental: { notebookSearch: true } });
503+
config.setUserConfiguration('search', { searchOnType: true });
504504
instantiationService.stub(IConfigurationService, config);
505505
return instantiationService.createInstance(ModelService);
506506
}

‎src/vs/workbench/contrib/search/test/browser/searchResult.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ suite('SearchResult', () => {
543543
function stubModelService(instantiationService: TestInstantiationService): IModelService {
544544
instantiationService.stub(IThemeService, new TestThemeService());
545545
const config = new TestConfigurationService();
546-
config.setUserConfiguration('search', { searchOnType: true, experimental: { notebookSearch: false } });
546+
config.setUserConfiguration('search', { searchOnType: true });
547547
instantiationService.stub(IConfigurationService, config);
548548
return instantiationService.createInstance(ModelService);
549549
}

‎src/vs/workbench/contrib/search/test/browser/searchTestCommon.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export function getRootName(): string {
4343
export function stubModelService(instantiationService: TestInstantiationService): IModelService {
4444
instantiationService.stub(IThemeService, new TestThemeService());
4545
const config = new TestConfigurationService();
46-
config.setUserConfiguration('search', { searchOnType: true, experimental: { notebookSearch: false } });
46+
config.setUserConfiguration('search', { searchOnType: true });
4747
instantiationService.stub(IConfigurationService, config);
4848
return instantiationService.createInstance(ModelService);
4949
}

‎src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ suite('Search - Viewlet', () => {
203203
instantiationService.stub(IThemeService, new TestThemeService());
204204

205205
const config = new TestConfigurationService();
206-
config.setUserConfiguration('search', { searchOnType: true, experimental: { notebookSearch: false } });
206+
config.setUserConfiguration('search', { searchOnType: true });
207207
instantiationService.stub(IConfigurationService, config);
208208

209209
return instantiationService.createInstance(ModelService);

‎src/vs/workbench/services/search/common/search.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,6 @@ export interface ISearchConfigurationProperties {
409409
badges: boolean;
410410
};
411411
defaultViewMode: ViewMode;
412-
experimental: {
413-
notebookSearch: boolean;
414-
};
415412
}
416413

417414
export interface ISearchConfiguration extends IFilesConfiguration {

0 commit comments

Comments
 (0)