From 76781a1a1b86750d91ef716b6cfe78e0b653c09f Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Fri, 14 Apr 2023 10:48:51 -0700 Subject: [PATCH] Enable markup search hybrid mode (#177542) * Enable markup search hybrid mode * Experiment turning all cells into editing mode on replace * Prepare for minimal find markdown mode * Add settings for controlling the initial state for find in markdown cells * Validate initial state --- .vscode/settings.json | 4 +++ .../browser/contrib/find/findFilters.ts | 26 +++++++++++--- .../contrib/find/notebookFindReplaceWidget.ts | 7 ++-- .../contrib/find/notebookFindWidget.ts | 2 +- .../notebook/browser/notebook.contribution.ts | 19 ++++++++++ .../notebook/browser/notebookEditorWidget.ts | 36 ++++++++++--------- .../viewModel/notebookViewModelImpl.ts | 20 ++++++++++- .../contrib/notebook/common/notebookCommon.ts | 1 + 8 files changed, 90 insertions(+), 25 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 52ab4c096fc..5cc96fa5af2 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -126,4 +126,8 @@ "application.experimental.rendererProfiling": true, "editor.experimental.asyncTokenization": true, "editor.experimental.asyncTokenizationVerification": true, + "notebook.experimental.findInMarkdownMode": { + "source": true, + "preview": true + } } diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/find/findFilters.ts b/src/vs/workbench/contrib/notebook/browser/contrib/find/findFilters.ts index f14ee0b70e2..a4bf2fd2d9f 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/find/findFilters.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/find/findFilters.ts @@ -26,8 +26,7 @@ export class NotebookFindFilters extends Disposable { set markupInput(value: boolean) { if (this._markupInput !== value) { this._markupInput = value; - this._markupPreview = !value; - this._onDidChange.fire({ markupInput: value, markupPreview: this._markupPreview }); + this._onDidChange.fire({ markupInput: value }); } } @@ -40,8 +39,7 @@ export class NotebookFindFilters extends Disposable { set markupPreview(value: boolean) { if (this._markupPreview !== value) { this._markupPreview = value; - this._markupInput = !value; - this._onDidChange.fire({ markupPreview: value, markupInput: this._markupInput }); + this._onDidChange.fire({ markupPreview: value }); } } private _codeInput: boolean = true; @@ -70,6 +68,12 @@ export class NotebookFindFilters extends Disposable { } } + private readonly _initialMarkupInput: boolean; + private readonly _initialMarkupPreview: boolean; + private readonly _initialCodeInput: boolean; + private readonly _initialCodeOutput: boolean; + + constructor( markupInput: boolean, markupPreview: boolean, @@ -82,6 +86,20 @@ export class NotebookFindFilters extends Disposable { this._markupPreview = markupPreview; this._codeInput = codeInput; this._codeOutput = codeOutput; + + this._initialMarkupInput = markupInput; + this._initialMarkupPreview = markupPreview; + this._initialCodeInput = codeInput; + this._initialCodeOutput = codeOutput; + } + + isModified(): boolean { + return ( + this._markupInput !== this._initialMarkupInput + || this._markupPreview !== this._initialMarkupPreview + || this._codeInput !== this._initialCodeInput + || this._codeOutput !== this._initialCodeOutput + ); } update(v: NotebookFindFilters) { diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts b/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts index 53a2ab5e921..11c26d81657 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts @@ -39,6 +39,7 @@ import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookB import { defaultInputBoxStyles, defaultProgressBarStyles, defaultToggleStyles } from 'vs/platform/theme/browser/defaultStyles'; import { IToggleStyles } from 'vs/base/browser/ui/toggle/toggle'; import { Disposable } from 'vs/base/common/lifecycle'; +import { NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon'; const NLS_FIND_INPUT_LABEL = nls.localize('label.find', "Find"); const NLS_FIND_INPUT_PLACEHOLDER = nls.localize('placeholder.find', "Find"); @@ -57,7 +58,7 @@ const NOTEBOOK_FIND_FILTERS = nls.localize('notebook.find.filter.filterAction', const NOTEBOOK_FIND_IN_MARKUP_INPUT = nls.localize('notebook.find.filter.findInMarkupInput', "Markdown Source"); const NOTEBOOK_FIND_IN_MARKUP_PREVIEW = nls.localize('notebook.find.filter.findInMarkupPreview', "Rendered Markdown"); const NOTEBOOK_FIND_IN_CODE_INPUT = nls.localize('notebook.find.filter.findInCodeInput', "Code Cell Source"); -const NOTEBOOK_FIND_IN_CODE_OUTPUT = nls.localize('notebook.find.filter.findInCodeOutput', "Cell Output"); +const NOTEBOOK_FIND_IN_CODE_OUTPUT = nls.localize('notebook.find.filter.findInCodeOutput', "Code Cell Output"); const NOTEBOOK_FIND_WIDGET_INITIAL_WIDTH = 318; const NOTEBOOK_FIND_WIDGET_INITIAL_HORIZONTAL_PADDING = 4; @@ -305,7 +306,9 @@ export abstract class SimpleFindReplaceWidget extends Widget { ) { super(); - this._filters = new NotebookFindFilters(true, false, true, true); + const findInMarkdownMode = this._configurationService.getValue<{ source: boolean; preview: boolean }>(NotebookSetting.experimentalFindInMarkdownMode) ?? { source: true, preview: false }; + + this._filters = new NotebookFindFilters(findInMarkdownMode.source, findInMarkdownMode.preview, true, true); this._state.change({ filters: this._filters }, false); this._filters.onDidChange(() => { diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindWidget.ts b/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindWidget.ts index 2d9f2a320f7..d7935e25c83 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindWidget.ts @@ -112,7 +112,7 @@ class NotebookFindWidget extends SimpleFindReplaceWidget implements INotebookEdi this._replaceAllBtn.setEnabled(matches.length > 0 && matches.find(match => match.webviewMatches.length > 0) === undefined); if (e.filters) { - this._findInput.updateFilterState((this._state.filters?.markupPreview ?? false) || (this._state.filters?.codeOutput !== true)); + this._findInput.updateFilterState(this._state.filters?.isModified() ?? false); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts index b833b14c51a..3924ec0c06e 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -911,6 +911,25 @@ configurationRegistry.registerConfiguration({ type: 'boolean', tags: ['notebookLayout'], default: false + }, + [NotebookSetting.experimentalFindInMarkdownMode]: { + markdownDescription: nls.localize('notebook.experimental.findInMarkdownMode', "Customize the Find Widget behavior for searching within markdown cells. When both are enabled, the Find Widget will search either the content or preview based on the current state of the markdown cell. Toggle the boolean values to control the Find Widget's scope for each mode as needed."), + type: 'object', + properties: { + source: { + type: 'boolean', + default: true + }, + preview: { + type: 'boolean', + default: false + } + }, + default: { + source: true, + preview: false + }, + tags: ['notebookLayout'] } } }); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 43f79edc467..c7cee1aa962 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -2440,24 +2440,14 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD if (!options.includeMarkupPreview && !options.includeOutput) { this._webview?.findStop(); - - return findMatches.filter(match => - (match.cell.cellKind === CellKind.Code && options.includeCodeInput) || - (match.cell.cellKind === CellKind.Markup && options.includeMarkupInput) - ); + return findMatches; } // search in webview enabled const matchMap: { [key: string]: CellFindMatchWithIndex } = {}; findMatches.forEach(match => { - if (match.cell.cellKind === CellKind.Code && options.includeCodeInput) { - matchMap[match.cell.id] = match; - } - - if (match.cell.cellKind === CellKind.Markup && options.includeMarkupInput) { - matchMap[match.cell.id] = match; - } + matchMap[match.cell.id] = match; }); if (this._webview) { @@ -2480,14 +2470,26 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD // attach webview matches to model find matches webviewMatches.forEach(match => { - if (!options.includeMarkupPreview && match.type === 'preview') { - // skip outputs if not included + const cell = this._notebookViewModel!.viewCells.find(cell => cell.id === match.cellId); + + if (!cell) { return; } - if (!options.includeOutput && match.type === 'output') { - // skip outputs if not included - return; + if (match.type === 'preview') { + // markup preview + if (cell.getEditState() === CellEditState.Preview && !options.includeMarkupPreview) { + return; + } + + if (cell.getEditState() === CellEditState.Editing && options.includeMarkupInput) { + return; + } + } else { + if (!options.includeOutput) { + // skip outputs if not included + return; + } } const exisitingMatch = matchMap[match.cellId]; diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts index 653d624c6a1..9f2edd91ffa 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts @@ -922,7 +922,25 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD } }); - return matches; + // filter based on options and editing state + + return matches.filter(match => { + if (match.cell.cellKind === CellKind.Code) { + // code cell, we only include its match if include input is enabled + return options.includeCodeInput; + } + + // markup cell, it depends on the editing state + if (match.cell.getEditState() === CellEditState.Editing) { + // editing, even if we includeMarkupPreview + return options.includeMarkupInput; + } else { + // cell in preview mode, we should only include it if includeMarkupPreview is false but includeMarkupInput is true + // if includeMarkupPreview is true, then we should include the webview match result other than this + return !options.includeMarkupPreview && options.includeMarkupInput; + } + } + ); } replaceOne(cell: ICellViewModel, range: Range, text: string): Promise { diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 16e72077c89..c5983320fd3 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -944,6 +944,7 @@ export const NotebookSetting = { outputFontSize: 'notebook.output.fontSize', outputFontFamilyDeprecated: 'notebook.outputFontFamily', outputFontFamily: 'notebook.output.fontFamily', + experimentalFindInMarkdownMode: 'notebook.experimental.findInMarkdownMode', logging: 'notebook.logging', } as const;