From 61ddfb76e42f38f9fdf8f8f28b247cf5d90bd99c Mon Sep 17 00:00:00 2001 From: Trevor Parscal Date: Wed, 2 Oct 2013 12:16:18 -0700 Subject: [PATCH] Make ve.Factory require static name property Objective: * Make ve.Factory behave like ve.NamedClassFactory * Remove the only remaining use of ve.Factory (actions) * Remove ve.NamedClassFactory Change-Id: Ie302ef5ea31081de7ab0db6091058a59946aef4c --- .docs/categories.json | 4 -- VisualEditor.php | 1 - demos/ve/index.php | 1 - modules/ve/ce/ve.ce.AnnotationFactory.js | 6 +-- modules/ve/ce/ve.ce.NodeFactory.js | 6 +-- modules/ve/dm/ve.dm.AnnotationFactory.js | 6 +-- modules/ve/dm/ve.dm.MetaItemFactory.js | 6 +-- modules/ve/dm/ve.dm.NodeFactory.js | 6 +-- modules/ve/test/index.php | 1 - modules/ve/test/ve.Factory.test.js | 17 ++++---- .../ve/ui/actions/ve.ui.AnnotationAction.js | 4 +- modules/ve/ui/actions/ve.ui.ContentAction.js | 4 +- modules/ve/ui/actions/ve.ui.FormatAction.js | 4 +- modules/ve/ui/actions/ve.ui.HistoryAction.js | 4 +- .../ve/ui/actions/ve.ui.IndentationAction.js | 4 +- .../ve/ui/actions/ve.ui.InspectorAction.js | 4 +- modules/ve/ui/actions/ve.ui.ListAction.js | 4 +- modules/ve/ui/ve.ui.DialogFactory.js | 6 +-- modules/ve/ui/ve.ui.InspectorFactory.js | 6 +-- modules/ve/ui/ve.ui.ToolFactory.js | 6 +-- modules/ve/ve.Factory.js | 25 +++++++----- modules/ve/ve.NamedClassFactory.js | 39 ------------------- 22 files changed, 71 insertions(+), 93 deletions(-) delete mode 100644 modules/ve/ve.NamedClassFactory.js diff --git a/.docs/categories.json b/.docs/categories.json index d5eba63906..d6062d7d0d 100644 --- a/.docs/categories.json +++ b/.docs/categories.json @@ -162,10 +162,6 @@ "ve.Range", "ve.Element", "ve.EventSequencer" ] }, - { - "name": "Factories", - "classes": ["ve.NamedClassFactory"] - }, { "name": "Nodes", "classes": ["ve.Node", "ve.BranchNode", "ve.LeafNode", "ve.Document"] diff --git a/VisualEditor.php b/VisualEditor.php index c90f379001..e2abe39f1f 100644 --- a/VisualEditor.php +++ b/VisualEditor.php @@ -265,7 +265,6 @@ $wgResourceModules += array( 've/ve.Factory.js', 've/ve.Range.js', 've/ve.Node.js', - 've/ve.NamedClassFactory.js', 've/ve.BranchNode.js', 've/ve.LeafNode.js', 've/ve.Element.js', diff --git a/demos/ve/index.php b/demos/ve/index.php index eef13e0d35..aab64b4307 100644 --- a/demos/ve/index.php +++ b/demos/ve/index.php @@ -120,7 +120,6 @@ $html = file_get_contents( $page ); - diff --git a/modules/ve/ce/ve.ce.AnnotationFactory.js b/modules/ve/ce/ve.ce.AnnotationFactory.js index 1e6e13c3d7..6b78611575 100644 --- a/modules/ve/ce/ve.ce.AnnotationFactory.js +++ b/modules/ve/ce/ve.ce.AnnotationFactory.js @@ -9,17 +9,17 @@ * ContentEditable annotation factory. * * @class - * @extends ve.NamedClassFactory + * @extends ve.Factory * @constructor */ ve.ce.AnnotationFactory = function VeCeAnnotationFactory() { // Parent constructor - ve.NamedClassFactory.call( this ); + ve.Factory.call( this ); }; /* Inheritance */ -ve.inheritClass( ve.ce.AnnotationFactory, ve.NamedClassFactory ); +ve.inheritClass( ve.ce.AnnotationFactory, ve.Factory ); /* Methods */ diff --git a/modules/ve/ce/ve.ce.NodeFactory.js b/modules/ve/ce/ve.ce.NodeFactory.js index 64d80315ab..68971a1c21 100644 --- a/modules/ve/ce/ve.ce.NodeFactory.js +++ b/modules/ve/ce/ve.ce.NodeFactory.js @@ -9,17 +9,17 @@ * ContentEditable node factory. * * @class - * @extends ve.NamedClassFactory + * @extends ve.Factory * @constructor */ ve.ce.NodeFactory = function VeCeNodeFactory() { // Parent constructor - ve.NamedClassFactory.call( this ); + ve.Factory.call( this ); }; /* Inheritance */ -ve.inheritClass( ve.ce.NodeFactory, ve.NamedClassFactory ); +ve.inheritClass( ve.ce.NodeFactory, ve.Factory ); /* Methods */ diff --git a/modules/ve/dm/ve.dm.AnnotationFactory.js b/modules/ve/dm/ve.dm.AnnotationFactory.js index b035fda525..9918065fa4 100644 --- a/modules/ve/dm/ve.dm.AnnotationFactory.js +++ b/modules/ve/dm/ve.dm.AnnotationFactory.js @@ -9,17 +9,17 @@ * DataModel annotation factory. * * @class - * @extends ve.NamedClassFactory + * @extends ve.Factory * @constructor */ ve.dm.AnnotationFactory = function VeDmAnnotationFactory() { // Parent constructor - ve.NamedClassFactory.call( this ); + ve.Factory.call( this ); }; /* Inheritance */ -ve.inheritClass( ve.dm.AnnotationFactory, ve.NamedClassFactory ); +ve.inheritClass( ve.dm.AnnotationFactory, ve.Factory ); /* Initialization */ diff --git a/modules/ve/dm/ve.dm.MetaItemFactory.js b/modules/ve/dm/ve.dm.MetaItemFactory.js index 7a8de937dc..5db32d8656 100644 --- a/modules/ve/dm/ve.dm.MetaItemFactory.js +++ b/modules/ve/dm/ve.dm.MetaItemFactory.js @@ -9,17 +9,17 @@ * DataModel meta item factory. * * @class - * @extends ve.NamedClassFactory + * @extends ve.Factory * @constructor */ ve.dm.MetaItemFactory = function VeDmMetaItemFactory() { // Parent constructor - ve.NamedClassFactory.call( this ); + ve.Factory.call( this ); }; /* Inheritance */ -ve.inheritClass( ve.dm.MetaItemFactory, ve.NamedClassFactory ); +ve.inheritClass( ve.dm.MetaItemFactory, ve.Factory ); /* Methods */ diff --git a/modules/ve/dm/ve.dm.NodeFactory.js b/modules/ve/dm/ve.dm.NodeFactory.js index 3d99d31c8a..b49f1906e0 100644 --- a/modules/ve/dm/ve.dm.NodeFactory.js +++ b/modules/ve/dm/ve.dm.NodeFactory.js @@ -9,17 +9,17 @@ * DataModel node factory. * * @class - * @extends ve.NamedClassFactory + * @extends ve.Factory * @constructor */ ve.dm.NodeFactory = function VeDmNodeFactory() { // Parent constructor - ve.NamedClassFactory.call( this ); + ve.Factory.call( this ); }; /* Inheritance */ -ve.inheritClass( ve.dm.NodeFactory, ve.NamedClassFactory ); +ve.inheritClass( ve.dm.NodeFactory, ve.Factory ); /* Methods */ diff --git a/modules/ve/test/index.php b/modules/ve/test/index.php index d188d32e77..f23b027acc 100644 --- a/modules/ve/test/index.php +++ b/modules/ve/test/index.php @@ -65,7 +65,6 @@ - diff --git a/modules/ve/test/ve.Factory.test.js b/modules/ve/test/ve.Factory.test.js index 0991043651..2a5f6375f1 100644 --- a/modules/ve/test/ve.Factory.test.js +++ b/modules/ve/test/ve.Factory.test.js @@ -16,21 +16,24 @@ ve.FactoryObjectStub = function VeFactoryObjectStub( a, b, c, d ) { this.d = d; }; +ve.FactoryObjectStub.static = {}; + +ve.FactoryObjectStub.static.name = 'factory-object-stub'; + /* Tests */ -QUnit.test( 'register', 3, function ( assert ) { +QUnit.test( 'register', 2, function ( assert ) { var factory = new ve.Factory(); assert.throws( function () { - factory.register( 'factory-object-stub', 'not-a-function' ); + factory.register( 'not-a-function' ); }, Error, 'Throws an exception when trying to register a non-function value as a constructor' ); - factory.register( ['factory-object-stub-1', 'factory-object-stub-2'], ve.FactoryObjectStub ); - assert.strictEqual( factory.lookup( 'factory-object-stub-1' ), ve.FactoryObjectStub ); - assert.strictEqual( factory.lookup( 'factory-object-stub-2' ), ve.FactoryObjectStub ); + factory.register( ve.FactoryObjectStub ); + assert.strictEqual( factory.lookup( 'factory-object-stub' ), ve.FactoryObjectStub ); } ); QUnit.test( 'create', 3, function ( assert ) { @@ -45,7 +48,7 @@ QUnit.test( 'create', 3, function ( assert ) { 'Throws an exception when trying to create a object of an unregistered type' ); - factory.register( 'factory-object-stub', ve.FactoryObjectStub ); + factory.register( ve.FactoryObjectStub ); obj = factory.create( 'factory-object-stub', 16, 'foo', { 'baz': 'quux' }, 5 ); @@ -64,6 +67,6 @@ QUnit.test( 'create', 3, function ( assert ) { QUnit.test( 'lookup', 1, function ( assert ) { var factory = new ve.Factory(); - factory.register( 'factory-object-stub', ve.FactoryObjectStub ); + factory.register( ve.FactoryObjectStub ); assert.strictEqual( factory.lookup( 'factory-object-stub' ), ve.FactoryObjectStub ); } ); diff --git a/modules/ve/ui/actions/ve.ui.AnnotationAction.js b/modules/ve/ui/actions/ve.ui.AnnotationAction.js index 7f4361ce12..fb420693e3 100644 --- a/modules/ve/ui/actions/ve.ui.AnnotationAction.js +++ b/modules/ve/ui/actions/ve.ui.AnnotationAction.js @@ -24,6 +24,8 @@ ve.inheritClass( ve.ui.AnnotationAction, ve.ui.Action ); /* Static Properties */ +ve.ui.AnnotationAction.static.name = 'annotation'; + /** * List of allowed methods for the action. * @@ -109,4 +111,4 @@ ve.ui.AnnotationAction.prototype.clearAll = function () { /* Registration */ -ve.ui.actionFactory.register( 'annotation', ve.ui.AnnotationAction ); +ve.ui.actionFactory.register( ve.ui.AnnotationAction ); diff --git a/modules/ve/ui/actions/ve.ui.ContentAction.js b/modules/ve/ui/actions/ve.ui.ContentAction.js index 8eb72fb8e8..d7d628ed80 100644 --- a/modules/ve/ui/actions/ve.ui.ContentAction.js +++ b/modules/ve/ui/actions/ve.ui.ContentAction.js @@ -24,6 +24,8 @@ ve.inheritClass( ve.ui.ContentAction, ve.ui.Action ); /* Static Properties */ +ve.ui.ContentAction.static.name = 'content'; + /** * List of allowed methods for the action. * @@ -66,4 +68,4 @@ ve.ui.ContentAction.prototype.select = function ( range ) { /* Registration */ -ve.ui.actionFactory.register( 'content', ve.ui.ContentAction ); +ve.ui.actionFactory.register( ve.ui.ContentAction ); diff --git a/modules/ve/ui/actions/ve.ui.FormatAction.js b/modules/ve/ui/actions/ve.ui.FormatAction.js index 05e395bf92..afda6c9681 100644 --- a/modules/ve/ui/actions/ve.ui.FormatAction.js +++ b/modules/ve/ui/actions/ve.ui.FormatAction.js @@ -24,6 +24,8 @@ ve.inheritClass( ve.ui.FormatAction, ve.ui.Action ); /* Static Properties */ +ve.ui.FormatAction.static.name = 'format'; + /** * List of allowed methods for this action. * @@ -74,4 +76,4 @@ ve.ui.FormatAction.prototype.convert = function ( type, attributes ) { /* Registration */ -ve.ui.actionFactory.register( 'format', ve.ui.FormatAction ); +ve.ui.actionFactory.register( ve.ui.FormatAction ); diff --git a/modules/ve/ui/actions/ve.ui.HistoryAction.js b/modules/ve/ui/actions/ve.ui.HistoryAction.js index 61a1129690..6580c8ce2c 100644 --- a/modules/ve/ui/actions/ve.ui.HistoryAction.js +++ b/modules/ve/ui/actions/ve.ui.HistoryAction.js @@ -24,6 +24,8 @@ ve.inheritClass( ve.ui.HistoryAction, ve.ui.Action ); /* Static Properties */ +ve.ui.HistoryAction.static.name = 'history'; + /** * List of allowed methods for the action. * @@ -60,4 +62,4 @@ ve.ui.HistoryAction.prototype.redo = function () { /* Registration */ -ve.ui.actionFactory.register( 'history', ve.ui.HistoryAction ); +ve.ui.actionFactory.register( ve.ui.HistoryAction ); diff --git a/modules/ve/ui/actions/ve.ui.IndentationAction.js b/modules/ve/ui/actions/ve.ui.IndentationAction.js index a502699d38..cca6f41b83 100644 --- a/modules/ve/ui/actions/ve.ui.IndentationAction.js +++ b/modules/ve/ui/actions/ve.ui.IndentationAction.js @@ -24,6 +24,8 @@ ve.inheritClass( ve.ui.IndentationAction, ve.ui.Action ); /* Static Properties */ +ve.ui.IndentationAction.static.name = 'indentation'; + /** * List of allowed methods for the action. * @@ -312,4 +314,4 @@ ve.ui.IndentationAction.prototype.unindentListItem = function ( listItem ) { /* Registration */ -ve.ui.actionFactory.register( 'indentation', ve.ui.IndentationAction ); +ve.ui.actionFactory.register( ve.ui.IndentationAction ); diff --git a/modules/ve/ui/actions/ve.ui.InspectorAction.js b/modules/ve/ui/actions/ve.ui.InspectorAction.js index 3b0a12c8f7..683e383506 100644 --- a/modules/ve/ui/actions/ve.ui.InspectorAction.js +++ b/modules/ve/ui/actions/ve.ui.InspectorAction.js @@ -24,6 +24,8 @@ ve.inheritClass( ve.ui.InspectorAction, ve.ui.Action ); /* Static Properties */ +ve.ui.InspectorAction.static.name = 'inspector'; + /** * List of allowed methods for the action. * @@ -46,4 +48,4 @@ ve.ui.InspectorAction.prototype.open = function ( name ) { /* Registration */ -ve.ui.actionFactory.register( 'inspector', ve.ui.InspectorAction ); +ve.ui.actionFactory.register( ve.ui.InspectorAction ); diff --git a/modules/ve/ui/actions/ve.ui.ListAction.js b/modules/ve/ui/actions/ve.ui.ListAction.js index 42e1782f27..79628a0c7d 100644 --- a/modules/ve/ui/actions/ve.ui.ListAction.js +++ b/modules/ve/ui/actions/ve.ui.ListAction.js @@ -24,6 +24,8 @@ ve.inheritClass( ve.ui.ListAction, ve.ui.Action ); /* Static Properties */ +ve.ui.ListAction.static.name = 'list'; + /** * List of allowed methods for the action. * @@ -143,4 +145,4 @@ ve.ui.ListAction.prototype.unwrap = function () { /* Registration */ -ve.ui.actionFactory.register( 'list', ve.ui.ListAction ); +ve.ui.actionFactory.register( ve.ui.ListAction ); diff --git a/modules/ve/ui/ve.ui.DialogFactory.js b/modules/ve/ui/ve.ui.DialogFactory.js index 31795157eb..7a1af6f0fc 100644 --- a/modules/ve/ui/ve.ui.DialogFactory.js +++ b/modules/ve/ui/ve.ui.DialogFactory.js @@ -9,17 +9,17 @@ * UserInterface Dialog factory. * * @class - * @extends ve.NamedClassFactory + * @extends ve.Factory * @constructor */ ve.ui.DialogFactory = function VeUiDialogFactory() { // Parent constructor - ve.NamedClassFactory.call( this ); + ve.Factory.call( this ); }; /* Inheritance */ -ve.inheritClass( ve.ui.DialogFactory, ve.NamedClassFactory ); +ve.inheritClass( ve.ui.DialogFactory, ve.Factory ); /* Initialization */ diff --git a/modules/ve/ui/ve.ui.InspectorFactory.js b/modules/ve/ui/ve.ui.InspectorFactory.js index 1241c6cb63..7b41554d08 100644 --- a/modules/ve/ui/ve.ui.InspectorFactory.js +++ b/modules/ve/ui/ve.ui.InspectorFactory.js @@ -9,17 +9,17 @@ * UserInterface inspector factory. * * @class - * @extends ve.NamedClassFactory + * @extends ve.Factory * @constructor */ ve.ui.InspectorFactory = function VeUiInspectorFactory() { // Parent constructor - ve.NamedClassFactory.call( this ); + ve.Factory.call( this ); }; /* Inheritance */ -ve.inheritClass( ve.ui.InspectorFactory, ve.NamedClassFactory ); +ve.inheritClass( ve.ui.InspectorFactory, ve.Factory ); /* Initialization */ diff --git a/modules/ve/ui/ve.ui.ToolFactory.js b/modules/ve/ui/ve.ui.ToolFactory.js index 04bf53ee3e..d50ddb4e0b 100644 --- a/modules/ve/ui/ve.ui.ToolFactory.js +++ b/modules/ve/ui/ve.ui.ToolFactory.js @@ -9,17 +9,17 @@ * UserInterface tool factory. * * @class - * @extends ve.NamedClassFactory + * @extends ve.Factory * @constructor */ ve.ui.ToolFactory = function VeUiToolFactory() { // Parent constructor - ve.NamedClassFactory.call( this ); + ve.Factory.call( this ); }; /* Inheritance */ -ve.inheritClass( ve.ui.ToolFactory, ve.NamedClassFactory ); +ve.inheritClass( ve.ui.ToolFactory, ve.Factory ); /* Methods */ diff --git a/modules/ve/ve.Factory.js b/modules/ve/ve.Factory.js index 9e5327fe41..5eacc6e3d3 100644 --- a/modules/ve/ve.Factory.js +++ b/modules/ve/ve.Factory.js @@ -29,24 +29,31 @@ ve.inheritClass( ve.Factory, ve.Registry ); /** * Register a constructor with the factory. * + * Classes must have a static `name` property to be registered. + * + * @example + * function MyClass() {}; + * // Adds a static property to the class defining a symbolic name + * MyClass.static = { 'name': 'mine' }; + * // Registers class with factory, available via symbolic name 'mine' + * factory.register( MyClass ); + * * @method - * @param {string|string[]} name Symbolic name or list of symbolic names * @param {Function} constructor Constructor to use when creating object + * @throws {Error} Name must be a string and must not be empty * @throws {Error} Constructor must be a function */ -ve.Factory.prototype.register = function ( name, constructor ) { - var i, len; +ve.Factory.prototype.register = function ( constructor ) { + var name; if ( typeof constructor !== 'function' ) { throw new Error( 'constructor must be a function, cannot be a ' + typeof constructor ); } - if ( typeof name === 'string' ) { - this.entries.push( name ); - } else if ( ve.isArray( name ) ) { - for ( i = 0, len = name.length; i < len; i++ ) { - this.entries.push( name[i] ); - } + name = constructor.static && constructor.static.name; + if ( typeof name !== 'string' || name === '' ) { + throw new Error( 'Name must be a string and must not be empty' ); } + this.entries.push( name ); ve.Registry.prototype.register.call( this, name, constructor ); }; diff --git a/modules/ve/ve.NamedClassFactory.js b/modules/ve/ve.NamedClassFactory.js deleted file mode 100644 index 8089053ada..0000000000 --- a/modules/ve/ve.NamedClassFactory.js +++ /dev/null @@ -1,39 +0,0 @@ -/*! - * VisualEditor NamedClassFactory class. - * - * @copyright 2011-2013 VisualEditor Team and others; see AUTHORS.txt - * @license The MIT License (MIT); see LICENSE.txt - */ - -/** - * Generic factory for classes with a .static.name property. - * - * @abstract - * @extends ve.Factory - * @constructor - */ -ve.NamedClassFactory = function VeNamedClassFactory() { - // Parent constructor - ve.Factory.call( this ); -}; - -/* Inheritance */ - -ve.inheritClass( ve.NamedClassFactory, ve.Factory ); - -/* Methods */ - -/** - * Register a constructor with the factory. - * - * @method - * @param {Function} constructor Constructor to use when creating object - * @throws {Error} Names must be strings and must not be empty - */ -ve.NamedClassFactory.prototype.register = function ( constructor ) { - var name = constructor.static && constructor.static.name; - if ( typeof name !== 'string' || name === '' ) { - throw new Error( 'Names must be strings and must not be empty' ); - } - ve.Factory.prototype.register.call( this, name, constructor ); -};