convert settings API to have Setting class with get() and set() methods. Also several tidy-ups

plugins/pilot/lib/commands/settings.js

@@ -37,26 +37,28 @@
 
 define(function(require, exports, module) {
 
+
 var setCommandSpec = {
-    name: "set",
+    name: 'set',
     params: [
         {
-            name: "setting",
+            name: 'setting',
-            type: "setting",
+            type: 'setting',
-            description: "The name of the setting to display or alter",
+            description: 'The name of the setting to display or alter',
             defaultValue: null
         },
         {
-            name: "value",
+            name: 'value',
-            type: "settingValue",
+            type: 'settingValue',
-            description: "The new value for the chosen setting",
+            description: 'The new value for the chosen setting',
             defaultValue: null
         }
     ],
-    description: "define and show settings",
+    description: 'define and show settings',
     exec: function(env, args, request) {
         var html;
         if (!args.setting) {
+            // 'set' by itself lists all the settings
             var settingsList = env.settings._list();
             html = '';
             // first sort the settingsList based on the key
@@ -69,7 +71,7 @@ var setCommandSpec = {
                     return 1;
                 }
             });
-            var url = "https://wiki.mozilla.org/Labs/Skywriter/Settings#" +
+            var url = 'https://wiki.mozilla.org/Labs/Skywriter/Settings#' +
                     setting.key;
             settingsList.forEach(function(setting) {
                 html += '<a class="setting" href="' + url +
@@ -82,13 +84,22 @@ var setCommandSpec = {
                         '<br/>';
             });
         } else {
+            var setting = env.settings.get(args.setting);
+            if (!setting) {
+                request.doneWithError('No setting with the name <strong>' +
+                    setting.name + '</strong>.');
+                return;
+            }
+
+            // set with only a setting, shows the value for that setting
             if (args.value === undefined) {
-                html = '<strong>' + args.setting + '</strong> = ' +
+                html = '<strong>' + setting.name + '</strong> = ' +
-                        env.settings.get(args.setting);
+                        setting.get();
             } else {
-                html = 'Setting: <strong>' + args.setting + '</strong> = ' +
+                // Actually change the setting
-                        args.value;
+                setting.set(args.value);
-                env.settings.set(args.setting, args.value);
+                html = 'Setting: <strong>' + setting.name + '</strong> = ' +
+                        setting.get();
             }
         }
         request.done(html);
@@ -96,18 +107,25 @@ var setCommandSpec = {
 };
 
 var unsetCommandSpec = {
-    name: "unset",
+    name: 'unset',
     params: [
         {
-            name: "setting",
+            name: 'setting',
-            type: "setting",
+            type: 'setting',
-            description: "The name of the setting to return to defaults"
+            description: 'The name of the setting to return to defaults'
         }
     ],
-    description: "unset a setting entirely",
+    description: 'unset a setting entirely',
     exec: function(env, args, request) {
-        env.settings.resetValue(args.setting);
+        var setting = env.settings.get(args.setting);
-        request.done('Reset ' + args.setting + ' to default: ' +
+        if (!setting) {
+            request.doneWithError('No setting with the name <strong>' +
+                args.setting + '</strong>.');
+            return;
+        }
+
+        setting.reset();
+        request.done('Reset ' + setting.name + ' to default: ' +
                 env.settings.get(args.setting));
     }
 };

plugins/pilot/lib/settings.js

@@ -43,18 +43,18 @@ define(function(require, exports, module) {
  * This plug-in manages settings.
  */
 
-var console = require("pilot/console");
+var console = require('pilot/console');
-var oop = require("pilot/oop").oop;
+var oop = require('pilot/oop').oop;
-var types = require("pilot/types");
+var types = require('pilot/types');
-var EventEmitter = require("pilot/event_emitter").EventEmitter;
+var EventEmitter = require('pilot/event_emitter').EventEmitter;
-var catalog = require("pilot/catalog");
+var catalog = require('pilot/catalog');
 
 var settingExtensionSpec = {
-    name: "setting",
+    name: 'setting',
-    description: "A setting is something that the application offers as a " +
+    description: 'A setting is something that the application offers as a ' +
-            "way to customize how it works",
+            'way to customize how it works',
-    register: "env.settings.addSetting",
+    register: 'env.settings.addSetting',
-    indexOn: "name"
+    indexOn: 'name'
 };
 
 exports.startup = function(data, reason) {
@@ -65,6 +65,68 @@ exports.shutdown = function(data, reason) {
     catalog.removeExtensionSpec(settingExtensionSpec);
 };
 
+
+/**
+ * Create a new setting.
+ * @param settingSpec An object literal that looks like this:
+ * {
+ *   name: 'thing',
+ *   description: 'Thing is an example setting',
+ *   type: 'string',
+ *   defaultValue: 'something'
+ * }
+ */
+function Setting(settingSpec, settings) {
+    this._settings = settings;
+
+    Object.keys(settingSpec).forEach(function(key) {
+        this[key] = settingSpec[key];
+    }, this);
+
+    this.type = types.getType(this.type);
+    if (this.type == null) {
+        throw new Error('In ' + this.name +
+            ': can\'t find type for: ' + JSON.stringify(settingSpec.type));
+    }
+
+    if (!this.name) {
+        throw new Error('Setting.name == undefined. Ignoring.', this);
+    }
+
+    if (!this.defaultValue === undefined) {
+        throw new Error('Setting.defaultValue == undefined', this);
+    }
+
+    this.value = this.defaultValue;
+}
+Setting.prototype = {
+    get: function() {
+        return this.value;
+    },
+
+    set: function(value) {
+        if (this.value === value) {
+            return;
+        }
+
+        this.value = value;
+        if (this._settings.persister) {
+            this._settings.persister.persistValue(this._settings, this.name, value);
+        }
+
+        this._dispatchEvent('change', { setting: this, value: value });
+    },
+
+    /**
+     * Reset the value of the <code>key</code> setting to it's default
+     */
+    resetValue: function() {
+        this.set(this.defaultValue);
+    }
+};
+oop.implement(Setting.prototype, EventEmitter);
+
+
 /**
  * A base class for all the various methods of storing settings.
  * <p>Usage:
@@ -89,11 +151,6 @@ function Settings(persister) {
      */
     this._deactivated = {};
 
-    /**
-     * Storage for the setting values
-     */
-    this._values = {};
-
     this._settings = {};
 
     if (persister) {
@@ -116,33 +173,8 @@ Settings.prototype = {
      * @param {object} settingSpec Object containing name/type/defaultValue members.
      */
     addSetting: function(settingSpec) {
-        if (!settingSpec.name) {
+        var setting = new Setting(settingSpec, this);
-            console.error('Setting.name == undefined. Ignoring.', settingSpec);
+        this._settings[setting.name] = setting;
-            return;
-        }
-
-        if (!settingSpec.defaultValue === undefined) {
-            console.error('Setting.defaultValue == undefined', settingSpec);
-            return;
-        }
-
-        var type = types.getType(settingSpec.type);
-        if (!type) {
-            console.error('Missing type', settingSpec);
-            return;
-        }
-
-        /*
-        // The value can be
-        // 1) the value of a setting that is not activated at the moment
-        //       OR
-        // 2) the defaultValue of the setting.
-        var value = this._deactivated[settingSpec.name] ||
-                settingSpec.defaultValue;
-        */
-
-        var setting = { type: type, spec: settingSpec };
-        this._settings[settingSpec.name] = setting;
     },
 
     removeSetting: function(name) {
@@ -153,6 +185,10 @@ Settings.prototype = {
         return Object.keys(this._settings);
     },
 
+    getSetting: function(name) {
+        return this._settings[name];
+    },
+
     /**
      * A Persister is able to store settings. It is an object that defines
      * two functions:
@@ -165,44 +201,6 @@ Settings.prototype = {
         }
     },
 
-    /**
-     * Read accessor
-     */
-    get: function(key) {
-        return this._values[key];
-    },
-
-    /**
-     * Override observable.set(key, value) to provide type conversion and
-     * validation.
-     */
-    set: function(key, value) {
-        var setting = this._settings[key];
-        if (!setting) {
-            console.warn('Setting not defined: ', key, value);
-        }
-
-        this._values[key] = value;
-        if (persister) {
-            persister.persistValue(this, key, value);
-        }
-
-        this._dispatchEvent('settingChange', { key: key, value: value });
-        return this;
-    },
-
-    /**
-     * Reset the value of the <code>key</code> setting to it's default
-     */
-    resetValue: function(key) {
-        var setting = this._settings[key];
-        if (setting) {
-            this.set(key, setting.spec.defaultValue);
-        } else {
-            console.log('ignore resetValue on ', key);
-        }
-    },
-
     resetAll: function() {
         this.getSettingNames().forEach(function(key) {
             this.resetValue(key);
@@ -267,9 +265,6 @@ Settings.prototype = {
         }.bind(this));
     }
 };
-
-oop.implement(Settings.prototype, EventEmitter);
-
 exports.settings = new Settings();
 
 /**

plugins/pilot/lib/types/basic.js

@@ -200,11 +200,12 @@ function DeferredType(typeSpec) {
 DeferredType.prototype = new Type();
 
 DeferredType.prototype.stringify = function(value) {
-    return this.defer.stringify(value);
+    return this.defer().stringify(value);
 };
 
 DeferredType.prototype.parse = function(value) {
-    return this.defer.parse(value);
+    console.log(this.defer);
+    return this.defer().parse(value);
 };
 
 DeferredType.prototype.name = 'deferred';

plugins/pilot/lib/types/settings.js

@@ -38,9 +38,10 @@
 
 define(function(require, exports, module) {
 
-var SelectionType = require("pilot/types/basic").SelectionType;
+var SelectionType = require('pilot/types/basic').SelectionType;
-var DeferredType = require("pilot/types/basic").DeferredType;
+var DeferredType = require('pilot/types/basic').DeferredType;
-var types = require("pilot/types");
+var types = require('pilot/types');
+var settings = require('pilot/settings').settings;
 
 
 /**
@@ -58,13 +59,21 @@ var setting = new SelectionType({
     data: function() {
         return env.settings.getSettingNames();
     },
-    stringify: function(value) {
+    stringify: function(setting) {
-        lastSetting = value;
+        lastSetting = setting;
-        return SelectionType.prototype.stringify.call(this, value);
+        return setting.name;
     },
-    parse: function(value) {
+    parse: function(text) {
-        lastSetting = value;
+        lastSetting = text;
-        return SelectionType.prototype.parse.call(this, value);
+
+        var conversion = SelectionType.prototype.parse.call(this, text);
+        if (conversion.value) {
+            conversion.value = settings.getSetting(conversion.value);
+        }
+        else {
+            conversion.message = 'Several possibilities for \'' + text + '\'';
+        }
+        return conversion;
     }
 });
 
@@ -73,7 +82,7 @@ var setting = new SelectionType({
  * of the type to the command line.
  */
 var settingValue = new DeferredType({
-    name: "settingValue",
+    name: 'settingValue',
     defer: function() {
         return env.settings.getSetting(lastSetting).type;
     }