From c8f6937166650b52cbea40aeaf8d05047490491d 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 --- .../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 03fb07b3aa..6d655d6001 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'; @@ -101,7 +101,8 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange @Inject(IInstantiationService) private _instantiationService: IInstantiationService, @Inject(IModelService) private _modelService: IModelService, @Inject(IModeService) private _modeService: IModeService, - @Inject(IConfigurationService) private _configurationService: IConfigurationService + @Inject(IConfigurationService) private _configurationService: IConfigurationService, + @Inject(forwardRef(() => ChangeDetectorRef)) private _changeRef: ChangeDetectorRef ) { super(); this._cellToggleMoreActions = this._instantiationService.createInstance(CellToggleMoreActions); @@ -159,7 +160,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(); @@ -197,6 +205,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; }