Skip to content

Commit 9f1605b

Browse files
YoyokrazyYoyokrazy
authored
Add code cell symbols to nb outline + debt fixes (microsoft#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`
1 parent 70e896c commit 9f1605b

File tree

6 files changed

+136
-64
lines changed

6 files changed

+136
-64
lines changed

‎src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts

Lines changed: 88 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,16 @@ class NotebookOutlineRenderer implements ITreeRenderer<OutlineEntry, FuzzyScore,
106106
};
107107

108108
const isCodeCell = node.element.cell.cellKind === CellKind.Code;
109-
if (isCodeCell && this._themeService.getFileIconTheme().hasFileIcons && !node.element.isExecuting) {
109+
if (node.element.level >= 8) { // symbol
110+
template.iconClass.className = 'element-icon ' + ThemeIcon.asClassNameArray(node.element.icon).join(' ');
111+
} else if (isCodeCell && this._themeService.getFileIconTheme().hasFileIcons && !node.element.isExecuting) {
110112
template.iconClass.className = '';
111113
extraClasses.push(...getIconClassesForLanguageId(node.element.cell.language ?? ''));
112114
} else {
113115
template.iconClass.className = 'element-icon ' + ThemeIcon.asClassNameArray(node.element.icon).join(' ');
114116
}
115117

116-
template.iconLabel.setLabel(node.element.label, undefined, options);
118+
template.iconLabel.setLabel(' ' + node.element.label, undefined, options);
117119

118120
const { markerInfo } = node.element;
119121

@@ -377,7 +379,11 @@ export class NotebookCellOutline implements IOutline<OutlineEntry> {
377379
}));
378380

379381
installSelectionListener();
380-
const treeDataSource: IDataSource<this, OutlineEntry> = { getChildren: parent => parent instanceof NotebookCellOutline ? (this._outlineProvider?.entries ?? []) : parent.children };
382+
const treeDataSource: IDataSource<this, OutlineEntry> = {
383+
getChildren: parent => {
384+
return this.getChildren(parent, _configurationService);
385+
}
386+
};
381387
const delegate = new NotebookOutlineVirtualDelegate();
382388
const renderers = [instantiationService.createInstance(NotebookOutlineRenderer, this._editor.getControl(), _target)];
383389
const comparator = new NotebookComparator();
@@ -412,6 +418,29 @@ export class NotebookCellOutline implements IOutline<OutlineEntry> {
412418
};
413419
}
414420

421+
*getChildren(parent: OutlineEntry | NotebookCellOutline, configurationService: IConfigurationService): Iterable<OutlineEntry> {
422+
const showCodeCells = configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCells);
423+
const showCodeCellSymbols = configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCellSymbols);
424+
const showMarkdownHeadersOnly = configurationService.getValue<boolean>(NotebookSetting.outlineShowMarkdownHeadersOnly);
425+
426+
for (const entry of parent instanceof NotebookCellOutline ? (this._outlineProvider?.entries ?? []) : parent.children) {
427+
if (entry.cell.cellKind === CellKind.Markup) {
428+
if (!showMarkdownHeadersOnly) {
429+
yield entry;
430+
} else if (entry.level < 7) {
431+
yield entry;
432+
}
433+
434+
} else if (showCodeCells && entry.cell.cellKind === CellKind.Code) {
435+
if (showCodeCellSymbols) {
436+
yield entry;
437+
} else if (entry.level === 7) {
438+
yield entry;
439+
}
440+
}
441+
}
442+
}
443+
415444
async setFullSymbols(cancelToken: CancellationToken) {
416445
await this._outlineProvider?.setFullSymbols(cancelToken);
417446
}
@@ -524,10 +553,14 @@ export class NotebookOutlineCreator implements IOutlineCreator<NotebookEditor, O
524553
async createOutline(editor: NotebookEditor, target: OutlineTarget, cancelToken: CancellationToken): Promise<IOutline<OutlineEntry> | undefined> {
525554
const outline = this._instantiationService.createInstance(NotebookCellOutline, editor, target);
526555

527-
const showAllSymbols = this._configurationService.getValue<boolean>(NotebookSetting.gotoSymbolsAllSymbols);
528-
if (target === OutlineTarget.QuickPick && showAllSymbols) {
556+
const showAllGotoSymbols = this._configurationService.getValue<boolean>(NotebookSetting.gotoSymbolsAllSymbols);
557+
const showAllOutlineSymbols = this._configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCellSymbols);
558+
if (target === OutlineTarget.QuickPick && showAllGotoSymbols) {
559+
await outline.setFullSymbols(cancelToken);
560+
} else if (target === OutlineTarget.OutlinePane && showAllOutlineSymbols) {
529561
await outline.setFullSymbols(cancelToken);
530562
}
563+
531564
return outline;
532565
}
533566
}
@@ -547,25 +580,30 @@ Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration).regis
547580
order: 100,
548581
type: 'object',
549582
'properties': {
550-
'notebook.outline.showCodeCells': {
583+
[NotebookSetting.outlineShowMarkdownHeadersOnly]: {
551584
type: 'boolean',
552-
default: false,
553-
markdownDescription: localize('outline.showCodeCells', "When enabled notebook outline shows code cells.")
585+
default: true,
586+
markdownDescription: localize('outline.showMarkdownHeadersOnly', "When enabled, notebook outline will show only markdown cells containing a header.")
554587
},
555-
'notebook.outline.showNonHeaderMarkdownCells': {
588+
[NotebookSetting.outlineShowCodeCells]: {
556589
type: 'boolean',
557590
default: false,
558-
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.")
591+
markdownDescription: localize('outline.showCodeCells', "When enabled, notebook outline shows code cells.")
592+
},
593+
[NotebookSetting.outlineShowCodeCellSymbols]: {
594+
type: 'boolean',
595+
default: true,
596+
markdownDescription: localize('outline.showCodeCellSymbols', "When enabled, notebook outline shows code cell symbols. Relies on `notebook.outline.showCodeCells` being enabled.")
559597
},
560-
'notebook.breadcrumbs.showCodeCells': {
598+
[NotebookSetting.breadcrumbsShowCodeCells]: {
561599
type: 'boolean',
562600
default: true,
563-
markdownDescription: localize('breadcrumbs.showCodeCells', "When enabled notebook breadcrumbs contain code cells.")
601+
markdownDescription: localize('breadcrumbs.showCodeCells', "When enabled, notebook breadcrumbs contain code cells.")
564602
},
565603
[NotebookSetting.gotoSymbolsAllSymbols]: {
566604
type: 'boolean',
567605
default: true,
568-
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.")
606+
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.")
569607
},
570608
}
571609
});
@@ -579,48 +617,74 @@ MenuRegistry.appendMenuItem(MenuId.ViewTitle, {
579617
when: ContextKeyExpr.and(ContextKeyExpr.equals('view', IOutlinePane.Id), NOTEBOOK_IS_ACTIVE_EDITOR),
580618
});
581619

620+
registerAction2(class ToggleShowMarkdownHeadersOnly extends Action2 {
621+
constructor() {
622+
super({
623+
id: 'notebook.outline.toggleShowMarkdownHeadersOnly',
624+
title: localize('toggleShowMarkdownHeadersOnly', "Markdown Headers Only"),
625+
f1: false,
626+
toggled: {
627+
condition: ContextKeyExpr.equals('config.notebook.outline.showMarkdownHeadersOnly', true)
628+
},
629+
menu: {
630+
id: MenuId.NotebookOutlineFilter,
631+
group: '0_markdown_cells',
632+
}
633+
});
634+
}
635+
636+
run(accessor: ServicesAccessor, ...args: any[]) {
637+
const configurationService = accessor.get(IConfigurationService);
638+
const showMarkdownHeadersOnly = configurationService.getValue<boolean>(NotebookSetting.outlineShowMarkdownHeadersOnly);
639+
configurationService.updateValue(NotebookSetting.outlineShowMarkdownHeadersOnly, !showMarkdownHeadersOnly);
640+
}
641+
});
642+
643+
582644
registerAction2(class ToggleCodeCellEntries extends Action2 {
583645
constructor() {
584646
super({
585647
id: 'notebook.outline.toggleCodeCells',
586-
title: localize('toggleCodeCells', "Toggle Code Cells"),
648+
title: localize('toggleCodeCells', "Code Cells"),
587649
f1: false,
588650
toggled: {
589651
condition: ContextKeyExpr.equals('config.notebook.outline.showCodeCells', true)
590652
},
591653
menu: {
592654
id: MenuId.NotebookOutlineFilter,
593-
group: 'filter',
655+
order: 1,
656+
group: '1_code_cells',
594657
}
595658
});
596659
}
597660

598661
run(accessor: ServicesAccessor, ...args: any[]) {
599662
const configurationService = accessor.get(IConfigurationService);
600-
const showCodeCells = configurationService.getValue<boolean>('notebook.outline.showCodeCells');
601-
configurationService.updateValue('notebook.outline.showCodeCells', !showCodeCells);
663+
const showCodeCells = configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCells);
664+
configurationService.updateValue(NotebookSetting.outlineShowCodeCells, !showCodeCells);
602665
}
603666
});
604667

605-
registerAction2(class ToggleNonHeaderMarkdownCells extends Action2 {
668+
registerAction2(class ToggleCodeCellSymbolEntries extends Action2 {
606669
constructor() {
607670
super({
608-
id: 'notebook.outline.toggleNonHeaderMarkdownCells',
609-
title: localize('toggleNonHeaderMarkdownCells', "Toggle Non-Header Markdown Cells"),
671+
id: 'notebook.outline.toggleCodeCellSymbols',
672+
title: localize('toggleCodeCellSymbols', "Code Cell Symbols"),
610673
f1: false,
611674
toggled: {
612-
condition: ContextKeyExpr.equals('config.notebook.outline.showNonHeaderMarkdownCells', true)
675+
condition: ContextKeyExpr.equals('config.notebook.outline.showCodeCellSymbols', true)
613676
},
614677
menu: {
615678
id: MenuId.NotebookOutlineFilter,
616-
group: 'filter',
679+
order: 2,
680+
group: '1_code_cells',
617681
}
618682
});
619683
}
620684

621685
run(accessor: ServicesAccessor, ...args: any[]) {
622686
const configurationService = accessor.get(IConfigurationService);
623-
const showNonHeaderMarkdownCells = configurationService.getValue<boolean>('notebook.outline.showNonHeaderMarkdownCells');
624-
configurationService.updateValue('notebook.outline.showNonHeaderMarkdownCells', !showNonHeaderMarkdownCells);
687+
const showCodeCellSymbols = configurationService.getValue<boolean>(NotebookSetting.outlineShowCodeCellSymbols);
688+
configurationService.updateValue(NotebookSetting.outlineShowCodeCellSymbols, !showCodeCellSymbols);
625689
}
626690
});

‎src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineEntryFactory.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';
1414
import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
1515
import { IRange } from 'vs/editor/common/core/range';
1616
import { SymbolKind } from 'vs/editor/common/languages';
17+
import { OutlineTarget } from 'vs/workbench/services/outline/browser/outline';
1718

1819
type entryDesc = {
1920
name: string;
@@ -30,7 +31,7 @@ export class NotebookOutlineEntryFactory {
3031
private readonly executionStateService: INotebookExecutionStateService
3132
) { }
3233

33-
public getOutlineEntries(cell: ICellViewModel, index: number): OutlineEntry[] {
34+
public getOutlineEntries(cell: ICellViewModel, target: OutlineTarget, index: number): OutlineEntry[] {
3435
const entries: OutlineEntry[] = [];
3536

3637
const isMarkdown = cell.cellKind === CellKind.Markup;
@@ -65,26 +66,30 @@ export class NotebookOutlineEntryFactory {
6566
}
6667

6768
if (!hasHeader) {
69+
const exeState = !isMarkdown && this.executionStateService.getCellExecution(cell.uri);
70+
let preview = content.trim();
71+
6872
if (!isMarkdown && cell.model.textModel) {
6973
const cachedEntries = this.cellOutlineEntryCache[cell.model.textModel.id];
7074

7175
// Gathering symbols from the model is an async operation, but this provider is syncronous.
7276
// So symbols need to be precached before this function is called to get the full list.
7377
if (cachedEntries) {
78+
// push code cell that is a parent of cached symbols if we are targeting the outlinePane
79+
if (target === OutlineTarget.OutlinePane) {
80+
entries.push(new OutlineEntry(index++, 7, cell, preview, !!exeState, exeState ? exeState.isPaused : false));
81+
}
7482
cachedEntries.forEach((cached) => {
7583
entries.push(new OutlineEntry(index++, cached.level, cell, cached.name, false, false, cached.range, cached.kind));
7684
});
7785
}
7886
}
7987

80-
const exeState = !isMarkdown && this.executionStateService.getCellExecution(cell.uri);
81-
if (entries.length === 0) {
82-
let preview = content.trim();
88+
if (entries.length === 0) { // if there are no cached entries, use the first line of the cell as a code cell
8389
if (preview.length === 0) {
8490
// empty or just whitespace
8591
preview = localize('empty', "empty cell");
8692
}
87-
8893
entries.push(new OutlineEntry(index++, 7, cell, preview, !!exeState, exeState ? exeState.isPaused : false));
8994
}
9095
}
@@ -95,7 +100,7 @@ export class NotebookOutlineEntryFactory {
95100
public async cacheSymbols(cell: ICellViewModel, outlineModelService: IOutlineModelService, cancelToken: CancellationToken) {
96101
const textModel = await cell.resolveTextModel();
97102
const outlineModel = await outlineModelService.getOrCreate(textModel, cancelToken);
98-
const entries = createOutlineEntries(outlineModel.getTopLevelSymbols(), 7);
103+
const entries = createOutlineEntries(outlineModel.getTopLevelSymbols(), 8);
99104
this.cellOutlineEntryCache[textModel.id] = entries;
100105
}
101106
}

‎src/vs/workbench/contrib/notebook/browser/viewModel/notebookOutlineProvider.ts

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import { URI } from 'vs/base/common/uri';
1010
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
1111
import { IMarkerService } from 'vs/platform/markers/common/markers';
1212
import { IThemeService } from 'vs/platform/theme/common/themeService';
13-
import { IActiveNotebookEditor, INotebookEditor, INotebookViewCellsUpdateEvent } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
14-
import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';
13+
import { IActiveNotebookEditor, ICellViewModel, INotebookEditor, INotebookViewCellsUpdateEvent } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
14+
import { CellKind, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon';
1515
import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
1616
import { OutlineChangeEvent, OutlineConfigKeys, OutlineTarget } from 'vs/workbench/services/outline/browser/outline';
1717
import { OutlineEntry } from './OutlineEntry';
@@ -70,7 +70,11 @@ export class NotebookCellOutlineProvider {
7070
);
7171

7272
this._dispoables.add(_configurationService.onDidChangeConfiguration(e => {
73-
if (e.affectsConfiguration('notebook.outline.showCodeCells') || e.affectsConfiguration('notebook.outline.showNonHeaderMarkdownCells')) {
73+
if (e.affectsConfiguration(NotebookSetting.outlineShowMarkdownHeadersOnly) ||
74+
e.affectsConfiguration(NotebookSetting.outlineShowCodeCells) ||
75+
e.affectsConfiguration(NotebookSetting.outlineShowCodeCellSymbols) ||
76+
e.affectsConfiguration(NotebookSetting.breadcrumbsShowCodeCells)
77+
) {
7478
this._recomputeState();
7579
}
7680
}));
@@ -136,19 +140,20 @@ export class NotebookCellOutlineProvider {
136140
}
137141

138142
let includeCodeCells = true;
139-
if (this._target === OutlineTarget.OutlinePane) {
140-
includeCodeCells = this._configurationService.getValue<boolean>('notebook.outline.showCodeCells');
141-
} else if (this._target === OutlineTarget.Breadcrumbs) {
143+
if (this._target === OutlineTarget.Breadcrumbs) {
142144
includeCodeCells = this._configurationService.getValue<boolean>('notebook.breadcrumbs.showCodeCells');
143145
}
144146

145-
const showNonHeaderMarkdownCells = this._configurationService.getValue<boolean>('notebook.outline.showNonHeaderMarkdownCells');
146-
147-
const notebookCells = notebookEditorWidget.getViewModel().viewCells.filter((cell) => cell.cellKind === CellKind.Markup || includeCodeCells);
147+
let notebookCells: ICellViewModel[];
148+
if (this._target === OutlineTarget.Breadcrumbs) {
149+
notebookCells = notebookEditorWidget.getViewModel().viewCells.filter((cell) => cell.cellKind === CellKind.Markup || includeCodeCells);
150+
} else {
151+
notebookCells = notebookEditorWidget.getViewModel().viewCells;
152+
}
148153

149154
const entries: OutlineEntry[] = [];
150155
for (const cell of notebookCells) {
151-
entries.push(...this._outlineEntryFactory.getOutlineEntries(cell, entries.length));
156+
entries.push(...this._outlineEntryFactory.getOutlineEntries(cell, this._target, entries.length));
152157
// send an event whenever any of the cells change
153158
this._entriesDisposables.add(cell.model.onDidChangeContent(() => {
154159
this._recomputeState();
@@ -164,11 +169,6 @@ export class NotebookCellOutlineProvider {
164169
for (let i = 1; i < entries.length; i++) {
165170
const entry = entries[i];
166171

167-
if (!showNonHeaderMarkdownCells && entry.cell.cellKind === CellKind.Markup && entry.level === 7) {
168-
// skip plain text markdown cells
169-
continue;
170-
}
171-
172172
while (true) {
173173
const len = parentStack.length;
174174
if (len === 0) {
@@ -269,8 +269,6 @@ export class NotebookCellOutlineProvider {
269269
}
270270
}
271271

272-
273-
274272
get isEmpty(): boolean {
275273
return this._entries.length === 0;
276274
}

‎src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorStickyScroll.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ export class NotebookStickyScroll extends Disposable {
281281

282282
static computeStickyHeight(entry: OutlineEntry) {
283283
let height = 0;
284-
if (entry.cell.cellKind === CellKind.Markup && entry.level !== 7) {
284+
if (entry.cell.cellKind === CellKind.Markup && entry.level < 7) {
285285
height += 22;
286286
}
287287
while (entry.parent) {
@@ -297,8 +297,8 @@ export class NotebookStickyScroll extends Disposable {
297297

298298
const elementsToRender = [];
299299
while (currentEntry) {
300-
if (currentEntry.level === 7) {
301-
// level 7 represents a non-header entry, which we don't want to render
300+
if (currentEntry.level >= 7) {
301+
// level 7+ represents a non-header entry, which we don't want to render
302302
currentEntry = currentEntry.parent;
303303
continue;
304304
}
@@ -382,7 +382,7 @@ export function computeContent(notebookEditor: INotebookEditor, notebookCellList
382382
if (visibleRange.start === 0) {
383383
const firstCell = notebookEditor.cellAt(0);
384384
const firstCellEntry = NotebookStickyScroll.getVisibleOutlineEntry(0, notebookOutlineEntries);
385-
if (firstCell && firstCellEntry && firstCell.cellKind === CellKind.Markup && firstCellEntry.level !== 7) {
385+
if (firstCell && firstCellEntry && firstCell.cellKind === CellKind.Markup && firstCellEntry.level < 7) {
386386
if (notebookEditor.scrollTop > 22) {
387387
const newMap = NotebookStickyScroll.checkCollapsedStickyLines(firstCellEntry, 100, notebookEditor);
388388
return newMap;
@@ -418,7 +418,7 @@ export function computeContent(notebookEditor: INotebookEditor, notebookCellList
418418
}
419419

420420
// check next cell, if markdown with non level 7 entry, that means this is the end of the section (new header) ---------------------
421-
if (nextCell.cellKind === CellKind.Markup && nextCellEntry.level !== 7) {
421+
if (nextCell.cellKind === CellKind.Markup && nextCellEntry.level < 7) {
422422
const sectionBottom = notebookCellList.getCellViewScrollTop(nextCell);
423423
const currentSectionStickyHeight = NotebookStickyScroll.computeStickyHeight(cellEntry);
424424
const nextSectionStickyHeight = NotebookStickyScroll.computeStickyHeight(nextCellEntry);

‎src/vs/workbench/contrib/notebook/common/notebookCommon.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,10 @@ export const NotebookSetting = {
947947
confirmDeleteRunningCell: 'notebook.confirmDeleteRunningCell',
948948
remoteSaving: 'notebook.experimental.remoteSave',
949949
gotoSymbolsAllSymbols: 'notebook.gotoSymbols.showAllSymbols',
950+
outlineShowMarkdownHeadersOnly: 'notebook.outline.showMarkdownHeadersOnly',
951+
outlineShowCodeCells: 'notebook.outline.showCodeCells',
952+
outlineShowCodeCellSymbols: 'notebook.outline.showCodeCellSymbols',
953+
breadcrumbsShowCodeCells: 'notebook.breadcrumbs.showCodeCells',
950954
scrollToRevealCell: 'notebook.scrolling.revealNextCellOnExecute',
951955
anchorToFocusedCell: 'notebook.scrolling.experimental.anchorToFocusedCell',
952956
cellChat: 'notebook.experimental.cellChat',

0 commit comments

Comments
 (0)