Finish docregistry cleanup and tests

src/docmanager/manager.ts

@@ -121,7 +121,7 @@ class DocumentManager implements IDisposable {
     if (widgetName === 'default') {
       widgetName = registry.defaultWidgetFactory(ContentsManager.extname(path));
     }
-    let mFactory = registry.getModelFactory(widgetName);
+    let mFactory = registry.getModelFactoryFor(widgetName);
     if (!mFactory) {
       return;
     }
@@ -155,7 +155,7 @@ class DocumentManager implements IDisposable {
     if (widgetName === 'default') {
       widgetName = registry.defaultWidgetFactory(ContentsManager.extname(path));
     }
-    let mFactory = registry.getModelFactory(widgetName);
+    let mFactory = registry.getModelFactoryFor(widgetName);
     if (!mFactory) {
       return;
     }

src/docregistry/default.ts

@@ -4,6 +4,8 @@
 import * as CodeMirror
   from 'codemirror';
 
+import 'codemirror/mode/meta';
+
 import {
   IContents, IKernel
 } from 'jupyter-js-services';

src/docregistry/interfaces.ts

@@ -252,6 +252,7 @@ interface IWidgetFactoryOptions {
    * Use "*" to denote all files. Specific file extensions must be preceded
    * with '.', like '.png', '.txt', etc. Entries in this attribute must also
    * be included in the fileExtensions attribute.
+   * The default is an empty array.
    *
    * **See also:** [[fileExtensions]].
    */
@@ -259,11 +260,15 @@ interface IWidgetFactoryOptions {
 
   /**
    * Whether the widgets prefer having a kernel started.
+   *
+   * The default is `false`.
    */
   preferKernel?: boolean;
 
   /**
    * Whether the widgets can start a kernel when opened.
+   *
+   * The default is `false`.
    */
   canStartKernel?: boolean;
 }

src/docregistry/registry.ts

@@ -71,29 +71,24 @@ class DocumentRegistry implements IDisposable {
    */
   addWidgetFactory(factory: IWidgetFactory<Widget, IDocumentModel>, options: IWidgetFactoryOptions): IDisposable {
     let name = options.displayName.toLowerCase();
-    let exOpt = utils.copy(options) as Private.IWidgetFactoryRecord;
-    exOpt.factory = factory;
     if (this._widgetFactories[name]) {
       console.warn(`Duplicate registered factory ${name}`);
       return new DisposableDelegate(null);
     }
-    this._widgetFactories[name] = exOpt;
-    if (options.defaultFor) {
-      for (let ext of options.defaultFor) {
-        ext = Private.normalizeExtension(ext);
-        if (options.fileExtensions.indexOf(ext) === -1) {
-          continue;
-        }
-        if (ext === '*') {
-          this._defaultWidgetFactory = name;
-        } else {
-          this._defaultWidgetFactories[ext] = name;
-        }
+    let record = Private.createRecord(factory, options);
+    this._widgetFactories[name] = record;
+    for (let ext of record.defaultFor) {
+      if (record.fileExtensions.indexOf(ext) === -1) {
+        continue;
+      }
+      if (ext === '*') {
+        this._defaultWidgetFactory = name;
+      } else {
+        this._defaultWidgetFactories[ext] = name;
       }
     }
     // For convenience, store a mapping of ext -> name
-    for (let ext of options.fileExtensions) {
-      ext = Private.normalizeExtension(ext);
+    for (let ext of record.fileExtensions) {
       if (!this._widgetFactoryExtensions[ext]) {
         this._widgetFactoryExtensions[ext] = new Set<string>();
       }
@@ -166,6 +161,7 @@ class DocumentRegistry implements IDisposable {
       console.warn(`Duplicate registered extension for ${widgetName}`);
       return new DisposableDelegate(null);
     }
+    this._extenders[widgetName].push(extension);
     return new DisposableDelegate(() => {
       index = this._extenders[widgetName].indexOf(extension);
       this._extenders[widgetName].splice(index, 1);
@@ -225,10 +221,11 @@ class DocumentRegistry implements IDisposable {
     let added = false;
     if (after) {
       for (let existing of this._creators) {
-        if (existing.name === after) {
+        if (existing.name.toLowerCase() === after.toLowerCase()) {
           let index = this._creators.indexOf(existing);
           this._creators.splice(index, 0, creator);
           added = true;
+          break;
         }
       }
     }
@@ -242,13 +239,15 @@ class DocumentRegistry implements IDisposable {
   }
 
   /**
-   * List the names of the registered widget factories.
+   * List the names of the valid registered widget factories.
    *
    * @param ext - An optional file extension to filter the results.
    *
    * @returns A new array of registered widget factory names.
    *
    * #### Notes
+   * Only the widget factories whose associated model factory have
+   * been registered will be returned.
    * The first item in the list is considered the default. The returned list
    * has widget factories in the following order:
    * - extension-specific default factory
@@ -288,7 +287,7 @@ class DocumentRegistry implements IDisposable {
     // model factories are registered.
     let factoryList: string[] = [];
     factories.forEach(name => {
-      if (this._widgetFactories[name].modelName.toLowerCase() in this._modelFactories) {
+      if (this._widgetFactories[name].modelName in this._modelFactories) {
         name = this._widgetFactories[name].displayName;
         factoryList.push(name);
       }
@@ -305,7 +304,8 @@ class DocumentRegistry implements IDisposable {
    * @returns The default widget factory name for the extension (if given) or the global default.
    */
   defaultWidgetFactory(ext: string = '*') {
-    return this.listWidgetFactories(ext)[0];
+    let widgets = this.listWidgetFactories(ext);
+    return widgets ? widgets[0] : void 0;
   }
 
   /**
@@ -374,7 +374,10 @@ class DocumentRegistry implements IDisposable {
     ext = Private.normalizeExtension(ext);
     widgetName = widgetName.toLowerCase();
     let widgetFactoryEx = this._getWidgetFactoryEx(widgetName);
-    let modelFactory = this.getModelFactory(widgetName);
+    if (!widgetFactoryEx) {
+      return void 0;
+    }
+    let modelFactory = this.getModelFactoryFor(widgetName);
     let language = modelFactory.preferredLanguage(ext);
     return {
       language,
@@ -390,7 +393,7 @@ class DocumentRegistry implements IDisposable {
    *
    * @returns A model factory instance.
    */
-  getModelFactory(widgetName: string): IModelFactory {
+  getModelFactoryFor(widgetName: string): IModelFactory {
     widgetName = widgetName.toLowerCase();
     let wFactoryEx = this._getWidgetFactoryEx(widgetName);
     if (!wFactoryEx) {
@@ -408,7 +411,8 @@ class DocumentRegistry implements IDisposable {
    */
   getWidgetFactory(widgetName: string): IWidgetFactory<Widget, IDocumentModel> {
     widgetName = widgetName.toLowerCase();
-    return this._getWidgetFactoryEx(widgetName).factory;
+    let ex = this._getWidgetFactoryEx(widgetName);
+    return ex ? ex.factory : void 0;
   }
 
   /**
@@ -463,6 +467,25 @@ namespace Private {
     factory: IWidgetFactory<Widget, IDocumentModel>;
   }
 
+  /**
+   * Create a widget factory record.
+   */
+  export
+  function createRecord(factory: IWidgetFactory<Widget, IDocumentModel>, options: IWidgetFactoryOptions): IWidgetFactoryRecord {
+    let fileExtensions = options.fileExtensions.map(ext => normalizeExtension(ext));
+    let defaultFor = options.defaultFor || [];
+    defaultFor = defaultFor.map(ext => normalizeExtension(ext));
+    return {
+      factory,
+      fileExtensions,
+      defaultFor,
+      displayName: options.displayName,
+      modelName: options.modelName.toLowerCase(),
+      preferKernel: !!options.preferKernel,
+      canStartKernel: !!options.canStartKernel
+    };
+  }
+
   /**
    * Normalize a file extension to be of the type `'.foo'`.
    *
@@ -473,6 +496,9 @@ namespace Private {
     if (extension === '*') {
       return extension;
     }
+    if (extension === '.*') {
+      return '*';
+    }
     if (extension.indexOf('.') !== 0) {
       extension = `.${extension}`;
     }

test/src/docregistry/registry.spec.ts

@@ -16,7 +16,7 @@ import {
 } from 'phosphor-widget';
 
 import {
-  ABCWidgetFactory, DocumentRegistry,
+  ABCWidgetFactory, Base64ModelFactory, DocumentRegistry,
   IDocumentModel, IDocumentContext, IWidgetExtension, TextModelFactory
 } from '../../../lib/docregistry';
 
@@ -31,17 +31,14 @@ class WidgetFactory extends ABCWidgetFactory<Widget, IDocumentModel> {
 
 
 class WidgetExtension implements IWidgetExtension<Widget, IDocumentModel> {
-   /**
-    * Create a new extension for a given widget.
-    */
+
    createNew(widget: Widget, context: IDocumentContext<IDocumentModel>): IDisposable {
-     return new DisposableDelegate(() => {
-       // no-op
-     });
+     return new DisposableDelegate(null);
    }
 }
 
 
+
 describe('docregistry/registry', () => {
 
   describe('DocumentRegistry', () => {
@@ -76,7 +73,6 @@ describe('docregistry/registry', () => {
         registry.addFileType({ name: 'notebook', extension: '.ipynb' });
         registry.dispose();
         expect(registry.isDisposed).to.be(true);
-        expect(registry.listFileTypes()).to.eql([]);
       });
 
       it('should be safe to call multiple times', () => {
@@ -97,20 +93,23 @@ describe('docregistry/registry', () => {
           modelName: 'bar'
         });
         expect(registry.getWidgetFactory('foo')).to.be(factory);
+        expect(registry.getWidgetFactory('FOO')).to.be(factory);
       });
 
-      it('should throw an error if the `displayName` is already registerd', () => {
-        registry.addWidgetFactory(new WidgetFactory(), {
+      it('should be a no-op if the `displayName` is already registered', () => {
+        let factory = new WidgetFactory();
+        registry.addWidgetFactory(factory, {
           fileExtensions: [],
           displayName: 'foo',
           modelName: 'bar'
         });
-        expect(() => {
-          registry.addWidgetFactory(new WidgetFactory(), {
+        let disposable = registry.addWidgetFactory(new WidgetFactory(), {
           fileExtensions: [],
           displayName: 'foo',
           modelName: 'bar'
-        }); }).to.throwError();
+        });
+        disposable.dispose();
+        expect(registry.getWidgetFactory('foo')).to.be(factory);
       });
 
       it('should become the global default if `*` is given as a defaultFor', () => {
@@ -121,23 +120,25 @@ describe('docregistry/registry', () => {
           displayName: 'foo',
           modelName: 'text'
         });
-        expect(registry.defaultWidgetFactory('*')).to.be(factory);
+        expect(registry.defaultWidgetFactory('*')).to.be('foo');
       });
 
       it('should override an existing global default', () => {
         registry.addModelFactory(new TextModelFactory());
         registry.addWidgetFactory(new WidgetFactory(), {
           fileExtensions: ['*'],
+          defaultFor: ['.*'],
           displayName: 'foo',
           modelName: 'text'
         });
         let factory = new WidgetFactory();
         registry.addWidgetFactory(factory, {
           fileExtensions: ['*'],
+          defaultFor: ['*'],
           displayName: 'bar',
           modelName: 'text'
         });
-        expect(registry.defaultWidgetFactory('*')).to.be(factory);
+        expect(registry.defaultWidgetFactory('*')).to.be('bar');
       });
 
       it('should override an existing extension default', () => {
@@ -153,7 +154,7 @@ describe('docregistry/registry', () => {
           displayName: 'bar',
           modelName: 'text'
         });
-        expect(registry.defaultWidgetFactory('.txt')).to.be(factory);
+        expect(registry.defaultWidgetFactory('.txt')).to.be('foo');
       });
 
       it('should be removed from the registry when disposed', () => {
@@ -175,7 +176,7 @@ describe('docregistry/registry', () => {
       it('should add the model factory to the registry', () => {
         let factory = new TextModelFactory();
         registry.addModelFactory(factory);
-        expect(registry.getModelFactory('text')).to.be(factory);
+        expect(registry.listModelFactories()).to.eql(['text']);
       });
 
       it('should be a no-op a factory with the given `name` is already registered', () => {
@@ -183,7 +184,7 @@ describe('docregistry/registry', () => {
         registry.addModelFactory(factory);
         let disposable = registry.addModelFactory(new TextModelFactory());
         disposable.dispose();
-        expect(registry.listModelFactories()).to.eql('text');
+        expect(registry.listModelFactories()).to.eql(['text']);
       });
 
       it('should be a no-op if the same factory is already registered', () => {
@@ -191,7 +192,7 @@ describe('docregistry/registry', () => {
         registry.addModelFactory(factory);
         let disposable = registry.addModelFactory(factory);
         disposable.dispose();
-        expect(registry.listModelFactories()).to.eql('text');
+        expect(registry.listModelFactories()).to.eql(['text']);
       });
 
       it('should be removed from the registry when disposed', () => {
@@ -261,6 +262,13 @@ describe('docregistry/registry', () => {
         expect(registry.listCreators()).to.eql([creator]);
       });
 
+      it('should be removed from the registry when disposed', () => {
+        let creator = { name: 'notebook', fileType: 'notebook' };
+        let disposable = registry.addCreator(creator);
+        disposable.dispose();
+        expect(registry.listCreators()).to.eql([]);
+      });
+
       it('should add after a named creator if given', () => {
         let creators = [
           { name: 'Python Notebook', fileType: 'notebook' },
@@ -269,69 +277,293 @@ describe('docregistry/registry', () => {
         ];
         registry.addCreator(creators[0]);
         registry.addCreator(creators[1]);
-        registry.addCreator(creators[2], creators[0].name);
+        registry.addCreator(creators[2], creators[1].name);
         expect(registry.listCreators()).to.eql([ creators[0], creators[2], creators[1]]);
       });
 
-      it('should be removed from the registry when disposed', () => {
-        let creator = { name: 'notebook', fileType: 'notebook' };
-        let disposable = registry.addCreator(creator);
-        disposable.dispose();
-        expect(registry.listCreators()).to.eql([]);
-      });
-
       it('should be a no-op if a file type of the same name is registered', () => {
         let creator = { name: 'notebook', fileType: 'notebook' };
         registry.addCreator(creator);
         let disposable = registry.addCreator(creator);
         disposable.dispose();
-        expect(registry.listFileTypes()).to.eql([creator]);
+        expect(registry.listCreators()).to.eql([creator]);
       });
 
     });
 
     describe('#listWidgetFactories()', () => {
 
+      it('should list the names of the valid registered widget factories', () => {
+        expect(registry.listWidgetFactories()).to.eql([]);
+        registry.addModelFactory(new TextModelFactory());
+        let factory = new WidgetFactory();
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['.foo'],
+          displayName: 'Foo',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['.foo'],
+          displayName: 'Bar',
+          modelName: 'text'
+        });
+        expect(registry.listWidgetFactories('.foo')).to.eql(['Foo', 'Bar']);
+      });
+
+      it('should not list a factory whose model is not registered', () => {
+        registry.addWidgetFactory(new WidgetFactory(), {
+          fileExtensions: ['.bar'],
+          displayName: 'Bar',
+          modelName: 'text'
+        });
+        expect(registry.listWidgetFactories()).to.eql([]);
+      });
+
+      it('should select the factory for a given extension', () => {
+        registry.addModelFactory(new TextModelFactory());
+        let factory = new WidgetFactory();
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['.foo'],
+          displayName: 'Foo',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['.bar'],
+          displayName: 'Bar',
+          modelName: 'text'
+        });
+        expect(registry.listWidgetFactories('.foo')).to.eql(['Foo']);
+      });
+
+      it('should respect the priority order', () => {
+        registry.addModelFactory(new TextModelFactory());
+        let factory = new WidgetFactory();
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['.txt'],
+          displayName: 'Foo',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['.txt'],
+          defaultFor: ['.txt'],
+          displayName: 'Bar',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['*'],
+          displayName: 'Buzz',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['*'],
+          defaultFor: ['*'],
+          displayName: 'Fizz',
+          modelName: 'text'
+        });
+        expect(registry.listWidgetFactories('.txt')).to.eql(['Bar', 'Fizz', 'Foo', 'Buzz']);
+      });
+
     });
 
     describe('#defaultWidgetFactory()', () => {
 
+      it('should get the default widget factory for a given extension', () => {
+        registry.addModelFactory(new TextModelFactory());
+        let factory = new WidgetFactory();
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['.txt'],
+          displayName: 'Foo',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['.txt'],
+          defaultFor: ['.txt'],
+          displayName: 'Bar',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['*'],
+          displayName: 'Buzz',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(factory, {
+          fileExtensions: ['*'],
+          defaultFor: ['*'],
+          displayName: 'Fizz',
+          modelName: 'text'
+        });
+        expect(registry.defaultWidgetFactory('.txt')).to.be('Bar');
+        expect(registry.defaultWidgetFactory()).to.be('Fizz');
+      });
+
     });
 
     describe('#listModelFactories()', () => {
 
+      it('should list the currently registered model factories', () => {
+        expect(registry.listModelFactories()).to.eql([]);
+        registry.addModelFactory(new TextModelFactory());
+        registry.addModelFactory(new Base64ModelFactory());
+        expect(registry.listModelFactories()).to.eql(['text', 'base64']);
+      });
+
     });
 
     describe('#listFileTypes()', () => {
 
+      it('should list the registered file types', () => {
+        expect(registry.listFileTypes()).to.eql([]);
+        let fileTypes = [
+          { name: 'notebook', extension: '.ipynb' },
+          { name: 'python', extension: '.py' }
+        ];
+        registry.addFileType(fileTypes[0]);
+        registry.addFileType(fileTypes[1]);
+        expect(registry.listFileTypes()).to.eql(fileTypes);
+      });
+
     });
 
     describe('#listCreators()', () => {
 
+      it('should list the registered file creators', () => {
+        expect(registry.listCreators()).to.eql([]);
+        let creators = [
+          { name: 'Python Notebook', fileType: 'notebook' },
+          { name: 'R Notebook', fileType: 'notebook' },
+          { name: 'Shell Notebook', fileType: 'notebook' }
+        ];
+        registry.addCreator(creators[0]);
+        registry.addCreator(creators[1]);
+        registry.addCreator(creators[2]);
+        expect(registry.listCreators()).to.eql(creators);
+      });
+
     });
 
     describe('#getFileType()', () => {
 
+      it('should get a file type by name', () => {
+        let fileTypes = [
+          { name: 'notebook', extension: '.ipynb' },
+          { name: 'python', extension: '.py' }
+        ];
+        registry.addFileType(fileTypes[0]);
+        registry.addFileType(fileTypes[1]);
+        expect(registry.getFileType('notebook')).to.be(fileTypes[0]);
+        expect(registry.getFileType('python')).to.be(fileTypes[1]);
+        expect(registry.getFileType('r')).to.be(void 0);
+      });
     });
 
     describe('#getCreator()', () => {
 
+      it('should get a creator by name', () => {
+        let creators = [
+          { name: 'Python Notebook', fileType: 'notebook' },
+          { name: 'R Notebook', fileType: 'notebook' },
+          { name: 'Shell Notebook', fileType: 'notebook' }
+        ];
+        registry.addCreator(creators[0]);
+        registry.addCreator(creators[1]);
+        registry.addCreator(creators[2]);
+        expect(registry.getCreator('Python Notebook')).to.be(creators[0]);
+        expect(registry.getCreator('r notebook')).to.be(creators[1]);
+        expect(registry.getCreator('shell Notebook')).to.be(creators[2]);
+        expect(registry.getCreator('foo')).to.be(void 0);
+      });
+
     });
 
     describe('#getKernelPreference()', () => {
 
+      it('should get a kernel preference', () => {
+        registry.addModelFactory(new TextModelFactory());
+        registry.addWidgetFactory(new WidgetFactory(), {
+          fileExtensions: ['.txt'],
+          displayName: 'foo',
+          modelName: 'text'
+        });
+        registry.addWidgetFactory(new WidgetFactory(), {
+          fileExtensions: ['.py'],
+          displayName: 'bar',
+          modelName: 'text',
+          canStartKernel: true,
+          preferKernel: true
+        });
+
+        let pref = registry.getKernelPreference('.c', 'foo');
+        expect(pref.language).to.be('clike');
+        expect(pref.preferKernel).to.be(false);
+        expect(pref.canStartKernel).to.be(false);
+
+        pref = registry.getKernelPreference('.py', 'bar');
+        expect(pref.language).to.be('python');
+        expect(pref.preferKernel).to.be(true);
+        expect(pref.canStartKernel).to.be(true);
+
+        pref = registry.getKernelPreference('.py', 'baz');
+        expect(pref).to.be(void 0);
+      });
+
     });
 
-    describe('#getModelFactory()', () => {
+    describe('#getModelFactoryFor()', () => {
+
+      it('should get a registered model factory for a given widget name', () => {
+        let mFactory = new TextModelFactory();
+        registry.addModelFactory(mFactory);
+        registry.addWidgetFactory(new WidgetFactory(), {
+          fileExtensions: ['.txt'],
+          displayName: 'foo',
+          modelName: 'text'
+        });
+        expect(registry.getModelFactoryFor('foo')).to.be(mFactory);
+        expect(registry.getModelFactoryFor('FOO')).to.be(mFactory);
+        expect(registry.getModelFactoryFor('text')).to.be(void 0);
+      });
 
     });
 
     describe('#getWidgetFactory()', () => {
 
+      it('should get a widget factory by name', () => {
+        registry.addModelFactory(new TextModelFactory());
+        let foo = new WidgetFactory();
+        registry.addWidgetFactory(foo, {
+          fileExtensions: ['*'],
+          defaultFor: ['.*'],
+          displayName: 'foo',
+          modelName: 'text'
+        });
+        let bar = new WidgetFactory();
+        registry.addWidgetFactory(bar, {
+          fileExtensions: ['*'],
+          defaultFor: ['*'],
+          displayName: 'bar',
+          modelName: 'text'
+        });
+        expect(registry.getWidgetFactory('foo')).to.be(foo);
+        expect(registry.getWidgetFactory('Foo')).to.be(foo);
+        expect(registry.getWidgetFactory('bar')).to.be(bar);
+        expect(registry.getWidgetFactory('baz')).to.be(void 0);
+      });
+
     });
 
     describe('#getWidgetExtensions()', () => {
 
+      it('should get the registered extensions for a given widget', () => {
+        let foo = new WidgetExtension();
+        let bar = new WidgetExtension();
+        registry.addWidgetExtension('fizz', foo);
+        registry.addWidgetExtension('fizz', bar);
+        registry.addWidgetExtension('buzz', foo);
+        expect(registry.getWidgetExtensions('fizz')).to.eql([foo, bar]);
+        expect(registry.getWidgetExtensions('buzz')).to.eql([foo]);
+        expect(registry.getWidgetExtensions('baz')).to.eql([]);
+      });
+
     });
 
   });

test/src/index.ts

@@ -6,6 +6,7 @@ import './common/activitymonitor.spec';
 import './dialog/dialog.spec';
 
 import './docregistry/default.spec';
+import './docregistry/registry.spec';
 
 import './filebrowser/model.spec';