Merge pull request #1417 from blink1073/codemirror-tests

CodeMirror model cleanup and tests

src/codeeditor/editor.ts

@@ -348,15 +348,20 @@ namespace CodeEditor {
      * @param line - The line of interest.
      *
      * @returns The value of the line.
+     *
+     * #### Notes
+     * Lines are 0-based, and accessing a line out of range returns
+     * `undefined`.
      */
-    getLine(line: number): string;
+    getLine(line: number): string | undefined;
 
     /**
      * Find an offset for the given position.
      *
      * @param position - The position of interest.
      *
-     * @returns The offset at the position.
+     * @returns The offset at the position, clamped to the extent of the
+     * editor contents.
      */
     getOffsetAt(position: IPosition): number;
 
@@ -365,9 +370,10 @@ namespace CodeEditor {
      *
      * @param offset - The offset of interest.
      *
-     * @returns The position at the offset.
+     * @returns The position at the offset, clamped to the extent of the
+     * editor contents.
      */
-    getPositionAt(offset: number): IPosition;
+    getPositionAt(offset: number): IPosition | undefined;
 
     /**
      * Undo one edit (if any undo events are stored).

src/codemirror/model.ts

@@ -36,10 +36,7 @@ class CodeMirrorModel implements CodeEditor.IModel {
    */
   constructor(doc: CodeMirror.Doc = new CodeMirror.Doc('')) {
     this._doc = doc;
-    CodeMirror.on(this.doc, 'change', (instance, change) => {
-      this._onDocChange(instance, change);
-    });
-    this._value.changed.connect(this._onValueChanged, this);
+    this._value = new Private.ObservableDoc(doc);
   }
 
   /**
@@ -63,26 +60,6 @@ class CodeMirrorModel implements CodeEditor.IModel {
     return this._doc;
   }
 
-  /**
-   * Whether the model is disposed.
-   */
-  get isDisposed(): boolean {
-    return this._isDisposed;
-  }
-
-  /**
-   * Dipose of the resources used by the model.
-   */
-  dispose(): void {
-    if (this._isDisposed) {
-      return;
-    }
-    this._isDisposed = true;
-    this._selections.dispose();
-    this._value.dispose();
-    clearSignalData(this);
-  }
-
   /**
    * A mime type of the model.
    */
@@ -109,10 +86,30 @@ class CodeMirrorModel implements CodeEditor.IModel {
     return this._doc.lineCount();
   }
 
+  /**
+   * Whether the model is disposed.
+   */
+  get isDisposed(): boolean {
+    return this._isDisposed;
+  }
+
+  /**
+   * Dipose of the resources used by the model.
+   */
+  dispose(): void {
+    if (this._isDisposed) {
+      return;
+    }
+    this._isDisposed = true;
+    this._selections.dispose();
+    this._value.dispose();
+    clearSignalData(this);
+  }
+
   /**
    * Returns the content for the given line number.
    */
-  getLine(line: number): string {
+  getLine(line: number): string | undefined {
     return this._doc.getLine(line);
   }
 
@@ -155,41 +152,11 @@ class CodeMirrorModel implements CodeEditor.IModel {
     this._doc.clearHistory();
   }
 
-  /**
-   * Handle value changes.
-   */
-  private _onValueChanged(value: IObservableString, change: ObservableString.IChangedArgs): void {
-    if (this._changeGuard) {
-      return;
-    }
-    let doc = this._doc;
-    switch (change.type) {
-    case 'set':
-      doc.setValue(change.value);
-      break;
-    default:
-      let from = doc.posFromIndex(change.start);
-      let to = doc.posFromIndex(change.end);
-      doc.replaceRange(change.value, from, to);
-    }
-  }
-
-  /**
-   * Handles document changes.
-   */
-  protected _onDocChange(doc: CodeMirror.Doc, change: CodeMirror.EditorChange) {
-    if (change.origin !== 'setValue') {
-      this._changeGuard = true;
-      this._value.text = doc.getValue();
-      this._changeGuard = false;
-    }
-  }
 
-  private _mimetype = '';
-  private _value = new ObservableString();
+  private _mimetype = 'text/plain';
+  private _value: Private.ObservableDoc;
   private _isDisposed = false;
   private _doc: CodeMirror.Doc;
-  private _changeGuard = false;
   private _selections = new CodeEditor.Selections();
 }
 
@@ -198,3 +165,157 @@ class CodeMirrorModel implements CodeEditor.IModel {
  * The signals for the `CodeMirrorModel` class.
  */
 defineSignal(CodeMirrorModel.prototype, 'mimeTypeChanged');
+
+
+/**
+ * The namespace for module private data.
+ */
+namespace Private {
+  /**
+   * An observable string implementation wrapping a Codemirror Doc.
+   */
+  export
+  class ObservableDoc implements IObservableString {
+    /**
+     * Create a new observable doc.
+     */
+    constructor(doc: CodeMirror.Doc) {
+      this._doc = doc;
+      CodeMirror.on(doc, 'change', (instance, change) => {
+        this._onDocChange(instance, change);
+      });
+    }
+
+    /**
+     * A signal emitted when the string has changed.
+     */
+    readonly changed: ISignal<this, ObservableString.IChangedArgs>;
+
+    /**
+     * Set the value of the string.
+     */
+    set text(value: string) {
+      this._changeGuard = true;
+      this._doc.setValue(value);
+      this._changeGuard = false;
+      this.changed.emit({
+        type: 'set',
+        start: 0,
+        end: value.length,
+        value: value
+      });
+    }
+
+    /**
+     * Get the value of the string.
+     */
+    get text(): string {
+      return this._doc.getValue();
+    }
+
+    /**
+     * Insert a substring.
+     *
+     * @param index - The starting index.
+     *
+     * @param text - The substring to insert.
+     */
+    insert(index: number, text: string): void {
+      let doc = this._doc;
+      let pos = doc.posFromIndex(index);
+      doc.replaceRange(text, pos, pos);
+      this.changed.emit({
+        type: 'insert',
+        start: index,
+        end: index + text.length,
+        value: text
+      });
+    }
+
+    /**
+     * Remove a substring.
+     *
+     * @param start - The starting index.
+     *
+     * @param end - The ending index.
+     */
+    remove(start: number, end: number): void {
+      let doc = this._doc;
+      let from = doc.posFromIndex(start);
+      let to = doc.posFromIndex(end);
+      let oldValue = doc.getRange(from, to);
+      doc.replaceRange('', from, to);
+      this.changed.emit({
+        type: 'remove',
+        start: start,
+        end: end,
+        value: oldValue
+      });
+    }
+
+    /**
+     * Set the ObservableDoc to an empty string.
+     */
+    clear(): void {
+      this._doc.setValue('');
+    }
+
+    /**
+     * Test whether the string has been disposed.
+     */
+    get isDisposed(): boolean {
+      return this._isDisposed;
+    }
+
+    /**
+     * Dispose of the resources held by the string.
+     */
+    dispose(): void {
+      if (this._isDisposed) {
+        return;
+      }
+      this._isDisposed = true;
+      clearSignalData(this);
+      this._doc = null;
+    }
+
+    /**
+     * Handles document changes.
+     */
+    protected _onDocChange(doc: CodeMirror.Doc, change: CodeMirror.EditorChange) {
+      if (this._changeGuard) {
+        return;
+      }
+      let value = doc.getValue();
+      if (!change.origin || change.origin === 'setValue' ||
+          change.text.length > 1 || change.removed.length > 1) {
+        this.changed.emit({
+          type: 'set',
+          start: 0,
+          end: value.length,
+          value
+        });
+        return;
+      }
+      let start = doc.indexFromPos(change.from);
+      let end = doc.indexFromPos(change.to);
+      let changeType: ObservableString.ChangeType = 'insert';
+      if (change.origin.indexOf('delete') !== -1) {
+        changeType = 'remove';
+      }
+      this.changed.emit({
+        type: changeType,
+        start,
+        end,
+        value
+      });
+    }
+
+    private _doc: CodeMirror.Doc;
+    private _isDisposed : boolean = false;
+    private _changeGuard = false;
+  }
+
+  // Define the signals for the `ObservableDoc` class.
+  defineSignal(ObservableDoc.prototype, 'changed');
+}

test/src/codemirror/model.spec.ts

@@ -0,0 +1,219 @@
+// Copyright (c) Jupyter Development Team.
+// Distributed under the terms of the Modified BSD License.
+
+import expect = require('expect.js');
+
+import * as CodeMirror
+  from 'codemirror';
+
+import {
+  CodeMirrorModel
+} from '../../../lib/codemirror';
+
+
+describe('CodeMirrorModel', () => {
+
+  let model: CodeMirrorModel;
+
+  beforeEach(() => {
+    model = new CodeMirrorModel();
+  });
+
+  afterEach(() => {
+    model.dispose();
+  });
+
+  describe('#constructor()', () => {
+
+    it('should create a CodeMirrorModel', () => {
+      expect(model).to.be.a(CodeMirrorModel);
+    });
+
+  });
+
+  describe('#mimeTypeChanged', () => {
+
+    it('should be emitted when the mime type changes', () => {
+      let called = false;
+      model.mimeTypeChanged.connect((sender, args) => {
+        expect(sender).to.be(model);
+        expect(args.oldValue).to.be('text/plain');
+        expect(args.newValue).to.be('text/foo');
+        called = true;
+      });
+      model.mimeType = 'text/foo';
+      expect(called).to.be(true);
+    });
+
+  });
+
+  describe('#value', () => {
+
+    it('should be the observable value of the model', () => {
+      let called = false;
+      model.value.changed.connect((sender, args) => {
+        expect(sender).to.be(model.value);
+        expect(args.type).to.be('set');
+        expect(args.value).to.be('foo');
+        called = true;
+      });
+      model.value.text = 'foo';
+      expect(called).to.be(true);
+    });
+
+    it('should handle changes to the doc itself', (done) => {
+      let doc = model.doc;
+      model.value.changed.connect((sender, args) => {
+        expect(args.type).to.be('set');
+        expect(args.value).to.be('foo');
+        done();
+      });
+      doc.setValue('foo');
+    });
+
+    it('should handle an insert', () => {
+      let called = false;
+      model.value.changed.connect((sender, args) => {
+        expect(args.type).to.be('insert');
+        expect(args.value).to.be('foo');
+        called = true;
+      });
+      model.value.insert(0, 'foo');
+      expect(called).to.be(true);
+    });
+
+    it('should handle a remove', () => {
+      let called = false;
+      model.value.text = 'foo';
+      model.value.changed.connect((sender, args) => {
+        expect(args.type).to.be('remove');
+        expect(args.value).to.be('f');
+        called = true;
+      });
+      model.value.remove(0, 1);
+      expect(called).to.be(true);
+    });
+
+  });
+
+  describe('#selections', () => {
+
+    it('should be the selections associated with the model', () => {
+      expect(model.selections.uuids.length).to.be(0);
+    });
+
+  });
+
+  describe('#doc', () => {
+
+    it('should be the underlying CodeMirror Doc', () => {
+      expect(model.doc).to.be.a(CodeMirror.Doc);
+    });
+
+  });
+
+  describe('#mimeType', () => {
+
+    it('should be the mime type of the model', () => {
+      expect(model.mimeType).to.be('text/plain');
+      model.mimeType = 'text/foo';
+      expect(model.mimeType).to.be('text/foo');
+    });
+
+  });
+
+  describe('#lineCount', () => {
+
+    it('should get the number of lines in the model', () => {
+      expect(model.lineCount).to.be(1);
+      model.value.text = 'foo\nbar';
+      expect(model.lineCount).to.be(2);
+    });
+
+  });
+
+  describe('#isDisposed', () => {
+
+    it('should test whether the model is disposed', () => {
+      expect(model.isDisposed).to.be(false);
+      model.dispose();
+      expect(model.isDisposed).to.be(true);
+    });
+
+  });
+
+  describe('#getLine()', () => {
+
+    it('should get a line of text', () => {
+      model.value.text = 'foo\nbar';
+      expect(model.getLine(0)).to.be('foo');
+      expect(model.getLine(1)).to.be('bar');
+      expect(model.getLine(2)).to.be(void 0);
+    });
+
+  });
+
+  describe('#getOffsetAt()', () => {
+
+    it('should get the offset for a given position', () => {
+      model.value.text = 'foo\nbar';
+      let pos = {
+        column: 2,
+        line: 1
+      };
+      expect(model.getOffsetAt(pos)).to.be(6);
+      pos = {
+        column: 2,
+        line: 5
+      };
+      expect(model.getOffsetAt(pos)).to.be(7);
+    });
+  });
+
+  describe('#getPositionAt()', () => {
+
+    it('should get the position for a given offset', () => {
+      model.value.text = 'foo\nbar';
+      let pos = model.getPositionAt(6);
+      expect(pos.column).to.be(2);
+      expect(pos.line).to.be(1);
+      pos = model.getPositionAt(101);
+      expect(pos.column).to.be(3);
+      expect(pos.line).to.be(1);
+    });
+
+  });
+
+  describe('#undo()', () => {
+
+    it('should undo one edit', () => {
+      model.value.text = 'foo';
+      model.undo();
+      expect(model.value.text).to.be('');
+    });
+
+  });
+
+  describe('#redo()', () => {
+
+    it('should redo one undone edit', () => {
+      model.value.text = 'foo';
+      model.undo();
+      model.redo();
+      expect(model.value.text).to.be('foo');
+    });
+
+  });
+
+  describe('#clearHistory()', () => {
+
+    it('should clear the undo history', () => {
+      model.value.text = 'foo';
+      model.clearHistory();
+      model.undo();
+      expect(model.value.text).to.be('foo');
+    });
+
+  });
+
+});

test/src/index.ts

@@ -7,6 +7,8 @@ import './application/shell.spec';
 import './codeeditor/editor.spec';
 import './codeeditor/widget.spec';
 
+import './codemirror/model.spec';
+
 import './commandlinker/commandlinker.spec';
 
 import './common/activitymonitor.spec';