Skip to content

Commit

Permalink
move dirty state to NotebookEditorModel.
Browse files Browse the repository at this point in the history
  • Loading branch information
@rebornix
rebornix committed Sep 9, 2020
1 parent 4e70e4a commit 8bc3145
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 61 deletions.
20 changes: 16 additions & 4 deletions src/vs/workbench/api/browser/mainThreadNotebook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/com
import { ACCESSIBLE_NOTEBOOK_DISPLAY_ORDER, CellEditType, CellKind, DisplayOrderKey, ICellEditOperation, ICellRange, IEditor, IMainCellDto, INotebookDocumentFilter, NotebookCellMetadata, NotebookCellOutputsSplice, NotebookCellsChangeType, NotebookDocumentMetadata, NOTEBOOK_DISPLAY_ORDER, TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { IMainNotebookController, INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
import { ExtHostContext, ExtHostNotebookShape, IExtHostContext, INotebookCellStatusBarEntryDto, INotebookDocumentsAndEditorsDelta, INotebookModelAddedData, MainContext, MainThreadNotebookShape, NotebookEditorRevealType, NotebookExtensionDescription } from '../common/extHost.protocol';

class DocumentAndEditorState {
Expand Down Expand Up @@ -147,7 +148,8 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
@IEditorService private readonly editorService: IEditorService,
@IAccessibilityService private readonly accessibilityService: IAccessibilityService,
@ILogService private readonly logService: ILogService,
@INotebookCellStatusBarService private readonly cellStatusBarService: INotebookCellStatusBarService
@INotebookCellStatusBarService private readonly cellStatusBarService: INotebookCellStatusBarService,
@IWorkingCopyService private readonly _workingCopyService: IWorkingCopyService,
) {
super();
this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebook);
Expand Down Expand Up @@ -295,7 +297,15 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
}
: e
);
this._proxy.$acceptModelChanged(textModel.uri, data, textModel.isDirty);

/**
* TODO@rebornix, @jrieken
* When a document is modified, it will trigger onDidChangeContent events.
* The first event listener is this one, which doesn't know if the text model is dirty or not. It can ask `workingCopyService` but get the wrong result
* The second event listener is `NotebookEditorModel`, which will then set `isDirty` to `true`.
* Since `e.transient` decides if the model should be dirty or not, we will use the same logic here.
*/
this._proxy.$acceptModelChanged(textModel.uri, data, !e.transient);
this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { metadata: null });
}));

Expand Down Expand Up @@ -611,9 +621,11 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo

if (textModel) {
textModel.handleUnknownEdit(label, () => {
return this._proxy.$undoNotebook(textModel.viewType, textModel.uri, editId, textModel.isDirty);
const isDirty = this._workingCopyService.isDirty(textModel.uri.with({ scheme: Schemas.vscodeNotebook }));
return this._proxy.$undoNotebook(textModel.viewType, textModel.uri, editId, isDirty);
}, () => {
return this._proxy.$redoNotebook(textModel.viewType, textModel.uri, editId, textModel.isDirty);
const isDirty = this._workingCopyService.isDirty(textModel.uri.with({ scheme: Schemas.vscodeNotebook }));
return this._proxy.$redoNotebook(textModel.viewType, textModel.uri, editId, isDirty);
});
}
}
Expand Down
76 changes: 33 additions & 43 deletions src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,10 @@ class DelayedEmitter {
}

emit(data: {
triggerDirty: { value: boolean } | undefined,
contentChange: { value: NotebookTextModelChangedEvent } | undefined,
}) {
this._increaseVersion();

if (data.triggerDirty) {
this._textModel.setDirty(data.triggerDirty.value);
}

if (data.contentChange) {
this._onDidChangeContent.fire(
{
Expand Down Expand Up @@ -163,10 +158,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
private _operationManager: NotebookOperationManager;
private _eventEmitter: DelayedEmitter;

private _dirty = false;
protected readonly _onDidChangeDirty = this._register(new Emitter<void>());
readonly onDidChangeDirty = this._onDidChangeDirty.event;

constructor(
readonly viewType: string,
readonly supportBackup: boolean,
Expand Down Expand Up @@ -201,17 +192,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
return this._versionId;
}

get isDirty() {
return this._dirty;
}

setDirty(newState: boolean) {
if (this._dirty !== newState) {
this._dirty = newState;
this._onDidChangeDirty.fire();
}
}

createCellTextModel(
source: string,
language: string,
Expand All @@ -238,9 +218,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
this._mapping.set(mainCells[i].handle, mainCells[i]);
const dirtyStateListener = mainCells[i].onDidChangeContent(() => {
this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: {
value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true }
value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true, transient: false }
}
});
});
Expand Down Expand Up @@ -334,8 +313,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
);
const dirtyStateListener = cell.onDidChangeContent(() => {
this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: { value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true } }
contentChange: { value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true, transient: false } }
});
});
this._cellListeners.set(cell.handle, dirtyStateListener);
Expand Down Expand Up @@ -364,13 +342,13 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel

// should be deferred
this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: {
value: {
kind: NotebookCellsChangeType.ModelChange,
versionId: this._versionId,
changes: diffs,
synchronous
synchronous,
transient: false
}
}
});
Expand All @@ -388,11 +366,30 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
redo();
},
});
this.setDirty(true);

this._eventEmitter.emit({
contentChange: {
value: {
kind: NotebookCellsChangeType.Unknown,
transient: false,
synchronous: true,
versionId: this._versionId,
}
}
});
}

handleUnknownChange() {
this.setDirty(true);
this._eventEmitter.emit({
contentChange: {
value: {
kind: NotebookCellsChangeType.Unknown,
transient: false,
synchronous: true,
versionId: this._versionId,
}
}
});
}

createSnapshot(preserveBOM?: boolean): ITextSnapshot {
Expand All @@ -417,8 +414,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
updateNotebookMetadata(metadata: NotebookDocumentMetadata) {
this.metadata = metadata;
this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: { value: { kind: NotebookCellsChangeType.ChangeDocumentMetadata, versionId: this.versionId, metadata: this.metadata, synchronous: true } }
contentChange: { value: { kind: NotebookCellsChangeType.ChangeDocumentMetadata, versionId: this.versionId, metadata: this.metadata, synchronous: true, transient: false } }
});
}

Expand All @@ -427,8 +423,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
this._mapping.set(cells[i].handle, cells[i]);
const dirtyStateListener = cells[i].onDidChangeContent(() => {
this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: { value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true } }
contentChange: { value: { kind: NotebookCellsChangeType.ChangeCellContent, versionId: this.versionId, synchronous: true, transient: false } }
});
});

Expand All @@ -437,7 +432,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel

this._cells.splice(index, 0, ...cells);
this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: {
value: {
kind: NotebookCellsChangeType.ModelChange,
Expand All @@ -448,7 +442,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
cells
]],
synchronous,
endSelections: endSelections
endSelections: endSelections,
transient: false
}
}
});
Expand All @@ -464,8 +459,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
}
this._cells.splice(index, count);
this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: { value: { kind: NotebookCellsChangeType.ModelChange, versionId: this._versionId, changes: [[index, count, []]], synchronous, endSelections } }
contentChange: { value: { kind: NotebookCellsChangeType.ModelChange, versionId: this._versionId, changes: [[index, count, []]], synchronous, endSelections, transient: false } }
});
}

Expand Down Expand Up @@ -513,9 +507,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
}

this._eventEmitter.emit({
triggerDirty: { value: triggerDirtyChange },
contentChange: {
value: { kind: NotebookCellsChangeType.ChangeCellMetadata, versionId: this._versionId, index: this._cells.indexOf(cell), metadata: cell.metadata, synchronous: true }
value: { kind: NotebookCellsChangeType.ChangeCellMetadata, versionId: this._versionId, index: this._cells.indexOf(cell), metadata: cell.metadata, synchronous: true, transient: !triggerDirtyChange }
}
});
}
Expand All @@ -526,8 +519,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
cell.language = languageId;

this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: { value: { kind: NotebookCellsChangeType.ChangeLanguage, versionId: this._versionId, index: this._cells.indexOf(cell), language: languageId, synchronous: true } }
contentChange: { value: { kind: NotebookCellsChangeType.ChangeLanguage, versionId: this._versionId, index: this._cells.indexOf(cell), language: languageId, synchronous: true, transient: false } }
});
}
}
Expand All @@ -539,7 +531,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
cell.spliceNotebookCellOutputs(splices);

this._eventEmitter.emit({
triggerDirty: { value: !this.transientOptions.transientOutputs },
contentChange: {
value: {
kind: NotebookCellsChangeType.Output,
Expand Down Expand Up @@ -602,8 +593,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
const cells = this._cells.splice(index, length);
this._cells.splice(newIdx, 0, ...cells);
this._eventEmitter.emit({
triggerDirty: { value: true },
contentChange: { value: { kind: NotebookCellsChangeType.Move, versionId: this._versionId, index, length, newIdx, cells, synchronous, endSelections } }
contentChange: { value: { kind: NotebookCellsChangeType.Move, versionId: this._versionId, index, length, newIdx, cells, synchronous, endSelections, transient: false } }
});
}

Expand Down
12 changes: 9 additions & 3 deletions src/vs/workbench/contrib/notebook/common/notebookCommon.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ export enum NotebookCellsChangeType {
ChangeCellMetadata = 7,
Output = 8,
ChangeCellContent = 9,
ChangeDocumentMetadata = 10
ChangeDocumentMetadata = 10,
Unknown = 11
}

export interface NotebookCellsInitializeEvent<T> {
Expand Down Expand Up @@ -405,8 +406,13 @@ export interface NotebookDocumentChangeMetadataEvent {
readonly metadata: NotebookDocumentMetadata | undefined;
}

export type NotebookCellsChangedEventDto = NotebookCellsInitializeEvent<IMainCellDto> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent | NotebookCellsModelChangedEvent<IMainCellDto> | NotebookCellsModelMoveEvent<IMainCellDto> | NotebookOutputChangedEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent;
export type NotebookTextModelChangedEvent = (NotebookCellsInitializeEvent<ICell> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent | NotebookCellsModelChangedEvent<ICell> | NotebookCellsModelMoveEvent<ICell> | NotebookOutputChangedEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent) & { synchronous: boolean; endSelections?: number[] };
export interface NotebookDocumentUnknownChangeEvent {
readonly kind: NotebookCellsChangeType.Unknown;
readonly versionId: number;
}

export type NotebookCellsChangedEventDto = NotebookCellsInitializeEvent<IMainCellDto> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent | NotebookCellsModelChangedEvent<IMainCellDto> | NotebookCellsModelMoveEvent<IMainCellDto> | NotebookOutputChangedEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent | NotebookDocumentUnknownChangeEvent;
export type NotebookTextModelChangedEvent = (NotebookCellsInitializeEvent<ICell> | NotebookDocumentChangeMetadataEvent | NotebookCellContentChangeEvent | NotebookCellsModelChangedEvent<ICell> | NotebookCellsModelMoveEvent<ICell> | NotebookOutputChangedEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent | NotebookDocumentUnknownChangeEvent) & { synchronous: boolean; endSelections?: number[]; transient: boolean };

export const enum CellEditType {
Replace = 1,
Expand Down
27 changes: 18 additions & 9 deletions src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
private readonly _name: string;
private readonly _workingCopyResource: URI;

private _dirty = false;

constructor(
readonly resource: URI,
readonly viewType: string,
Expand Down Expand Up @@ -80,6 +82,13 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
return this._notebook;
}

setDirty(newState: boolean) {
if (this._dirty !== newState) {
this._dirty = newState;
this._onDidChangeDirty.fire();
}
}

async backup(): Promise<IWorkingCopyBackup<NotebookDocumentBackupData>> {
if (this._notebook.supportBackup) {
const tokenSource = new CancellationTokenSource();
Expand Down Expand Up @@ -116,7 +125,7 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
const newStats = await this._resolveStats(this.resource);
this._lastResolvedFileStat = newStats;

this._notebook.setDirty(false);
this.setDirty(false);
this._onDidChangeDirty.fire();
}

Expand Down Expand Up @@ -150,19 +159,19 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
if (e.kind !== NotebookCellsChangeType.Initialize) {
this._onDidChangeContent.fire();
}
}));

this._register(this._notebook.onDidChangeDirty(() => {
this._onDidChangeDirty.fire();
if (!e.transient) {
this.setDirty(true);
}
}));

if (forceReloadFromDisk) {
this._notebook.setDirty(false);
this.setDirty(false);
}

if (backupId) {
await this._backupFileService.discardBackup(this._workingCopyResource);
this._notebook.setDirty(true);
this.setDirty(true);
}

return this;
Expand All @@ -173,7 +182,7 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
}

isDirty() {
return this._notebook?.isDirty;
return this._dirty;
}

isUntitled() {
Expand Down Expand Up @@ -225,7 +234,7 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
await this._notebookService.save(this.notebook.viewType, this.notebook.uri, tokenSource.token);
const newStats = await this._resolveStats(this.resource);
this._lastResolvedFileStat = newStats;
this._notebook.setDirty(false);
this.setDirty(false);
return true;
}

Expand All @@ -245,7 +254,7 @@ export class NotebookEditorModel extends EditorModel implements INotebookEditorM
await this._notebookService.saveAs(this.notebook.viewType, this.notebook.uri, targetResource, tokenSource.token);
const newStats = await this._resolveStats(this.resource);
this._lastResolvedFileStat = newStats;
this._notebook.setDirty(false);
this.setDirty(false);
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/notebook/common/notebookEditorModelResolverService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ export class NotebookModelResolverService implements INotebookEditorModelResolve
private static _autoReferenceDirtyModel(model: INotebookEditorModel, ref: () => IDisposable) {

const references = new DisposableStore();
const listener = model.notebook.onDidChangeDirty(() => {
if (model.notebook.isDirty) {
const listener = model.onDidChangeDirty(() => {
if (model.isDirty()) {
references.add(ref());
} else {
references.clear();
Expand Down

0 comments on commit 8bc3145

Please sign in to comment.