📓 support layout logging. (#171692)

* Turn on mru kernel picker.

* 📓 logging support.

* Logging for kernel detection

* set input collapse container default display

* fix unit test.

src/vs/workbench/contrib/notebook/browser/contrib/kernelDetection/notebookKernelDetection.ts

@@ -9,6 +9,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
 import { Registry } from 'vs/platform/registry/common/platform';
 import { Extensions as WorkbenchExtensions, IWorkbenchContribution, IWorkbenchContributionsRegistry } from 'vs/workbench/common/contributions';
 import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
+import { INotebookLoggingService } from 'vs/workbench/contrib/notebook/common/notebookLoggingService';
 import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
 import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle';
 
@@ -19,6 +20,7 @@ class NotebookKernelDetection extends Disposable implements IWorkbenchContributi
 		@INotebookKernelService private readonly _notebookKernelService: INotebookKernelService,
 		@IExtensionService private readonly _extensionService: IExtensionService,
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
+		@INotebookLoggingService private readonly _notebookLoggingService: INotebookLoggingService
 	) {
 		super();
 
@@ -45,6 +47,11 @@ class NotebookKernelDetection extends Disposable implements IWorkbenchContributi
 					// parse the event to get the notebook type
 					const notebookType = e.event.substring('onNotebook:'.length);
 
+					if (notebookType === '*') {
+						// ignore
+						return;
+					}
+
 					let shouldStartDetection = false;
 
 					const extensionStatus = this._extensionService.getExtensionsStatus();
@@ -59,6 +66,7 @@ class NotebookKernelDetection extends Disposable implements IWorkbenchContributi
 					});
 
 					if (shouldStartDetection && !this._detectionMap.has(notebookType)) {
+						this._notebookLoggingService.log('KernelDetection', `start extension activation for ${notebookType}`);
 						const task = this._notebookKernelService.registerNotebookKernelDetectionTask({
 							notebookType: notebookType
 						});
@@ -77,11 +85,18 @@ class NotebookKernelDetection extends Disposable implements IWorkbenchContributi
 
 				// activation state might not be updated yet, postpone to next frame
 				timer = setTimeout(() => {
+					const taskToDelete: string[] = [];
 					for (const [notebookType, task] of this._detectionMap) {
 						if (this._extensionService.activationEventIsDone(`onNotebook:${notebookType}`)) {
+							this._notebookLoggingService.log('KernelDetection', `finish extension activation for ${notebookType}`);
+							taskToDelete.push(notebookType);
 							task.dispose();
 						}
 					}
+
+					taskToDelete.forEach(notebookType => {
+						this._detectionMap.delete(notebookType);
+					});
 				});
 			}));
 

src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts

@@ -107,6 +107,8 @@ import { NotebookInfo } from 'vs/editor/common/languageFeatureRegistry';
 import { COMMENTEDITOR_DECORATION_KEY } from 'vs/workbench/contrib/comments/browser/commentReply';
 import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService';
 import { NotebookKernelHistoryService } from 'vs/workbench/contrib/notebook/browser/services/notebookKernelHistoryServiceImpl';
+import { INotebookLoggingService } from 'vs/workbench/contrib/notebook/common/notebookLoggingService';
+import { NotebookLoggingService } from 'vs/workbench/contrib/notebook/browser/services/notebookLoggingServiceImpl';
 
 /*--------------------------------------------------------------------------------------------- */
 
@@ -710,6 +712,7 @@ registerSingleton(INotebookExecutionService, NotebookExecutionService, Instantia
 registerSingleton(INotebookExecutionStateService, NotebookExecutionStateService, InstantiationType.Delayed);
 registerSingleton(INotebookRendererMessagingService, NotebookRendererMessagingService, InstantiationType.Delayed);
 registerSingleton(INotebookKeymapService, NotebookKeymapService, InstantiationType.Delayed);
+registerSingleton(INotebookLoggingService, NotebookLoggingService, InstantiationType.Delayed);
 
 const schemas: IJSONSchemaMap = {};
 function isConfigurationPropertySchema(x: IConfigurationPropertySchema | { [path: string]: IConfigurationPropertySchema }): x is IConfigurationPropertySchema {
@@ -927,6 +930,13 @@ configurationRegistry.registerConfiguration({
 		[NotebookSetting.outputScrolling]: {
 			markdownDescription: nls.localize('notebook.outputScrolling', "Use a scrollable region for notebook output when longer than the limit"),
 			type: 'boolean',
+			tags: ['notebookLayout'],
+			default: false
+		},
+		[NotebookSetting.logging]: {
+			markdownDescription: nls.localize('notebook.logging', "Enable logging for notebook support."),
+			type: 'boolean',
+			tags: ['notebookLayout'],
 			default: false
 		}
 	}

src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts

@@ -84,10 +84,10 @@ import { EditorExtensionsRegistry } from 'vs/editor/browser/editorExtensions';
 import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
 import { NotebookPerfMarks } from 'vs/workbench/contrib/notebook/common/notebookPerformance';
 import { BaseCellEditorOptions } from 'vs/workbench/contrib/notebook/browser/viewModel/cellEditorOptions';
-import { ILogService } from 'vs/platform/log/common/log';
 import { FloatingClickMenu } from 'vs/workbench/browser/codeeditor';
 import { IDimension } from 'vs/editor/common/core/dimension';
 import { CellFindMatchModel } from 'vs/workbench/contrib/notebook/browser/contrib/find/findModel';
+import { INotebookLoggingService } from 'vs/workbench/contrib/notebook/common/notebookLoggingService';
 
 const $ = DOM.$;
 
@@ -287,7 +287,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
 		@INotebookExecutionService private readonly notebookExecutionService: INotebookExecutionService,
 		@INotebookExecutionStateService notebookExecutionStateService: INotebookExecutionStateService,
 		@IEditorProgressService private readonly editorProgressService: IEditorProgressService,
-		@ILogService private readonly logService: ILogService
+		@INotebookLoggingService readonly logService: INotebookLoggingService,
 	) {
 		super();
 
@@ -1504,15 +1504,15 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
 
 	private async _warmupWithMarkdownRenderer(viewModel: NotebookViewModel, viewState: INotebookEditorViewState | undefined) {
 
-		this.logService.trace('NotebookEditorWidget.warmup', this.viewModel?.uri.toString());
+		this.logService.log('NotebookEditorWidget', 'warmup ' + this.viewModel?.uri.toString());
 		await this._resolveWebview();
-		this.logService.trace('NotebookEditorWidget.warmup - webview resolved');
+		this.logService.log('NotebookEditorWidget', 'warmup - webview resolved');
 
 		// make sure that the webview is not visible otherwise users will see pre-rendered markdown cells in wrong position as the list view doesn't have a correct `top` offset yet
 		this._webview!.element.style.visibility = 'hidden';
 		// warm up can take around 200ms to load markdown libraries, etc.
 		await this._warmupViewport(viewModel, viewState);
-		this.logService.trace('NotebookEditorWidget.warmup - viewport warmed up');
+		this.logService.log('NotebookEditorWidget', 'warmup - viewport warmed up');
 
 		// todo@rebornix @mjbvz, is this too complicated?
 
@@ -1531,7 +1531,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
 		this._list.scrollTop = viewState?.scrollPosition?.top ?? 0;
 		this._debug('finish initial viewport warmup and view state restore.');
 		this._webview!.element.style.visibility = 'visible';
-		this.logService.trace('NotebookEditorWidget.warmup - list view model attached, set to visible');
+		this.logService.log('NotebookEditorWidget', 'warmup - list view model attached, set to visible');
 	}
 
 	private async _warmupViewport(viewModel: NotebookViewModel, viewState: INotebookEditorViewState | undefined) {

src/vs/workbench/contrib/notebook/browser/services/notebookKernelHistoryServiceImpl.ts

@@ -11,6 +11,7 @@ import { Action2, registerAction2 } from 'vs/platform/actions/common/actions';
 import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
 import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
 import { INotebookKernel, INotebookKernelHistoryService, INotebookKernelService, INotebookTextModelLike } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
+import { INotebookLoggingService } from 'vs/workbench/contrib/notebook/common/notebookLoggingService';
 
 interface ISerializedKernelsListPerType {
 	entries: string[];
@@ -29,7 +30,8 @@ export class NotebookKernelHistoryService extends Disposable implements INoteboo
 	private _mostRecentKernelsMap: { [key: string]: LinkedMap<string, string> } = {};
 
 	constructor(@IStorageService private readonly _storageService: IStorageService,
-		@INotebookKernelService private readonly _notebookKernelService: INotebookKernelService) {
+		@INotebookKernelService private readonly _notebookKernelService: INotebookKernelService,
+		@INotebookLoggingService private readonly _notebookLoggingService: INotebookLoggingService) {
 		super();
 
 		this._loadState();
@@ -42,10 +44,10 @@ export class NotebookKernelHistoryService extends Disposable implements INoteboo
 		const selectedKernel = allAvailableKernels.selected;
 		// We will suggest the only kernel
 		const suggested = allAvailableKernels.all.length === 1 ? allAvailableKernels.all[0] : undefined;
-
+		this._notebookLoggingService.log('History', `getMatchingKernels: ${allAvailableKernels.all.length} kernels available for ${notebook.uri.path}. Selected: ${allAvailableKernels.selected?.label}. Suggested: ${suggested?.label}`);
 		const mostRecentKernelIds = this._mostRecentKernelsMap[notebook.viewType] ? [...this._mostRecentKernelsMap[notebook.viewType].values()] : [];
-
 		const all = mostRecentKernelIds.map(kernelId => allKernels.find(kernel => kernel.id === kernelId)).filter(kernel => !!kernel) as INotebookKernel[];
+		this._notebookLoggingService.log('History', `mru: ${mostRecentKernelIds.length} kernels in history, ${all.length} registered already.`);
 
 		return {
 			selected: selectedKernel ?? suggested,

src/vs/workbench/contrib/notebook/browser/services/notebookLoggingServiceImpl.ts

@@ -0,0 +1,67 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as nls from 'vs/nls';
+import { Disposable } from 'vs/base/common/lifecycle';
+import { INotebookLoggingService } from 'vs/workbench/contrib/notebook/common/notebookLoggingService';
+import { Registry } from 'vs/platform/registry/common/platform';
+import { IOutputChannelRegistry, IOutputService, Extensions as OutputExt, IOutputChannel } from 'vs/workbench/services/output/common/output';
+import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
+
+
+export class NotebookLoggingService extends Disposable implements INotebookLoggingService {
+	_serviceBrand: undefined;
+
+	static ID: string = 'notebook';
+	private _enabled: boolean = false;
+	private _outputChannel: IOutputChannel | undefined = undefined;
+
+	constructor(
+		@IOutputService private readonly _outputService: IOutputService,
+		@IConfigurationService private readonly _configurationService: IConfigurationService
+	) {
+		super();
+
+		this._enabled = this._configurationService.getValue<boolean>('notebook.logging');
+
+		this._register(this._configurationService.onDidChangeConfiguration(e => {
+			if (e.affectsConfiguration('notebook.logging')) {
+				this._enabled = this._configurationService.getValue<boolean>('notebook.logging');
+			}
+		}));
+	}
+
+	private _getOrCreateOutputChannel(): IOutputChannel {
+		if (this._outputChannel) {
+			return this._outputChannel;
+		}
+		const channel = this._outputService.getChannel(NotebookLoggingService.ID);
+
+		if (channel) {
+			this._outputChannel = channel;
+			return channel;
+		}
+		const outputChannelRegistry = Registry.as<IOutputChannelRegistry>(OutputExt.OutputChannels);
+		outputChannelRegistry.registerChannel({ id: NotebookLoggingService.ID, label: nls.localize('notebook.log', "Notebooks"), log: false });
+
+		this._outputChannel = this._outputService.getChannel(NotebookLoggingService.ID);
+		if (!this._outputChannel) {
+			throw new Error('output channel not found');
+		}
+
+		return this._outputChannel;
+	}
+
+
+	log(category: string, output: string): void {
+		if (!this._enabled) {
+			return;
+		}
+
+		const channel = this._getOrCreateOutputChannel();
+		channel.append(`[${category}] ${output}\n`);
+	}
+}
+

src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts

@@ -142,6 +142,7 @@ export class MarkupCellRenderer extends AbstractCellRenderer implements IListRen
 		const codeInnerContent = DOM.append(container, $('.cell.code'));
 		const editorPart = DOM.append(codeInnerContent, $('.cell-editor-part'));
 		const cellInputCollapsedContainer = DOM.append(codeInnerContent, $('.input-collapse-container'));
+		cellInputCollapsedContainer.style.display = 'none';
 		const editorContainer = DOM.append(editorPart, $('.cell-editor-container'));
 		editorPart.style.display = 'none';
 		const cellCommentPartContainer = DOM.append(container, $('.cell-comment-container'));
@@ -259,6 +260,7 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende
 		const cellContainer = DOM.append(container, $('.cell.code'));
 		const runButtonContainer = DOM.append(cellContainer, $('.run-button-container'));
 		const cellInputCollapsedContainer = DOM.append(cellContainer, $('.input-collapse-container'));
+		cellInputCollapsedContainer.style.display = 'none';
 		const executionOrderLabel = DOM.append(focusIndicatorLeft.domNode, $('div.execution-count-label'));
 		executionOrderLabel.title = localize('cellExecutionOrderCountLabel', 'Execution Order');
 		const editorPart = DOM.append(cellContainer, $('.cell-editor-part'));

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

@@ -929,6 +929,7 @@ export const NotebookSetting = {
 	outputFontFamily: 'notebook.outputFontFamily',
 	kernelPickerType: 'notebook.kernelPicker.type',
 	outputScrolling: 'notebook.experimental.outputScrolling',
+	logging: 'notebook.logging',
 } as const;
 
 export const enum CellStatusbarAlignment {

src/vs/workbench/contrib/notebook/common/notebookLoggingService.ts

@@ -0,0 +1,13 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
+
+export const INotebookLoggingService = createDecorator<INotebookLoggingService>('INotebookLoggingService');
+
+export interface INotebookLoggingService {
+	readonly _serviceBrand: undefined;
+	log(category: string, output: string): void;
+}

src/vs/workbench/contrib/notebook/test/browser/notebookKernelHistory.test.ts

@@ -19,6 +19,7 @@ import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/languages/modesRegistry'
 import { IMenu, IMenuService } from 'vs/platform/actions/common/actions';
 import { NotebookKernelHistoryService } from 'vs/workbench/contrib/notebook/browser/services/notebookKernelHistoryServiceImpl';
 import { IStorageService, IWillSaveStateEvent, StorageScope } from 'vs/platform/storage/common/storage';
+import { INotebookLoggingService } from 'vs/workbench/contrib/notebook/common/notebookLoggingService';
 
 suite('NotebookKernelHistoryService', () => {
 
@@ -84,6 +85,10 @@ suite('NotebookKernelHistoryService', () => {
 			}
 		});
 
+		instantiationService.stub(INotebookLoggingService, new class extends mock<INotebookLoggingService>() {
+			override log() { }
+		});
+
 		const kernelHistoryService = instantiationService.createInstance(NotebookKernelHistoryService);
 
 		let info = kernelHistoryService.getKernels({ uri: u1, viewType: 'foo' });
@@ -130,6 +135,10 @@ suite('NotebookKernelHistoryService', () => {
 			}
 		});
 
+		instantiationService.stub(INotebookLoggingService, new class extends mock<INotebookLoggingService>() {
+			override log() { }
+		});
+
 		const kernelHistoryService = instantiationService.createInstance(NotebookKernelHistoryService);
 		let info = kernelHistoryService.getKernels({ uri: u1, viewType: 'foo' });
 		assert.equal(info.all.length, 1);