Add code cell symbols to nb outline + debt fixes (#208323)

* add symbol entries as children of code cells in nb outline

* extract filtering logic to getChildren for IOutline config, adjust level checks in nb SS

* fix broken outline filter logic // pass target to entryFactory // fix tests for symbol entries being level 8+

* define settings in `notebookCommon.ts`
This commit is contained in:
Michael Lively 2024-03-21 15:04:52 -07:00 committed by GitHub
parent 70e896c977
commit 9f1605bd09
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 141 additions and 69 deletions

View file

@ -106,14 +106,16 @@ class NotebookOutlineRenderer implements ITreeRenderer<OutlineEntry, FuzzyScore,
};
const isCodeCell = node.element.cell.cellKind === CellKind.Code;
if (isCodeCell && this._themeService.getFileIconTheme().hasFileIcons && !node.element.isExecuting) {
if (node.element.level >= 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;
@ -377,7 +379,11 @@ export class NotebookCellOutline implements IOutline<OutlineEntry> {
}));
installSelectionListener();
const treeDataSource: IDataSource<this, OutlineEntry> = { getChildren: parent => parent instanceof NotebookCellOutline ? (this._outlineProvider?.entries ?? []) : parent.children };
const treeDataSource: IDataSource<this, OutlineEntry> = {
getChildren: parent => {
return this.getChildren(parent, _configurationService);
}
};
const delegate = new NotebookOutlineVirtualDelegate();
const renderers = [instantiationService.createInstance(NotebookOutlineRenderer, this._editor.getControl(), _target)];
const comparator = new NotebookComparator();
@ -412,6 +418,29 @@ export class NotebookCellOutline implements IOutline<OutlineEntry> {
};
}
*getChildren(parent: OutlineEntry | NotebookCellOutline, configurationService: IConfigurationService): Iterable<OutlineEntry> {
const showCodeCells = configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCells);
const showCodeCellSymbols = configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCellSymbols);
const showMarkdownHeadersOnly = configurationService.getValue<boolean>(NotebookSetting.outlineShowMarkdownHeadersOnly);
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;
}
} else if (showCodeCells && entry.cell.cellKind === CellKind.Code) {
if (showCodeCellSymbols) {
yield entry;
} else if (entry.level === 7) {
yield entry;
}
}
}
}
async setFullSymbols(cancelToken: CancellationToken) {
await this._outlineProvider?.setFullSymbols(cancelToken);
}
@ -524,10 +553,14 @@ export class NotebookOutlineCreator implements IOutlineCreator<NotebookEditor, O
async createOutline(editor: NotebookEditor, target: OutlineTarget, cancelToken: CancellationToken): Promise<IOutline<OutlineEntry> | undefined> {
const outline = this._instantiationService.createInstance(NotebookCellOutline, editor, target);
const showAllSymbols = this._configurationService.getValue<boolean>(NotebookSetting.gotoSymbolsAllSymbols);
if (target === OutlineTarget.QuickPick && showAllSymbols) {
const showAllGotoSymbols = this._configurationService.getValue<boolean>(NotebookSetting.gotoSymbolsAllSymbols);
const showAllOutlineSymbols = this._configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCellSymbols);
if (target === OutlineTarget.QuickPick && showAllGotoSymbols) {
await outline.setFullSymbols(cancelToken);
} else if (target === OutlineTarget.OutlinePane && showAllOutlineSymbols) {
await outline.setFullSymbols(cancelToken);
}
return outline;
}
}
@ -547,25 +580,30 @@ Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration).regis
order: 100,
type: 'object',
'properties': {
'notebook.outline.showCodeCells': {
type: 'boolean',
default: false,
markdownDescription: localize('outline.showCodeCells', "When enabled notebook outline shows code cells.")
},
'notebook.outline.showNonHeaderMarkdownCells': {
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.")
},
'notebook.breadcrumbs.showCodeCells': {
[NotebookSetting.outlineShowMarkdownHeadersOnly]: {
type: 'boolean',
default: true,
markdownDescription: localize('breadcrumbs.showCodeCells', "When enabled notebook breadcrumbs contain code cells.")
markdownDescription: localize('outline.showMarkdownHeadersOnly', "When enabled, notebook outline will show only markdown cells containing a header.")
},
[NotebookSetting.outlineShowCodeCells]: {
type: 'boolean',
default: false,
markdownDescription: localize('outline.showCodeCells', "When enabled, notebook outline shows code cells.")
},
[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.")
},
[NotebookSetting.breadcrumbsShowCodeCells]: {
type: 'boolean',
default: true,
markdownDescription: localize('breadcrumbs.showCodeCells', "When enabled, notebook breadcrumbs contain code cells.")
},
[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.")
},
}
});
@ -579,48 +617,74 @@ MenuRegistry.appendMenuItem(MenuId.ViewTitle, {
when: ContextKeyExpr.and(ContextKeyExpr.equals('view', IOutlinePane.Id), NOTEBOOK_IS_ACTIVE_EDITOR),
});
registerAction2(class ToggleShowMarkdownHeadersOnly extends Action2 {
constructor() {
super({
id: 'notebook.outline.toggleShowMarkdownHeadersOnly',
title: localize('toggleShowMarkdownHeadersOnly', "Markdown Headers Only"),
f1: false,
toggled: {
condition: ContextKeyExpr.equals('config.notebook.outline.showMarkdownHeadersOnly', true)
},
menu: {
id: MenuId.NotebookOutlineFilter,
group: '0_markdown_cells',
}
});
}
run(accessor: ServicesAccessor, ...args: any[]) {
const configurationService = accessor.get(IConfigurationService);
const showMarkdownHeadersOnly = configurationService.getValue<boolean>(NotebookSetting.outlineShowMarkdownHeadersOnly);
configurationService.updateValue(NotebookSetting.outlineShowMarkdownHeadersOnly, !showMarkdownHeadersOnly);
}
});
registerAction2(class ToggleCodeCellEntries extends Action2 {
constructor() {
super({
id: 'notebook.outline.toggleCodeCells',
title: localize('toggleCodeCells', "Toggle Code Cells"),
title: localize('toggleCodeCells', "Code Cells"),
f1: false,
toggled: {
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 showCodeCells = configurationService.getValue<boolean>('notebook.outline.showCodeCells');
configurationService.updateValue('notebook.outline.showCodeCells', !showCodeCells);
const showCodeCells = configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCells);
configurationService.updateValue(NotebookSetting.outlineShowCodeCells, !showCodeCells);
}
});
registerAction2(class ToggleNonHeaderMarkdownCells extends Action2 {
registerAction2(class ToggleCodeCellSymbolEntries extends Action2 {
constructor() {
super({
id: 'notebook.outline.toggleNonHeaderMarkdownCells',
title: localize('toggleNonHeaderMarkdownCells', "Toggle Non-Header Markdown Cells"),
id: 'notebook.outline.toggleCodeCellSymbols',
title: localize('toggleCodeCellSymbols', "Code Cell Symbols"),
f1: false,
toggled: {
condition: ContextKeyExpr.equals('config.notebook.outline.showNonHeaderMarkdownCells', true)
condition: ContextKeyExpr.equals('config.notebook.outline.showCodeCellSymbols', true)
},
menu: {
id: MenuId.NotebookOutlineFilter,
group: 'filter',
order: 2,
group: '1_code_cells',
}
});
}
run(accessor: ServicesAccessor, ...args: any[]) {
const configurationService = accessor.get(IConfigurationService);
const showNonHeaderMarkdownCells = configurationService.getValue<boolean>('notebook.outline.showNonHeaderMarkdownCells');
configurationService.updateValue('notebook.outline.showNonHeaderMarkdownCells', !showNonHeaderMarkdownCells);
const showCodeCellSymbols = configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCellSymbols);
configurationService.updateValue(NotebookSetting.outlineShowCodeCellSymbols, !showCodeCellSymbols);
}
});

View file

@ -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;
@ -65,26 +66,30 @@ export class NotebookOutlineEntryFactory {
}
if (!hasHeader) {
const exeState = !isMarkdown && this.executionStateService.getCellExecution(cell.uri);
let preview = content.trim();
if (!isMarkdown && cell.model.textModel) {
const cachedEntries = this.cellOutlineEntryCache[cell.model.textModel.id];
// 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));
});
}
}
const exeState = !isMarkdown && this.executionStateService.getCellExecution(cell.uri);
if (entries.length === 0) {
let preview = content.trim();
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));
}
}
@ -95,7 +100,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;
}
}

View file

@ -10,8 +10,8 @@ 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 { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { IActiveNotebookEditor, ICellViewModel, INotebookEditor, INotebookViewCellsUpdateEvent } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
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,7 +70,11 @@ export class NotebookCellOutlineProvider {
);
this._dispoables.add(_configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration('notebook.outline.showCodeCells') || e.affectsConfiguration('notebook.outline.showNonHeaderMarkdownCells')) {
if (e.affectsConfiguration(NotebookSetting.outlineShowMarkdownHeadersOnly) ||
e.affectsConfiguration(NotebookSetting.outlineShowCodeCells) ||
e.affectsConfiguration(NotebookSetting.outlineShowCodeCellSymbols) ||
e.affectsConfiguration(NotebookSetting.breadcrumbsShowCodeCells)
) {
this._recomputeState();
}
}));
@ -136,19 +140,20 @@ export class NotebookCellOutlineProvider {
}
let includeCodeCells = true;
if (this._target === OutlineTarget.OutlinePane) {
includeCodeCells = this._configurationService.getValue<boolean>('notebook.outline.showCodeCells');
} else if (this._target === OutlineTarget.Breadcrumbs) {
if (this._target === OutlineTarget.Breadcrumbs) {
includeCodeCells = this._configurationService.getValue<boolean>('notebook.breadcrumbs.showCodeCells');
}
const showNonHeaderMarkdownCells = this._configurationService.getValue<boolean>('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) {
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();
@ -164,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) {
@ -269,8 +269,6 @@ export class NotebookCellOutlineProvider {
}
}
get isEmpty(): boolean {
return this._entries.length === 0;
}

View file

@ -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);

View file

@ -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',

View file

@ -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');