From b475311f8587f19cb8f85d05fdbf167b40b98cc2 Mon Sep 17 00:00:00 2001 From: Kevin Cunnane Date: Thu, 11 Apr 2019 16:39:54 -0700 Subject: [PATCH] Fix #4500 Untitled notebook reopen doesn't show dirty (#5005) * Fix 2 notebook issues - Do not create notebook model twice on start - Do not cause disposed warnings due to markdown cell deserialization * Fix notebook dirty on open issue Before model is resolved we weren't getting dirty events. Solution is to use backing text model until it's ready. Must hook to the dirty event & notify to get the dot to appear # Conflicts: # src/sql/workbench/parts/notebook/cellViews/code.component.ts --- .../notebook/cellViews/code.component.ts | 20 ++++++++++-- .../workbench/parts/notebook/notebookInput.ts | 31 ++++++++++++++++--- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/sql/workbench/parts/notebook/cellViews/code.component.ts b/src/sql/workbench/parts/notebook/cellViews/code.component.ts index fe7d76485e..fc3aa198d4 100644 --- a/src/sql/workbench/parts/notebook/cellViews/code.component.ts +++ b/src/sql/workbench/parts/notebook/cellViews/code.component.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import 'vs/css!./code'; -import { OnInit, Component, Input, Inject, ElementRef, ViewChild, Output, EventEmitter, OnChanges, SimpleChange } from '@angular/core'; +import { OnInit, Component, Input, Inject, ElementRef, ViewChild, Output, EventEmitter, OnChanges, SimpleChange, forwardRef, ChangeDetectorRef } from '@angular/core'; import { AngularDisposable } from 'sql/base/node/lifecycle'; import { QueryTextEditor } from 'sql/parts/modelComponents/queryTextEditor'; @@ -103,7 +103,8 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange @Inject(IModelService) private _modelService: IModelService, @Inject(IModeService) private _modeService: IModeService, @Inject(IContextMenuService) private contextMenuService: IContextMenuService, - @Inject(IConfigurationService) private _configurationService: IConfigurationService + @Inject(IConfigurationService) private _configurationService: IConfigurationService, + @Inject(forwardRef(() => ChangeDetectorRef)) private _changeRef: ChangeDetectorRef ) { super(); this._cellToggleMoreActions = this._instantiationService.createInstance(CellToggleMoreActions); @@ -161,7 +162,14 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange && this.cellModel.cellUri; } + private get destroyed(): boolean{ + return !!(this._changeRef['destroyed']); + } + ngAfterContentInit(): void { + if (this.destroyed) { + return; + } this.createEditor(); this._register(DOM.addDisposableListener(window, DOM.EventType.RESIZE, e => { this._layoutEmitter.fire(); @@ -199,6 +207,14 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange let overrideEditorSetting = this._configurationService.getValue(OVERRIDE_EDITOR_THEMING_SETTING); this._editor.hideLineNumbers = (overrideEditorSetting && this.cellModel.cellType === CellTypes.Markdown); + + if (this.destroyed) { + // At this point, we may have been disposed (scenario: restoring markdown cell in preview mode). + // Exiting early to avoid warnings on registering already disposed items, which causes some churning + // due to re-disposing things. + // There's no negative impact as at this point the component isn't visible (it was removed from the DOM) + return; + } this._register(this._editor); this._register(this._editorInput); this._register(this._editorModel.onDidChangeContent(e => { diff --git a/src/sql/workbench/parts/notebook/notebookInput.ts b/src/sql/workbench/parts/notebook/notebookInput.ts index a211f8eb03..284b54b83c 100644 --- a/src/sql/workbench/parts/notebook/notebookInput.ts +++ b/src/sql/workbench/parts/notebook/notebookInput.ts @@ -26,6 +26,7 @@ import { LocalContentManager } from 'sql/workbench/services/notebook/node/localC import { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; import { UntitledEditorInput } from 'vs/workbench/common/editor/untitledEditorInput'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; +import { IDisposable } from 'vs/base/common/lifecycle'; export type ModeViewSaveHandler = (handle: number) => Thenable; @@ -61,7 +62,7 @@ export class NotebookEditorModel extends EditorModel { return model.getValue(); } - get isDirty(): boolean { + isDirty(): boolean { return this.textEditorModel.isDirty(); } @@ -135,6 +136,7 @@ export class NotebookInput extends EditorInput { private _untitledEditorModel: UntitledEditorModel; private _contentManager: IContentManager; private _providersLoaded: Promise; + private _dirtyListener: IDisposable; constructor(private _title: string, private resource: URI, @@ -148,6 +150,9 @@ export class NotebookInput extends EditorInput { this.resource = resource; this._standardKernels = []; this._providersLoaded = this.assignProviders(); + if (this._textInput) { + this.hookDirtyListener(this._textInput.onDidChangeDirty, () => this._onDidChangeDirty.fire()); + } } public get textInput(): UntitledEditorInput { @@ -256,7 +261,7 @@ export class NotebookInput extends EditorInput { } async resolve(): Promise { - if (this._model && this._model.isModelCreated()) { + if (this._model) { return Promise.resolve(this._model); } else { let textOrUntitledEditorModel: UntitledEditorModel | IEditorModel; @@ -268,11 +273,27 @@ export class NotebookInput extends EditorInput { textOrUntitledEditorModel = await textEditorModelReference.object.load(); } this._model = this.instantiationService.createInstance(NotebookEditorModel, this.resource, textOrUntitledEditorModel); - this._model.onDidChangeDirty(() => this._onDidChangeDirty.fire()); + this.hookDirtyListener(this._model.onDidChangeDirty, () => this._onDidChangeDirty.fire()); return this._model; } } + private hookDirtyListener(dirtyEvent: Event, listener: (e: any) => void): void { + let disposable = dirtyEvent(listener); + if (this._dirtyListener) { + this._dirtyListener.dispose(); + } else { + this._register({ + dispose: () => { + if (this._dirtyListener) { + this._dirtyListener.dispose(); + } + } + }); + } + this._dirtyListener = disposable; + } + private async assignProviders(): Promise { await this.extensionService.whenInstalledExtensionsRegistered(); let providerIds: string[] = getProvidersForFileName(this._title, this.notebookService); @@ -318,7 +339,9 @@ export class NotebookInput extends EditorInput { */ isDirty(): boolean { if (this._model) { - return this._model.isDirty; + return this._model.isDirty(); + } else if (this._textInput) { + return this._textInput.isDirty(); } return false; }