Skip to content

Commit 24af01a

Browse files
connor4312connor4312
committed
notebooks: better memorization of preferred renderers
Fixes microsoft#125876
1 parent baa380d commit 24af01a

File tree

5 files changed

+75
-79
lines changed

5 files changed

+75
-79
lines changed

‎src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts

Lines changed: 70 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { Lazy } from 'vs/base/common/lazy';
1111
import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
1212
import { ResourceMap } from 'vs/base/common/map';
1313
import { Schemas } from 'vs/base/common/network';
14+
import { isDefined } from 'vs/base/common/types';
1415
import { URI } from 'vs/base/common/uri';
1516
import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService';
1617
import { IEditorOptions } from 'vs/editor/common/config/editorOptions';
@@ -264,12 +265,16 @@ export class NotebookProviderInfoStore extends Disposable {
264265
}
265266

266267
export class NotebookOutputRendererInfoStore {
267-
private readonly contributedRenderers = new Map<string, NotebookOutputRendererInfo>();
268+
private readonly contributedRenderers = new Map</* rendererId */ string, NotebookOutputRendererInfo>();
268269
private readonly preferredMimetypeMemento: Memento;
269-
private readonly preferredMimetype = new Lazy(() => this.preferredMimetypeMemento.getMemento(StorageScope.WORKSPACE, StorageTarget.USER));
270+
private readonly preferredMimetype = new Lazy<{ [notebookType: string]: { [mimeType: string]: /* rendererId */ string } }>(
271+
() => this.preferredMimetypeMemento.getMemento(StorageScope.WORKSPACE, StorageTarget.USER));
270272

271-
constructor(@IStorageService storageService: IStorageService) {
272-
this.preferredMimetypeMemento = new Memento('workbench.editor.notebook.preferredRenderer', storageService);
273+
constructor(
274+
@IStorageService storageService: IStorageService,
275+
@IWorkspaceTrustManagementService private readonly workspaceTrustManagementService: IWorkspaceTrustManagementService,
276+
) {
277+
this.preferredMimetypeMemento = new Memento('workbench.editor.notebook.preferredRenderer2', storageService);
273278
}
274279

275280
clear() {
@@ -292,25 +297,65 @@ export class NotebookOutputRendererInfoStore {
292297
}
293298

294299
/** Update and remember the preferred renderer for the given mimetype in this workspace */
295-
setPreferred(mimeType: string, rendererId: string) {
296-
this.preferredMimetype.getValue()[mimeType] = rendererId;
300+
setPreferred(notebookProviderInfo: NotebookProviderInfo, mimeType: string, rendererId: string) {
301+
const mementoObj = this.preferredMimetype.getValue();
302+
const forNotebook = mementoObj[notebookProviderInfo.id];
303+
if (forNotebook) {
304+
forNotebook[mimeType] = rendererId;
305+
} else {
306+
mementoObj[notebookProviderInfo.id] = { [mimeType]: rendererId };
307+
}
308+
297309
this.preferredMimetypeMemento.saveMemento();
298310
}
299311

300-
getContributedRenderer(mimeType: string, kernelProvides: readonly string[] | undefined): NotebookOutputRendererInfo[] {
301-
const preferred = this.preferredMimetype.getValue()[mimeType];
302-
const possible = Array.from(this.contributedRenderers.values())
303-
.map(renderer => ({
304-
renderer,
305-
score: kernelProvides === undefined
312+
findBestRenderers(notebookProviderInfo: NotebookProviderInfo | undefined, mimeType: string, kernelProvides: readonly string[] | undefined): IOrderedMimeType[] {
313+
314+
const enum ReuseOrder {
315+
PreviouslySelected = 1 << 8,
316+
SameExtensionAsNotebook = 2 << 8,
317+
BuiltIn = 3 << 8,
318+
OtherRenderer = 4 << 8
319+
}
320+
321+
const preferred = notebookProviderInfo && this.preferredMimetype.getValue()[notebookProviderInfo.id]?.[mimeType];
322+
const renderers: { ordered: IOrderedMimeType, score: number }[] = Array.from(this.contributedRenderers.values())
323+
.map(renderer => {
324+
const ownScore = kernelProvides === undefined
306325
? renderer.matchesWithoutKernel(mimeType)
307-
: renderer.matches(mimeType, kernelProvides),
308-
}))
309-
.sort((a, b) => a.score - b.score)
310-
.filter(r => r.score !== NotebookRendererMatch.Never)
311-
.map(r => r.renderer);
326+
: renderer.matches(mimeType, kernelProvides);
327+
328+
if (ownScore === NotebookRendererMatch.Never) {
329+
return undefined;
330+
}
331+
332+
const reuseScore = preferred === renderer.id
333+
? ReuseOrder.PreviouslySelected
334+
: renderer.extensionId.value === notebookProviderInfo?.extension?.value
335+
? ReuseOrder.SameExtensionAsNotebook
336+
: ReuseOrder.BuiltIn;
337+
return {
338+
ordered: { mimeType, rendererId: renderer.id, isTrusted: true },
339+
score: reuseScore | ownScore,
340+
};
341+
}).filter(isDefined);
342+
343+
if (mimeTypeSupportedByCore(mimeType)) {
344+
renderers.push({
345+
score: ReuseOrder.BuiltIn << 8,
346+
ordered: {
347+
mimeType,
348+
rendererId: BUILTIN_RENDERER_ID,
349+
isTrusted: mimeTypeIsAlwaysSecure(mimeType) || this.workspaceTrustManagementService.isWorkspaceTrusted()
350+
}
351+
});
352+
}
353+
354+
if (renderers.length === 0) {
355+
return [{ mimeType, rendererId: RENDERER_NOT_AVAILABLE, isTrusted: true }];
356+
}
312357

313-
return preferred ? possible.sort((a, b) => (a.id === preferred ? -1 : 0) + (b.id === preferred ? 1 : 0)) : possible;
358+
return renderers.sort((a, b) => a.score - b.score).map(r => r.ordered);
314359
}
315360
}
316361

@@ -376,7 +421,6 @@ export class NotebookService extends Disposable implements INotebookService {
376421
@IInstantiationService private readonly _instantiationService: IInstantiationService,
377422
@ICodeEditorService private readonly _codeEditorService: ICodeEditorService,
378423
@IConfigurationService private readonly configurationService: IConfigurationService,
379-
@IWorkspaceTrustManagementService private readonly workspaceTrustManagementService: IWorkspaceTrustManagementService,
380424
) {
381425
super();
382426

@@ -569,8 +613,11 @@ export class NotebookService extends Disposable implements INotebookService {
569613
return this._notebookRenderersInfoStore.get(rendererId);
570614
}
571615

572-
updateMimePreferredRenderer(mimeType: string, rendererId: string): void {
573-
this._notebookRenderersInfoStore.setPreferred(mimeType, rendererId);
616+
updateMimePreferredRenderer(viewType: string, mimeType: string, rendererId: string): void {
617+
const info = this.notebookProviderInfoStore.get(viewType);
618+
if (info) {
619+
this._notebookRenderersInfoStore.setPreferred(info, mimeType, rendererId);
620+
}
574621
}
575622

576623
getRenderers(): INotebookRendererInfo[] {
@@ -620,58 +667,9 @@ export class NotebookService extends Disposable implements INotebookService {
620667

621668
const coreDisplayOrder = this._displayOrder;
622669
const sorted = sortMimeTypes(mimeTypes, coreDisplayOrder?.userOrder ?? [], coreDisplayOrder?.defaultOrder ?? []);
670+
const notebookProviderInfo = this.notebookProviderInfoStore.get(textModel.viewType);
623671

624-
const orderMimeTypes: IOrderedMimeType[] = [];
625-
626-
sorted.forEach(mimeType => {
627-
const handlers = this._findBestMatchedRenderer(mimeType, kernelProvides);
628-
629-
if (handlers.length) {
630-
const handler = handlers[0];
631-
632-
orderMimeTypes.push({
633-
mimeType: mimeType,
634-
rendererId: handler.id,
635-
isTrusted: true
636-
});
637-
638-
for (let i = 1; i < handlers.length; i++) {
639-
orderMimeTypes.push({
640-
mimeType: mimeType,
641-
rendererId: handlers[i].id,
642-
isTrusted: true
643-
});
644-
}
645-
646-
if (mimeTypeSupportedByCore(mimeType)) {
647-
orderMimeTypes.push({
648-
mimeType: mimeType,
649-
rendererId: BUILTIN_RENDERER_ID,
650-
isTrusted: mimeTypeIsAlwaysSecure(mimeType) || this.workspaceTrustManagementService.isWorkspaceTrusted()
651-
});
652-
}
653-
} else {
654-
if (mimeTypeSupportedByCore(mimeType)) {
655-
orderMimeTypes.push({
656-
mimeType: mimeType,
657-
rendererId: BUILTIN_RENDERER_ID,
658-
isTrusted: mimeTypeIsAlwaysSecure(mimeType) || this.workspaceTrustManagementService.isWorkspaceTrusted()
659-
});
660-
} else {
661-
orderMimeTypes.push({
662-
mimeType: mimeType,
663-
rendererId: RENDERER_NOT_AVAILABLE,
664-
isTrusted: true
665-
});
666-
}
667-
}
668-
});
669-
670-
return orderMimeTypes;
671-
}
672-
673-
private _findBestMatchedRenderer(mimeType: string, kernelProvides: readonly string[] | undefined): readonly NotebookOutputRendererInfo[] {
674-
return this._notebookRenderersInfoStore.getContributedRenderer(mimeType, kernelProvides);
672+
return sorted.flatMap(mimeType => this._notebookRenderersInfoStore.findBestRenderers(notebookProviderInfo, mimeType, kernelProvides));
675673
}
676674

677675
getContributedNotebookTypes(resource?: URI): readonly NotebookProviderInfo[] {

‎src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ export class CellOutputElement extends Disposable {
487487
this.viewCell.updateOutputMinHeight(this.viewCell.layoutInfo.outputTotalHeight);
488488

489489
const { mimeType, rendererId } = mimeTypes[pick.index];
490-
this.notebookService.updateMimePreferredRenderer(mimeType, rendererId);
490+
this.notebookService.updateMimePreferredRenderer(notebookTextModel.viewType, mimeType, rendererId);
491491
this.render(nextElement as HTMLElement);
492492
this._validateFinalOutputHeight(false);
493493
this._relayoutCell();

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ export interface TransientOptions {
109109
transientDocumentMetadata: TransientDocumentMetadata;
110110
}
111111

112-
113-
114112
/** Note: enum values are used for sorting */
115113
export const enum NotebookRendererMatch {
116114
/** Renderer has a hard dependency on an available kernel */

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ export class NotebookOutputRendererInfo implements INotebookRendererInfo {
8181
this.messaging = descriptor.requiresMessaging ?? RendererMessagingSpec.Never;
8282
}
8383

84-
get dependencies(): string[] {
84+
public get dependencies(): string[] {
8585
return this.hardDependencies.values();
8686
}
8787

88-
matchesWithoutKernel(mimeType: string) {
88+
public matchesWithoutKernel(mimeType: string) {
8989
if (!this.matchesMimeTypeOnly(mimeType)) {
9090
return NotebookRendererMatch.Never;
9191
}
@@ -101,7 +101,7 @@ export class NotebookOutputRendererInfo implements INotebookRendererInfo {
101101
return NotebookRendererMatch.Pure;
102102
}
103103

104-
matches(mimeType: string, kernelProvides: ReadonlyArray<string>) {
104+
public matches(mimeType: string, kernelProvides: ReadonlyArray<string>) {
105105
if (!this.matchesMimeTypeOnly(mimeType)) {
106106
return NotebookRendererMatch.Never;
107107
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export interface INotebookService {
7676
getRenderers(): INotebookRendererInfo[];
7777

7878
/** Updates the preferred renderer for the given mimetype in the workspace. */
79-
updateMimePreferredRenderer(mimeType: string, rendererId: string): void;
79+
updateMimePreferredRenderer(viewType: string, mimeType: string, rendererId: string): void;
8080

8181
createNotebookTextModel(viewType: string, uri: URI, data: NotebookData, transientOptions: TransientOptions): NotebookTextModel;
8282
getNotebookTextModel(uri: URI): NotebookTextModel | undefined;

0 commit comments

Comments
 (0)