Skip to content

Commit

Permalink
Fix #96007.
Browse files Browse the repository at this point in the history
  • Loading branch information
@rebornix
rebornix committed Apr 23, 2020
1 parent ca16acf commit bf25559
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 21 deletions.
17 changes: 8 additions & 9 deletions extensions/vscode-notebook-tests/src/notebook.test.ts
View file
Expand Up @@ -100,16 +100,15 @@ suite('notebook workflow', () => {

// ---- move up and down ---- //

// await vscode.commands.executeCommand('workbench.notebook.cell.moveDown');
// await vscode.commands.executeCommand('workbench.notebook.cell.moveDown');
// activeCell = vscode.notebook.activeNotebookEditor!.selection;

// assert.equal(vscode.notebook.activeNotebookEditor!.document.cells.indexOf(activeCell!), 2);
// assert.equal(vscode.notebook.activeNotebookEditor!.document.cells[0].source, 'test');
// assert.equal(vscode.notebook.activeNotebookEditor!.document.cells[1].source, '');
// assert.equal(vscode.notebook.activeNotebookEditor!.document.cells[2].source, 'test');
// assert.equal(vscode.notebook.activeNotebookEditor!.document.cells[3].source, '');
await vscode.commands.executeCommand('workbench.notebook.cell.moveDown');
await vscode.commands.executeCommand('workbench.notebook.cell.moveDown');
activeCell = vscode.notebook.activeNotebookEditor!.selection;

assert.equal(vscode.notebook.activeNotebookEditor!.document.cells.indexOf(activeCell!), 2);
assert.equal(vscode.notebook.activeNotebookEditor!.document.cells[0].source, 'test');
assert.equal(vscode.notebook.activeNotebookEditor!.document.cells[1].source, '');
assert.equal(vscode.notebook.activeNotebookEditor!.document.cells[2].source, 'test');
assert.equal(vscode.notebook.activeNotebookEditor!.document.cells[3].source, '');

// ---- ---- //

Expand Down
20 changes: 15 additions & 5 deletions src/vs/workbench/api/common/extHostNotebook.ts
View file
Expand Up @@ -13,7 +13,7 @@ import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'
import { CellKind, CellOutputKind, ExtHostNotebookShape, IMainContext, MainContext, MainThreadNotebookShape, NotebookCellOutputsSplice, MainThreadDocumentsShape, INotebookEditorPropertiesChangeData } from 'vs/workbench/api/common/extHost.protocol';
import { ExtHostCommands } from 'vs/workbench/api/common/extHostCommands';
import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors';
import { CellEditType, CellUri, diff, ICellEditOperation, ICellInsertEdit, IErrorOutput, INotebookDisplayOrder, INotebookEditData, IOrderedMimeType, IStreamOutput, ITransformedDisplayOutputDto, mimeTypeSupportedByCore, NotebookCellsChangedEvent, NotebookCellsSplice2, sortMimeTypes, ICellDeleteEdit, notebookDocumentMetadataDefaults } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { CellEditType, CellUri, diff, ICellEditOperation, ICellInsertEdit, IErrorOutput, INotebookDisplayOrder, INotebookEditData, IOrderedMimeType, IStreamOutput, ITransformedDisplayOutputDto, mimeTypeSupportedByCore, NotebookCellsChangedEvent, NotebookCellsSplice2, sortMimeTypes, ICellDeleteEdit, notebookDocumentMetadataDefaults, NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { Disposable as VSCodeDisposable } from './extHostTypes';
import { CancellationToken } from 'vs/base/common/cancellation';
import { ExtHostDocumentData } from 'vs/workbench/api/common/extHostDocumentData';
Expand Down Expand Up @@ -247,7 +247,12 @@ export class ExtHostNotebookDocument extends Disposable implements vscode.Notebo
get isDirty() { return false; }

accpetModelChanged(event: NotebookCellsChangedEvent) {
this.$spliceNotebookCells(event.changes);
if (event.kind === NotebookCellsChangeType.ModelChange) {
this.$spliceNotebookCells(event.changes);
} else if (event.kind === NotebookCellsChangeType.Move) {
this.$moveCell(event.index, event.newIdx);
}

this._versionId = event.versionId;
}

Expand Down Expand Up @@ -289,6 +294,11 @@ export class ExtHostNotebookDocument extends Disposable implements vscode.Notebo
});
}

private $moveCell(index: number, newIdx: number) {
const cells = this.cells.splice(index, 1);
this.cells.splice(newIdx, 0, ...cells);
}

eventuallyUpdateCellOutputs(cell: ExtHostCell, diffs: ISplice<vscode.CellOutput>[]) {
let renderers = new Set<number>();
let outputDtos: NotebookCellOutputsSplice[] = diffs.map(diff => {
Expand Down Expand Up @@ -600,8 +610,8 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN
private readonly _editors = new Map<string, { editor: ExtHostNotebookEditor, onDidReceiveMessage: Emitter<any> }>();
private readonly _notebookOutputRenderers = new Map<number, ExtHostNotebookOutputRenderer>();

private readonly _onDidChangeNotebookDocument = new Emitter<{ document: ExtHostNotebookDocument, changes: NotebookCellsSplice2[] }>();
readonly onDidChangeNotebookDocument: Event<{ document: ExtHostNotebookDocument, changes: NotebookCellsSplice2[] }> = this._onDidChangeNotebookDocument.event;
private readonly _onDidChangeNotebookDocument = new Emitter<{ document: ExtHostNotebookDocument, changes: NotebookCellsChangedEvent[] }>();
readonly onDidChangeNotebookDocument: Event<{ document: ExtHostNotebookDocument, changes: NotebookCellsChangedEvent[] }> = this._onDidChangeNotebookDocument.event;

private _outputDisplayOrder: INotebookDisplayOrder | undefined;

Expand Down Expand Up @@ -801,7 +811,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN
editor.editor.document.accpetModelChanged(event);
this._onDidChangeNotebookDocument.fire({
document: editor.editor.document,
changes: event.changes
changes: [event]
});
}

Expand Down
4 changes: 4 additions & 0 deletions src/vs/workbench/contrib/notebook/browser/notebookEditorInput.ts
View file
Expand Up @@ -69,6 +69,10 @@ export class NotebookEditorModel extends EditorModel {
}
}

moveCellToIdx(index: number, newIdx: number) {
this.notebook.moveCellToIdx(index, newIdx);
}

async save(): Promise<boolean> {
if (this._notebook) {
this._dirty = false;
Expand Down
4 changes: 1 addition & 3 deletions src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts
View file
Expand Up @@ -664,10 +664,8 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD
}

this.viewCells.splice(index, 1);
this._model.deleteCell(index);

this.viewCells!.splice(newIdx, 0, viewCell);
this._model.insertCell(viewCell.model, newIdx);
this._model.moveCellToIdx(index, newIdx);

if (pushedToUndoStack) {
this.undoService.pushElement(new MoveCellEdit(this.uri, index, newIdx, {
Expand Down
23 changes: 21 additions & 2 deletions src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts
View file
Expand Up @@ -7,7 +7,7 @@ import { Emitter, Event } from 'vs/base/common/event';
import { Disposable, IDisposable } from 'vs/base/common/lifecycle';
import { URI } from 'vs/base/common/uri';
import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel';
import { INotebookTextModel, NotebookCellOutputsSplice, NotebookCellTextModelSplice, NotebookDocumentMetadata, NotebookCellMetadata, ICellEditOperation, CellEditType, CellUri, ICellInsertEdit, NotebookCellsChangedEvent, CellKind, IOutput, notebookDocumentMetadataDefaults, diff, ICellDeleteEdit } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookTextModel, NotebookCellOutputsSplice, NotebookCellTextModelSplice, NotebookDocumentMetadata, NotebookCellMetadata, ICellEditOperation, CellEditType, CellUri, ICellInsertEdit, NotebookCellsChangedEvent, CellKind, IOutput, notebookDocumentMetadataDefaults, diff, ICellDeleteEdit, NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon';

function compareRangesUsingEnds(a: [number, number], b: [number, number]): number {
if (a[1] === b[1]) {
Expand Down Expand Up @@ -192,6 +192,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
this._onDidChangeContent.fire();

this._onDidModelChangeProxy.fire({
kind: NotebookCellsChangeType.ModelChange,
versionId: this._versionId, changes: [
[
0,
Expand Down Expand Up @@ -228,6 +229,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
this._onDidChangeContent.fire();
this._increaseVersionId();
this._onDidModelChangeProxy.fire({
kind: NotebookCellsChangeType.ModelChange,
versionId: this._versionId, changes: [
[
index,
Expand Down Expand Up @@ -258,7 +260,24 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
this._onDidChangeContent.fire();

this._increaseVersionId();
this._onDidModelChangeProxy.fire({ versionId: this._versionId, changes: [[index, 1, []]] });
this._onDidModelChangeProxy.fire({ kind: NotebookCellsChangeType.ModelChange, versionId: this._versionId, changes: [[index, 1, []]] });
}

moveCellToIdx(index: number, newIdx: number) {
this.assertIndex(index);
this.assertIndex(newIdx);

const cells = this.cells.splice(index, 1);
this.cells.splice(newIdx, 0, ...cells);

this._increaseVersionId();
this._onDidModelChangeProxy.fire({ kind: NotebookCellsChangeType.Move, versionId: this._versionId, index, newIdx });
}

assertIndex(index: number) {
if (index < 0 || index >= this.cells.length) {
throw new Error(`model index out of range ${index}`);
}
}

// TODO@rebornix should this trigger content change event?
Expand Down
16 changes: 15 additions & 1 deletion src/vs/workbench/contrib/notebook/common/notebookCommon.ts
View file
Expand Up @@ -215,11 +215,25 @@ export type NotebookCellsSplice2 = [
IMainCellDto[]
];

export interface NotebookCellsChangedEvent {
export enum NotebookCellsChangeType {
ModelChange = 1,
Move = 2
}

export interface NotebookCellsModelChangedEvent {
readonly kind: NotebookCellsChangeType.ModelChange;
readonly changes: NotebookCellsSplice2[];
readonly versionId: number;
}

export interface NotebookCellsModelMoveEvent {
readonly kind: NotebookCellsChangeType.Move;
readonly index: number;
readonly newIdx: number;
readonly versionId: number;
}

export type NotebookCellsChangedEvent = NotebookCellsModelChangedEvent | NotebookCellsModelMoveEvent;
export enum CellEditType {
Insert = 1,
Delete = 2
Expand Down
13 changes: 12 additions & 1 deletion src/vs/workbench/test/browser/api/extHostNotebookConcatDocument.test.ts
View file
Expand Up @@ -11,7 +11,7 @@ import { NullLogService } from 'vs/platform/log/common/log';
import { ExtHostNotebookConcatDocument } from 'vs/workbench/api/common/extHostNotebookConcatDocument';
import { ExtHostNotebookDocument, ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebook';
import { URI } from 'vs/base/common/uri';
import { CellKind, CellUri } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { CellKind, CellUri, NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { Position, Location, Range } from 'vs/workbench/api/common/extHostTypes';
import { ExtHostCommands } from 'vs/workbench/api/common/extHostCommands';
import { nullExtensionDescription } from 'vs/workbench/services/extensions/common/extensions';
Expand Down Expand Up @@ -51,6 +51,7 @@ suite('NotebookConcatDocument', function () {
});
await extHostNotebooks.$resolveNotebook('test', notebookUri);
extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: 0,
changes: [[0, 0, [{
handle: 0,
Expand Down Expand Up @@ -104,6 +105,7 @@ suite('NotebookConcatDocument', function () {
test('location, position mapping', function () {

extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[0, 0, [{
handle: 1,
Expand Down Expand Up @@ -142,6 +144,7 @@ suite('NotebookConcatDocument', function () {

// UPDATE 1
extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[0, 0, [{
handle: 1,
Expand All @@ -163,6 +166,7 @@ suite('NotebookConcatDocument', function () {

// UPDATE 2
extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[1, 0, [{
handle: 2,
Expand All @@ -185,6 +189,7 @@ suite('NotebookConcatDocument', function () {

// UPDATE 3 (remove cell #2 again)
extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[1, 1, []]]
});
Expand All @@ -202,6 +207,7 @@ suite('NotebookConcatDocument', function () {

// UPDATE 1
extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[0, 0, [{
handle: 1,
Expand Down Expand Up @@ -264,6 +270,7 @@ suite('NotebookConcatDocument', function () {
test('selector', function () {

extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[0, 0, [{
handle: 1,
Expand Down Expand Up @@ -291,6 +298,7 @@ suite('NotebookConcatDocument', function () {
assertLines(barLangDoc, 'barLang-document');

extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[2, 0, [{
handle: 3,
Expand Down Expand Up @@ -323,6 +331,7 @@ suite('NotebookConcatDocument', function () {
test('offsetAt(position) <-> positionAt(offset)', function () {

extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[0, 0, [{
handle: 1,
Expand Down Expand Up @@ -373,6 +382,7 @@ suite('NotebookConcatDocument', function () {
test('locationAt(position) <-> positionAt(location)', function () {

extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[0, 0, [{
handle: 1,
Expand Down Expand Up @@ -407,6 +417,7 @@ suite('NotebookConcatDocument', function () {
test('getText(range)', function () {

extHostNotebooks.$acceptModelChanged(notebookUri, {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.versionId + 1,
changes: [[0, 0, [{
handle: 1,
Expand Down

0 comments on commit bf25559

Please sign in to comment.