From 74bc18638f2adbf630c566422714450639735ed0 Mon Sep 17 00:00:00 2001 From: Michael Lively Date: Tue, 19 Mar 2024 12:00:04 -0700 Subject: [PATCH 1/4] add symbol entries as children of code cells in nb outline --- .../contrib/outline/notebookOutline.ts | 48 ++++++++++++++++--- .../viewModel/notebookOutlineEntryFactory.ts | 21 ++++---- .../viewModel/notebookOutlineProvider.ts | 7 +-- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts index 17690008403d9..d4b0c1aaedf56 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts @@ -106,14 +106,16 @@ class NotebookOutlineRenderer implements ITreeRenderer= 8) { // symbol + template.iconClass.className = 'element-icon ' + ThemeIcon.asClassNameArray(node.element.icon).join(' '); + } else if (isCodeCell && this._themeService.getFileIconTheme().hasFileIcons && !node.element.isExecuting) { template.iconClass.className = ''; extraClasses.push(...getIconClassesForLanguageId(node.element.cell.language ?? '')); } else { template.iconClass.className = 'element-icon ' + ThemeIcon.asClassNameArray(node.element.icon).join(' '); } - template.iconLabel.setLabel(node.element.label, undefined, options); + template.iconLabel.setLabel(' ' + node.element.label, undefined, options); const { markerInfo } = node.element; @@ -524,10 +526,14 @@ export class NotebookOutlineCreator implements IOutlineCreator | undefined> { const outline = this._instantiationService.createInstance(NotebookCellOutline, editor, target); - const showAllSymbols = this._configurationService.getValue(NotebookSetting.gotoSymbolsAllSymbols); - if (target === OutlineTarget.QuickPick && showAllSymbols) { + const showAllGotoSymbols = this._configurationService.getValue(NotebookSetting.gotoSymbolsAllSymbols); + const showAllOutlineSymbols = this._configurationService.getValue('notebook.outline.showCodeCellSymbols'); + if (target === OutlineTarget.QuickPick && showAllGotoSymbols) { + await outline.setFullSymbols(cancelToken); + } else if (target === OutlineTarget.OutlinePane && showAllOutlineSymbols) { await outline.setFullSymbols(cancelToken); } + return outline; } } @@ -550,7 +556,7 @@ Registry.as(ConfigurationExtensions.Configuration).regis 'notebook.outline.showCodeCells': { type: 'boolean', default: false, - markdownDescription: localize('outline.showCodeCells', "When enabled notebook outline shows code cells.") + markdownDescription: localize('outline.showCodeCells', "When enabled, notebook outline shows code cells.") }, 'notebook.outline.showNonHeaderMarkdownCells': { type: 'boolean', @@ -560,12 +566,17 @@ Registry.as(ConfigurationExtensions.Configuration).regis 'notebook.breadcrumbs.showCodeCells': { type: 'boolean', default: true, - markdownDescription: localize('breadcrumbs.showCodeCells', "When enabled notebook breadcrumbs contain code cells.") + markdownDescription: localize('breadcrumbs.showCodeCells', "When enabled, notebook breadcrumbs contain code cells.") + }, + 'notebook.outline.showCodeCellSymbols': { + type: 'boolean', + default: true, + markdownDescription: localize('outline.showCodeCellSymbols', "When enabled, notebook outline shows code cell symbols. Relies on `notebook.outline.showCodeCells` being enabled.") }, [NotebookSetting.gotoSymbolsAllSymbols]: { type: 'boolean', default: true, - markdownDescription: localize('notebook.gotoSymbols.showAllSymbols', "When enabled the Go to Symbol Quick Pick will display full code symbols from the notebook, as well as Markdown headers.") + markdownDescription: localize('notebook.gotoSymbols.showAllSymbols', "When enabled, the Go to Symbol Quick Pick will display full code symbols from the notebook, as well as Markdown headers.") }, } }); @@ -624,3 +635,26 @@ registerAction2(class ToggleNonHeaderMarkdownCells extends Action2 { configurationService.updateValue('notebook.outline.showNonHeaderMarkdownCells', !showNonHeaderMarkdownCells); } }); + +registerAction2(class ToggleCodeCellSymbolEntries extends Action2 { + constructor() { + super({ + id: 'notebook.outline.toggleCodeCellSymbolEntries', + title: localize('toggleCodeCellSymbolEntries', "Toggle Code Cell Symbol Entries"), + f1: false, + toggled: { + condition: ContextKeyExpr.equals('config.notebook.outline.showCodeCellSymbols', true) + }, + menu: { + id: MenuId.NotebookOutlineFilter, + group: 'filter', + } + }); + } + + run(accessor: ServicesAccessor, ...args: any[]) { + const configurationService = accessor.get(IConfigurationService); + const showCodeCellSymbols = configurationService.getValue('notebook.outline.showCodeCellSymbols'); + configurationService.updateValue('notebook.outline.showCodeCellSymbols', !showCodeCellSymbols); + } +}); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts index 54335576ac92e..4a0cc1e862be7 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts @@ -65,6 +65,14 @@ export class NotebookOutlineEntryFactory { } if (!hasHeader) { + const exeState = !isMarkdown && this.executionStateService.getCellExecution(cell.uri); + let preview = content.trim(); + if (preview.length === 0) { + // empty or just whitespace + preview = localize('empty', "empty cell"); + } + entries.push(new OutlineEntry(index++, 7, cell, preview, !!exeState, exeState ? exeState.isPaused : false)); + if (!isMarkdown && cell.model.textModel) { const cachedEntries = this.cellOutlineEntryCache[cell.model.textModel.id]; @@ -76,17 +84,6 @@ export class NotebookOutlineEntryFactory { }); } } - - const exeState = !isMarkdown && this.executionStateService.getCellExecution(cell.uri); - if (entries.length === 0) { - let preview = content.trim(); - if (preview.length === 0) { - // empty or just whitespace - preview = localize('empty', "empty cell"); - } - - entries.push(new OutlineEntry(index++, 7, cell, preview, !!exeState, exeState ? exeState.isPaused : false)); - } } return entries; @@ -95,7 +92,7 @@ export class NotebookOutlineEntryFactory { public async cacheSymbols(cell: ICellViewModel, outlineModelService: IOutlineModelService, cancelToken: CancellationToken) { const textModel = await cell.resolveTextModel(); const outlineModel = await outlineModelService.getOrCreate(textModel, cancelToken); - const entries = createOutlineEntries(outlineModel.getTopLevelSymbols(), 7); + const entries = createOutlineEntries(outlineModel.getTopLevelSymbols(), 8); this.cellOutlineEntryCache[textModel.id] = entries; } } diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts index c57adfbdbdf5c..67c4cccd83740 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts @@ -70,7 +70,10 @@ export class NotebookCellOutlineProvider { ); this._dispoables.add(_configurationService.onDidChangeConfiguration(e => { - if (e.affectsConfiguration('notebook.outline.showCodeCells') || e.affectsConfiguration('notebook.outline.showNonHeaderMarkdownCells')) { + if (e.affectsConfiguration('notebook.outline.showCodeCells') || + e.affectsConfiguration('notebook.outline.showNonHeaderMarkdownCells') || + e.affectsConfiguration('notebook.outline.showCodeCellSymbols') + ) { this._recomputeState(); } })); @@ -269,8 +272,6 @@ export class NotebookCellOutlineProvider { } } - - get isEmpty(): boolean { return this._entries.length === 0; } From 717608fbdf7402073b3975916624dbf4da44e762 Mon Sep 17 00:00:00 2001 From: Michael Lively Date: Thu, 21 Mar 2024 11:01:00 -0700 Subject: [PATCH 2/4] extract filtering logic to getChildren for IOutline config, adjust level checks in nb SS --- .../contrib/outline/notebookOutline.ts | 77 ++++++++++++------- .../viewModel/notebookOutlineProvider.ts | 27 +++---- .../viewParts/notebookEditorStickyScroll.ts | 10 +-- 3 files changed, 68 insertions(+), 46 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts index d4b0c1aaedf56..d965598f9ca79 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts @@ -379,7 +379,11 @@ export class NotebookCellOutline implements IOutline { })); installSelectionListener(); - const treeDataSource: IDataSource = { getChildren: parent => parent instanceof NotebookCellOutline ? (this._outlineProvider?.entries ?? []) : parent.children }; + const treeDataSource: IDataSource = { + getChildren: parent => { + return this.getChildren(parent, _configurationService); + } + }; const delegate = new NotebookOutlineVirtualDelegate(); const renderers = [instantiationService.createInstance(NotebookOutlineRenderer, this._editor.getControl(), _target)]; const comparator = new NotebookComparator(); @@ -414,6 +418,24 @@ export class NotebookCellOutline implements IOutline { }; } + *getChildren(parent: OutlineEntry | NotebookCellOutline, configurationService: IConfigurationService): Iterable { + for (const entry of parent instanceof NotebookCellOutline ? (this._outlineProvider?.entries ?? []) : parent.children) { + const showOutlineCodeCells = configurationService.getValue('notebook.outline.showCodeCells'); + const showMarkdownHeadersOnly = configurationService.getValue('notebook.outline.showMarkdownHeadersOnly'); + + if (entry.cell.cellKind === CellKind.Markup) { + if (!showMarkdownHeadersOnly) { + yield entry; + } else if (entry.level < 7) { + yield entry; + } + } + if (showOutlineCodeCells && entry.cell.cellKind === CellKind.Code) { + yield entry; + } + } + } + async setFullSymbols(cancelToken: CancellationToken) { await this._outlineProvider?.setFullSymbols(cancelToken); } @@ -553,26 +575,26 @@ Registry.as(ConfigurationExtensions.Configuration).regis order: 100, type: 'object', 'properties': { + 'notebook.outline.showMarkdownHeadersOnly': { + type: 'boolean', + default: true, + markdownDescription: localize('outline.showMarkdownHeadersOnly', "When enabled, notebook outline will show only markdown cells containing a header.") + }, 'notebook.outline.showCodeCells': { type: 'boolean', default: false, markdownDescription: localize('outline.showCodeCells', "When enabled, notebook outline shows code cells.") }, - 'notebook.outline.showNonHeaderMarkdownCells': { + 'notebook.outline.showCodeCellSymbols': { type: 'boolean', - default: false, - markdownDescription: localize('outline.showNonHeaderMarkdownCells', "When enabled, notebook outline will show non-header markdown cell entries. When disabled, only markdown cells containing a header are shown.") + default: true, + markdownDescription: localize('outline.showCodeCellSymbols', "When enabled, notebook outline shows code cell symbols. Relies on `notebook.outline.showCodeCells` being enabled.") }, 'notebook.breadcrumbs.showCodeCells': { type: 'boolean', default: true, markdownDescription: localize('breadcrumbs.showCodeCells', "When enabled, notebook breadcrumbs contain code cells.") }, - 'notebook.outline.showCodeCellSymbols': { - type: 'boolean', - default: true, - markdownDescription: localize('outline.showCodeCellSymbols', "When enabled, notebook outline shows code cell symbols. Relies on `notebook.outline.showCodeCells` being enabled.") - }, [NotebookSetting.gotoSymbolsAllSymbols]: { type: 'boolean', default: true, @@ -590,64 +612,67 @@ MenuRegistry.appendMenuItem(MenuId.ViewTitle, { when: ContextKeyExpr.and(ContextKeyExpr.equals('view', IOutlinePane.Id), NOTEBOOK_IS_ACTIVE_EDITOR), }); -registerAction2(class ToggleCodeCellEntries extends Action2 { +registerAction2(class ToggleShowMarkdownHeadersOnly extends Action2 { constructor() { super({ - id: 'notebook.outline.toggleCodeCells', - title: localize('toggleCodeCells', "Toggle Code Cells"), + id: 'notebook.outline.toggleShowMarkdownHeadersOnly', + title: localize('toggleShowMarkdownHeadersOnly', "Markdown Headers Only"), f1: false, toggled: { - condition: ContextKeyExpr.equals('config.notebook.outline.showCodeCells', true) + condition: ContextKeyExpr.equals('config.notebook.outline.showMarkdownHeadersOnly', true) }, menu: { id: MenuId.NotebookOutlineFilter, - group: 'filter', + group: '0_markdown_cells', } }); } run(accessor: ServicesAccessor, ...args: any[]) { const configurationService = accessor.get(IConfigurationService); - const showCodeCells = configurationService.getValue('notebook.outline.showCodeCells'); - configurationService.updateValue('notebook.outline.showCodeCells', !showCodeCells); + const showMarkdownHeadersOnly = configurationService.getValue('notebook.outline.showMarkdownHeadersOnly'); + configurationService.updateValue('notebook.outline.showMarkdownHeadersOnly', !showMarkdownHeadersOnly); } }); -registerAction2(class ToggleNonHeaderMarkdownCells extends Action2 { + +registerAction2(class ToggleCodeCellEntries extends Action2 { constructor() { super({ - id: 'notebook.outline.toggleNonHeaderMarkdownCells', - title: localize('toggleNonHeaderMarkdownCells', "Toggle Non-Header Markdown Cells"), + id: 'notebook.outline.toggleCodeCells', + title: localize('toggleCodeCells', "Code Cells"), f1: false, toggled: { - condition: ContextKeyExpr.equals('config.notebook.outline.showNonHeaderMarkdownCells', true) + condition: ContextKeyExpr.equals('config.notebook.outline.showCodeCells', true) }, menu: { id: MenuId.NotebookOutlineFilter, - group: 'filter', + order: 1, + group: '1_code_cells', } }); } run(accessor: ServicesAccessor, ...args: any[]) { const configurationService = accessor.get(IConfigurationService); - const showNonHeaderMarkdownCells = configurationService.getValue('notebook.outline.showNonHeaderMarkdownCells'); - configurationService.updateValue('notebook.outline.showNonHeaderMarkdownCells', !showNonHeaderMarkdownCells); + const showCodeCells = configurationService.getValue('notebook.outline.showCodeCells'); + configurationService.updateValue('notebook.outline.showCodeCells', !showCodeCells); } }); registerAction2(class ToggleCodeCellSymbolEntries extends Action2 { constructor() { super({ - id: 'notebook.outline.toggleCodeCellSymbolEntries', - title: localize('toggleCodeCellSymbolEntries', "Toggle Code Cell Symbol Entries"), + id: 'notebook.outline.toggleCodeCellSymbols', + title: localize('toggleCodeCellSymbols', "Code Cell Symbols"), f1: false, toggled: { condition: ContextKeyExpr.equals('config.notebook.outline.showCodeCellSymbols', true) }, menu: { id: MenuId.NotebookOutlineFilter, - group: 'filter', + order: 2, + group: '1_code_cells', } }); } diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts index 67c4cccd83740..3978a4d06ecab 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts @@ -10,7 +10,7 @@ import { URI } from 'vs/base/common/uri'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IMarkerService } from 'vs/platform/markers/common/markers'; import { IThemeService } from 'vs/platform/theme/common/themeService'; -import { IActiveNotebookEditor, INotebookEditor, INotebookViewCellsUpdateEvent } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { IActiveNotebookEditor, ICellViewModel, INotebookEditor, INotebookViewCellsUpdateEvent } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { OutlineChangeEvent, OutlineConfigKeys, OutlineTarget } from 'vs/workbench/services/outline/browser/outline'; @@ -70,9 +70,10 @@ export class NotebookCellOutlineProvider { ); this._dispoables.add(_configurationService.onDidChangeConfiguration(e => { - if (e.affectsConfiguration('notebook.outline.showCodeCells') || - e.affectsConfiguration('notebook.outline.showNonHeaderMarkdownCells') || - e.affectsConfiguration('notebook.outline.showCodeCellSymbols') + if (e.affectsConfiguration('notebook.outline.showMarkdownHeadersOnly') || + e.affectsConfiguration('notebook.outline.showCodeCells') || + e.affectsConfiguration('notebook.outline.showCodeCellSymbols') || + e.affectsConfiguration('notebook.breadcrumbs.showCodeCells') ) { this._recomputeState(); } @@ -139,15 +140,16 @@ export class NotebookCellOutlineProvider { } let includeCodeCells = true; - if (this._target === OutlineTarget.OutlinePane) { - includeCodeCells = this._configurationService.getValue('notebook.outline.showCodeCells'); - } else if (this._target === OutlineTarget.Breadcrumbs) { + if (this._target === OutlineTarget.Breadcrumbs) { includeCodeCells = this._configurationService.getValue('notebook.breadcrumbs.showCodeCells'); } - const showNonHeaderMarkdownCells = this._configurationService.getValue('notebook.outline.showNonHeaderMarkdownCells'); - - const notebookCells = notebookEditorWidget.getViewModel().viewCells.filter((cell) => cell.cellKind === CellKind.Markup || includeCodeCells); + let notebookCells: ICellViewModel[]; + if (this._target === OutlineTarget.Breadcrumbs) { + notebookCells = notebookEditorWidget.getViewModel().viewCells.filter((cell) => cell.cellKind === CellKind.Markup || includeCodeCells); + } else { + notebookCells = notebookEditorWidget.getViewModel().viewCells; + } const entries: OutlineEntry[] = []; for (const cell of notebookCells) { @@ -167,11 +169,6 @@ export class NotebookCellOutlineProvider { for (let i = 1; i < entries.length; i++) { const entry = entries[i]; - if (!showNonHeaderMarkdownCells && entry.cell.cellKind === CellKind.Markup && entry.level === 7) { - // skip plain text markdown cells - continue; - } - while (true) { const len = parentStack.length; if (len === 0) { diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorStickyScroll.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorStickyScroll.ts index 1542d6751655d..5900460feb9f5 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorStickyScroll.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorStickyScroll.ts @@ -281,7 +281,7 @@ export class NotebookStickyScroll extends Disposable { static computeStickyHeight(entry: OutlineEntry) { let height = 0; - if (entry.cell.cellKind === CellKind.Markup && entry.level !== 7) { + if (entry.cell.cellKind === CellKind.Markup && entry.level < 7) { height += 22; } while (entry.parent) { @@ -297,8 +297,8 @@ export class NotebookStickyScroll extends Disposable { const elementsToRender = []; while (currentEntry) { - if (currentEntry.level === 7) { - // level 7 represents a non-header entry, which we don't want to render + if (currentEntry.level >= 7) { + // level 7+ represents a non-header entry, which we don't want to render currentEntry = currentEntry.parent; continue; } @@ -382,7 +382,7 @@ export function computeContent(notebookEditor: INotebookEditor, notebookCellList if (visibleRange.start === 0) { const firstCell = notebookEditor.cellAt(0); const firstCellEntry = NotebookStickyScroll.getVisibleOutlineEntry(0, notebookOutlineEntries); - if (firstCell && firstCellEntry && firstCell.cellKind === CellKind.Markup && firstCellEntry.level !== 7) { + if (firstCell && firstCellEntry && firstCell.cellKind === CellKind.Markup && firstCellEntry.level < 7) { if (notebookEditor.scrollTop > 22) { const newMap = NotebookStickyScroll.checkCollapsedStickyLines(firstCellEntry, 100, notebookEditor); return newMap; @@ -418,7 +418,7 @@ export function computeContent(notebookEditor: INotebookEditor, notebookCellList } // check next cell, if markdown with non level 7 entry, that means this is the end of the section (new header) --------------------- - if (nextCell.cellKind === CellKind.Markup && nextCellEntry.level !== 7) { + if (nextCell.cellKind === CellKind.Markup && nextCellEntry.level < 7) { const sectionBottom = notebookCellList.getCellViewScrollTop(nextCell); const currentSectionStickyHeight = NotebookStickyScroll.computeStickyHeight(cellEntry); const nextSectionStickyHeight = NotebookStickyScroll.computeStickyHeight(nextCellEntry); From 505e57618a84bde07ffb5490920fe259158d73e8 Mon Sep 17 00:00:00 2001 From: Michael Lively Date: Thu, 21 Mar 2024 14:35:29 -0700 Subject: [PATCH 3/4] fix broken outline filter logic // pass target to entryFactory // fix tests for symbol entries being level 8+ --- .../contrib/outline/notebookOutline.ts | 17 ++++++++----- .../viewModel/notebookOutlineEntryFactory.ts | 20 ++++++++++----- .../viewModel/notebookOutlineProvider.ts | 2 +- .../browser/contrib/notebookSymbols.test.ts | 25 ++++++++++--------- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts index d965598f9ca79..5205d655052e0 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts @@ -419,19 +419,24 @@ export class NotebookCellOutline implements IOutline { } *getChildren(parent: OutlineEntry | NotebookCellOutline, configurationService: IConfigurationService): Iterable { - for (const entry of parent instanceof NotebookCellOutline ? (this._outlineProvider?.entries ?? []) : parent.children) { - const showOutlineCodeCells = configurationService.getValue('notebook.outline.showCodeCells'); - const showMarkdownHeadersOnly = configurationService.getValue('notebook.outline.showMarkdownHeadersOnly'); + const showCodeCells = configurationService.getValue('notebook.outline.showCodeCells'); + const showCodeCellSymbols = configurationService.getValue('notebook.outline.showCodeCellSymbols'); + const showMarkdownHeadersOnly = configurationService.getValue('notebook.outline.showMarkdownHeadersOnly'); + for (const entry of parent instanceof NotebookCellOutline ? (this._outlineProvider?.entries ?? []) : parent.children) { if (entry.cell.cellKind === CellKind.Markup) { if (!showMarkdownHeadersOnly) { yield entry; } else if (entry.level < 7) { yield entry; } - } - if (showOutlineCodeCells && entry.cell.cellKind === CellKind.Code) { - yield entry; + + } else if (showCodeCells && entry.cell.cellKind === CellKind.Code) { + if (showCodeCellSymbols) { + yield entry; + } else if (entry.level === 7) { + yield entry; + } } } } diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts index 4a0cc1e862be7..0222b835bda33 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts @@ -14,6 +14,7 @@ import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { IRange } from 'vs/editor/common/core/range'; import { SymbolKind } from 'vs/editor/common/languages'; +import { OutlineTarget } from 'vs/workbench/services/outline/browser/outline'; type entryDesc = { name: string; @@ -30,7 +31,7 @@ export class NotebookOutlineEntryFactory { private readonly executionStateService: INotebookExecutionStateService ) { } - public getOutlineEntries(cell: ICellViewModel, index: number): OutlineEntry[] { + public getOutlineEntries(cell: ICellViewModel, target: OutlineTarget, index: number): OutlineEntry[] { const entries: OutlineEntry[] = []; const isMarkdown = cell.cellKind === CellKind.Markup; @@ -67,11 +68,6 @@ export class NotebookOutlineEntryFactory { if (!hasHeader) { const exeState = !isMarkdown && this.executionStateService.getCellExecution(cell.uri); let preview = content.trim(); - if (preview.length === 0) { - // empty or just whitespace - preview = localize('empty', "empty cell"); - } - entries.push(new OutlineEntry(index++, 7, cell, preview, !!exeState, exeState ? exeState.isPaused : false)); if (!isMarkdown && cell.model.textModel) { const cachedEntries = this.cellOutlineEntryCache[cell.model.textModel.id]; @@ -79,11 +75,23 @@ export class NotebookOutlineEntryFactory { // Gathering symbols from the model is an async operation, but this provider is syncronous. // So symbols need to be precached before this function is called to get the full list. if (cachedEntries) { + // push code cell that is a parent of cached symbols if we are targeting the outlinePane + if (target === OutlineTarget.OutlinePane) { + entries.push(new OutlineEntry(index++, 7, cell, preview, !!exeState, exeState ? exeState.isPaused : false)); + } cachedEntries.forEach((cached) => { entries.push(new OutlineEntry(index++, cached.level, cell, cached.name, false, false, cached.range, cached.kind)); }); } } + + if (entries.length === 0) { // if there are no cached entries, use the first line of the cell as a code cell + if (preview.length === 0) { + // empty or just whitespace + preview = localize('empty', "empty cell"); + } + entries.push(new OutlineEntry(index++, 7, cell, preview, !!exeState, exeState ? exeState.isPaused : false)); + } } return entries; diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts index 3978a4d06ecab..2937866999cba 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts @@ -153,7 +153,7 @@ export class NotebookCellOutlineProvider { const entries: OutlineEntry[] = []; for (const cell of notebookCells) { - entries.push(...this._outlineEntryFactory.getOutlineEntries(cell, entries.length)); + entries.push(...this._outlineEntryFactory.getOutlineEntries(cell, this._target, entries.length)); // send an event whenever any of the cells change this._entriesDisposables.add(cell.model.onDidChangeContent(() => { this._recomputeState(); diff --git a/src/vs/workbench/contrib/notebook/test/browser/contrib/notebookSymbols.test.ts b/src/vs/workbench/contrib/notebook/test/browser/contrib/notebookSymbols.test.ts index 8826eb3dda787..fdb19d202424e 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/contrib/notebookSymbols.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/contrib/notebookSymbols.test.ts @@ -12,6 +12,7 @@ import { IOutlineModelService, OutlineModel } from 'vs/editor/contrib/documentSy import { ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { NotebookOutlineEntryFactory } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { OutlineTarget } from 'vs/workbench/services/outline/browser/outline'; suite('Notebook Symbols', function () { ensureNoDisposablesAreLeakedInTestSuite(); @@ -66,7 +67,7 @@ suite('Notebook Symbols', function () { test('Cell without symbols cache', function () { setSymbolsForTextModel([{ name: 'var', range: {} }]); const entryFactory = new NotebookOutlineEntryFactory(executionService); - const entries = entryFactory.getOutlineEntries(createCellViewModel(), 0); + const entries = entryFactory.getOutlineEntries(createCellViewModel(), OutlineTarget.QuickPick, 0); assert.equal(entries.length, 1, 'no entries created'); assert.equal(entries[0].label, '# code', 'entry should fall back to first line of cell'); @@ -78,15 +79,15 @@ suite('Notebook Symbols', function () { const cell = createCellViewModel(); await entryFactory.cacheSymbols(cell, outlineModelService, CancellationToken.None); - const entries = entryFactory.getOutlineEntries(cell, 0); + const entries = entryFactory.getOutlineEntries(cell, OutlineTarget.QuickPick, 0); assert.equal(entries.length, 2, 'wrong number of outline entries'); assert.equal(entries[0].label, 'var1'); // 6 levels for markdown, all code symbols are greater than the max markdown level - assert.equal(entries[0].level, 7); + assert.equal(entries[0].level, 8); assert.equal(entries[0].index, 0); assert.equal(entries[1].label, 'var2'); - assert.equal(entries[1].level, 7); + assert.equal(entries[1].level, 8); assert.equal(entries[1].index, 1); }); @@ -99,19 +100,19 @@ suite('Notebook Symbols', function () { const cell = createCellViewModel(); await entryFactory.cacheSymbols(cell, outlineModelService, CancellationToken.None); - const entries = entryFactory.getOutlineEntries(createCellViewModel(), 0); + const entries = entryFactory.getOutlineEntries(createCellViewModel(), OutlineTarget.QuickPick, 0); assert.equal(entries.length, 5, 'wrong number of outline entries'); assert.equal(entries[0].label, 'root1'); - assert.equal(entries[0].level, 7); + assert.equal(entries[0].level, 8); assert.equal(entries[1].label, 'nested1'); - assert.equal(entries[1].level, 8); + assert.equal(entries[1].level, 9); assert.equal(entries[2].label, 'nested2'); - assert.equal(entries[2].level, 8); + assert.equal(entries[2].level, 9); assert.equal(entries[3].label, 'root2'); - assert.equal(entries[3].level, 7); + assert.equal(entries[3].level, 8); assert.equal(entries[4].label, 'nested1'); - assert.equal(entries[4].level, 8); + assert.equal(entries[4].level, 9); }); test('Multiple Cells with symbols', async function () { @@ -124,8 +125,8 @@ suite('Notebook Symbols', function () { await entryFactory.cacheSymbols(cell1, outlineModelService, CancellationToken.None); await entryFactory.cacheSymbols(cell2, outlineModelService, CancellationToken.None); - const entries1 = entryFactory.getOutlineEntries(createCellViewModel(1, '$1'), 0); - const entries2 = entryFactory.getOutlineEntries(createCellViewModel(1, '$2'), 0); + const entries1 = entryFactory.getOutlineEntries(createCellViewModel(1, '$1'), OutlineTarget.QuickPick, 0); + const entries2 = entryFactory.getOutlineEntries(createCellViewModel(1, '$2'), OutlineTarget.QuickPick, 0); assert.equal(entries1.length, 1, 'wrong number of outline entries'); From 28faf4bd761a675832d2afcbb1672f3f3307b039 Mon Sep 17 00:00:00 2001 From: Michael Lively Date: Thu, 21 Mar 2024 14:50:00 -0700 Subject: [PATCH 4/4] define settings in `notebookCommon.ts` --- .../contrib/outline/notebookOutline.ts | 28 +++++++++---------- .../viewModel/notebookOutlineProvider.ts | 10 +++---- .../contrib/notebook/common/notebookCommon.ts | 4 +++ 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts index 5205d655052e0..cac4053c00b55 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts @@ -419,9 +419,9 @@ export class NotebookCellOutline implements IOutline { } *getChildren(parent: OutlineEntry | NotebookCellOutline, configurationService: IConfigurationService): Iterable { - const showCodeCells = configurationService.getValue('notebook.outline.showCodeCells'); - const showCodeCellSymbols = configurationService.getValue('notebook.outline.showCodeCellSymbols'); - const showMarkdownHeadersOnly = configurationService.getValue('notebook.outline.showMarkdownHeadersOnly'); + const showCodeCells = configurationService.getValue(NotebookSetting.outlineShowCodeCells); + const showCodeCellSymbols = configurationService.getValue(NotebookSetting.outlineShowCodeCellSymbols); + const showMarkdownHeadersOnly = configurationService.getValue(NotebookSetting.outlineShowMarkdownHeadersOnly); for (const entry of parent instanceof NotebookCellOutline ? (this._outlineProvider?.entries ?? []) : parent.children) { if (entry.cell.cellKind === CellKind.Markup) { @@ -554,7 +554,7 @@ export class NotebookOutlineCreator implements IOutlineCreator(NotebookSetting.gotoSymbolsAllSymbols); - const showAllOutlineSymbols = this._configurationService.getValue('notebook.outline.showCodeCellSymbols'); + const showAllOutlineSymbols = this._configurationService.getValue(NotebookSetting.outlineShowCodeCellSymbols); if (target === OutlineTarget.QuickPick && showAllGotoSymbols) { await outline.setFullSymbols(cancelToken); } else if (target === OutlineTarget.OutlinePane && showAllOutlineSymbols) { @@ -580,22 +580,22 @@ Registry.as(ConfigurationExtensions.Configuration).regis order: 100, type: 'object', 'properties': { - 'notebook.outline.showMarkdownHeadersOnly': { + [NotebookSetting.outlineShowMarkdownHeadersOnly]: { type: 'boolean', default: true, markdownDescription: localize('outline.showMarkdownHeadersOnly', "When enabled, notebook outline will show only markdown cells containing a header.") }, - 'notebook.outline.showCodeCells': { + [NotebookSetting.outlineShowCodeCells]: { type: 'boolean', default: false, markdownDescription: localize('outline.showCodeCells', "When enabled, notebook outline shows code cells.") }, - 'notebook.outline.showCodeCellSymbols': { + [NotebookSetting.outlineShowCodeCellSymbols]: { type: 'boolean', default: true, markdownDescription: localize('outline.showCodeCellSymbols', "When enabled, notebook outline shows code cell symbols. Relies on `notebook.outline.showCodeCells` being enabled.") }, - 'notebook.breadcrumbs.showCodeCells': { + [NotebookSetting.breadcrumbsShowCodeCells]: { type: 'boolean', default: true, markdownDescription: localize('breadcrumbs.showCodeCells', "When enabled, notebook breadcrumbs contain code cells.") @@ -635,8 +635,8 @@ registerAction2(class ToggleShowMarkdownHeadersOnly extends Action2 { run(accessor: ServicesAccessor, ...args: any[]) { const configurationService = accessor.get(IConfigurationService); - const showMarkdownHeadersOnly = configurationService.getValue('notebook.outline.showMarkdownHeadersOnly'); - configurationService.updateValue('notebook.outline.showMarkdownHeadersOnly', !showMarkdownHeadersOnly); + const showMarkdownHeadersOnly = configurationService.getValue(NotebookSetting.outlineShowMarkdownHeadersOnly); + configurationService.updateValue(NotebookSetting.outlineShowMarkdownHeadersOnly, !showMarkdownHeadersOnly); } }); @@ -660,8 +660,8 @@ registerAction2(class ToggleCodeCellEntries extends Action2 { run(accessor: ServicesAccessor, ...args: any[]) { const configurationService = accessor.get(IConfigurationService); - const showCodeCells = configurationService.getValue('notebook.outline.showCodeCells'); - configurationService.updateValue('notebook.outline.showCodeCells', !showCodeCells); + const showCodeCells = configurationService.getValue(NotebookSetting.outlineShowCodeCells); + configurationService.updateValue(NotebookSetting.outlineShowCodeCells, !showCodeCells); } }); @@ -684,7 +684,7 @@ registerAction2(class ToggleCodeCellSymbolEntries extends Action2 { run(accessor: ServicesAccessor, ...args: any[]) { const configurationService = accessor.get(IConfigurationService); - const showCodeCellSymbols = configurationService.getValue('notebook.outline.showCodeCellSymbols'); - configurationService.updateValue('notebook.outline.showCodeCellSymbols', !showCodeCellSymbols); + const showCodeCellSymbols = configurationService.getValue(NotebookSetting.outlineShowCodeCellSymbols); + configurationService.updateValue(NotebookSetting.outlineShowCodeCellSymbols, !showCodeCellSymbols); } }); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts index 2937866999cba..fb5c32bb37c5b 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts @@ -11,7 +11,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur import { IMarkerService } from 'vs/platform/markers/common/markers'; import { IThemeService } from 'vs/platform/theme/common/themeService'; import { IActiveNotebookEditor, ICellViewModel, INotebookEditor, INotebookViewCellsUpdateEvent } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { OutlineChangeEvent, OutlineConfigKeys, OutlineTarget } from 'vs/workbench/services/outline/browser/outline'; import { OutlineEntry } from './OutlineEntry'; @@ -70,10 +70,10 @@ export class NotebookCellOutlineProvider { ); this._dispoables.add(_configurationService.onDidChangeConfiguration(e => { - if (e.affectsConfiguration('notebook.outline.showMarkdownHeadersOnly') || - e.affectsConfiguration('notebook.outline.showCodeCells') || - e.affectsConfiguration('notebook.outline.showCodeCellSymbols') || - e.affectsConfiguration('notebook.breadcrumbs.showCodeCells') + if (e.affectsConfiguration(NotebookSetting.outlineShowMarkdownHeadersOnly) || + e.affectsConfiguration(NotebookSetting.outlineShowCodeCells) || + e.affectsConfiguration(NotebookSetting.outlineShowCodeCellSymbols) || + e.affectsConfiguration(NotebookSetting.breadcrumbsShowCodeCells) ) { this._recomputeState(); } diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 37725f005dd81..c8d52b553d843 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -947,6 +947,10 @@ export const NotebookSetting = { confirmDeleteRunningCell: 'notebook.confirmDeleteRunningCell', remoteSaving: 'notebook.experimental.remoteSave', gotoSymbolsAllSymbols: 'notebook.gotoSymbols.showAllSymbols', + outlineShowMarkdownHeadersOnly: 'notebook.outline.showMarkdownHeadersOnly', + outlineShowCodeCells: 'notebook.outline.showCodeCells', + outlineShowCodeCellSymbols: 'notebook.outline.showCodeCellSymbols', + breadcrumbsShowCodeCells: 'notebook.breadcrumbs.showCodeCells', scrollToRevealCell: 'notebook.scrolling.revealNextCellOnExecute', anchorToFocusedCell: 'notebook.scrolling.experimental.anchorToFocusedCell', cellChat: 'notebook.experimental.cellChat',