Bug 1128988 (part 2)

docs/writing-types.md

@@ -7,7 +7,7 @@ number of built in types:
 
 * string. This is a JavaScript string
 * number. A JavaScript number
-* boolean. A Javascript boolean
+* boolean. A JavaScript boolean
 * selection. This is an selection from a number of alternatives
 * delegate. This type could change depending on other factors, but is well
   defined when one of the conversion routines is called.
@@ -49,10 +49,10 @@ All types must inherit from Type and have the following methods:
      */
     name: 'example',
 
-In addition, defining the following functions can be helpful, although Type
+In addition, defining the following function can be helpful, although Type
 contains default implementations:
-* increment(value)
-* decrement(value)
+
+* nudge(value, by)
 
 Type, Conversion and Status are all declared by commands.js.
 

lib/gcli/cli.js

@@ -575,11 +575,12 @@ Object.defineProperty(Requisition.prototype, 'executionContext', {
         enumerable: true
       });
 
+      this._executionContext.updateExec = this._contextUpdateExec.bind(this);
+
       if (legacy) {
         this._executionContext.createView = view.createView;
         this._executionContext.exec = this.exec.bind(this);
         this._executionContext.update = this._contextUpdate.bind(this);
-        this._executionContext.updateExec = this._contextUpdateExec.bind(this);
 
         Object.defineProperty(this._executionContext, 'document', {
           get: function() { return requisition.document; },
@@ -659,7 +660,7 @@ Requisition.prototype.getParameterNames = function() {
  * this is still an error status.
  */
 Object.defineProperty(Requisition.prototype, 'status', {
-  get : function() {
+  get: function() {
     var status = Status.VALID;
     if (this._unassigned.length !== 0) {
       var isAllIncomplete = true;
@@ -1446,26 +1447,9 @@ Requisition.prototype.complete = function(cursor, rank) {
 /**
  * Replace the current value with the lower value if such a concept exists.
  */
-Requisition.prototype.decrement = function(assignment) {
-  var ctx = this.executionContext;
-  var val = assignment.param.type.decrement(assignment.value, ctx);
-  return Promise.resolve(val).then(function(replacement) {
-    if (replacement != null) {
-      var val = assignment.param.type.stringify(replacement, ctx);
-      return Promise.resolve(val).then(function(str) {
-        var arg = assignment.arg.beget({ text: str });
-        return this.setAssignment(assignment, arg);
-      }.bind(this));
-    }
-  }.bind(this));
-};
-
-/**
- * Replace the current value with the higher value if such a concept exists.
- */
-Requisition.prototype.increment = function(assignment) {
+Requisition.prototype.nudge = function(assignment, by) {
   var ctx = this.executionContext;
-  var val = assignment.param.type.increment(assignment.value, ctx);
+  var val = assignment.param.type.nudge(assignment.value, by, ctx);
   return Promise.resolve(val).then(function(replacement) {
     if (replacement != null) {
       var val = assignment.param.type.stringify(replacement, ctx);
@@ -2102,10 +2086,14 @@ Requisition.prototype.exec = function(options) {
  * unexpected change to the current command.
  */
 Requisition.prototype._contextUpdateExec = function(typed, options) {
-  return this.updateExec(typed, options).then(function(reply) {
-    this.onExternalUpdate({ typed: typed });
+  var reqOpts = {
+    document: this.document,
+    environment: this.environment
+  };
+  var child = new Requisition(this.system, reqOpts);
+  return child.updateExec(typed, options).then(function(reply) {
     return reply;
-  }.bind(this));
+  }.bind(child));
 };
 
 /**
@@ -2181,11 +2169,23 @@ Output.prototype.convert = function(type, conversionContext) {
 };
 
 Output.prototype.toJson = function() {
+  // Exceptions don't stringify, so we try a bit harder
+  var data = this.data;
+  if (this.error && JSON.stringify(this.data) === '{}') {
+    data = {
+      columnNumber: data.columnNumber,
+      fileName: data.fileName,
+      lineNumber: data.lineNumber,
+      message: data.message,
+      stack: data.stack
+    };
+  }
+
   return {
     typed: this.typed,
     type: this.type,
-    data: this.data,
-    error: this.error
+    data: data,
+    isError: this.error
   };
 };
 

lib/gcli/commands/commands.js

@@ -195,6 +195,19 @@ Command.prototype.toJson = function(customProps) {
   return json;
 };
 
+/**
+ * Easy way to lookup parameters by full name
+ */
+Command.prototype.getParameterByName = function(name) {
+  var reply;
+  this.params.forEach(function(param) {
+    if (param.name === name) {
+      reply = param;
+    }
+  });
+  return reply;
+};
+
 /**
  * Easy way to lookup parameters by short name
  */
@@ -264,16 +277,27 @@ function Parameter(types, paramSpec, command, groupName) {
                     ': Missing defaultValue for optional parameter.');
   }
 
-  this.defaultValue = (this.paramSpec.defaultValue !== undefined) ?
-                      this.paramSpec.defaultValue :
-                      this.type.getBlank().value;
+  if (this.paramSpec.defaultValue !== undefined) {
+    this.defaultValue = this.paramSpec.defaultValue;
+  }
+  else {
+    Object.defineProperty(this, 'defaultValue', {
+      get: function() {
+        return this.type.getBlank().value;
+      },
+      enumerable: true
+    });
+  }
 
   // Resolve the documentation
   this.manual = lookup(this.paramSpec.manual);
   this.description = lookup(this.paramSpec.description, 'canonDescNone');
 
   // Is the user required to enter data for this parameter? (i.e. has
   // defaultValue been set to something other than undefined)
+  // TODO: When the defaultValue comes from type.getBlank().value (see above)
+  // then perhaps we should set using something like
+  //   isDataRequired = (type.getBlank().status !== VALID)
   this.isDataRequired = (this.defaultValue === undefined);
 
   // Are we allowed to assign data to this parameter using positional

lib/gcli/commands/mocks.js

@@ -19,6 +19,7 @@
 var cli = require('../cli');
 var mockCommands = require('../test/mockCommands');
 var mockSettings = require('../test/mockSettings');
+var mockDocument = require('../test/mockDocument');
 
 exports.items = [
   {
@@ -46,11 +47,13 @@ exports.items = [
     on: function(requisition) {
       mockCommands.setup(requisition);
       mockSettings.setup(requisition.system);
+      mockDocument.setup(requisition);
     },
 
     off: function(requisition) {
       mockCommands.shutdown(requisition);
       mockSettings.shutdown(requisition.system);
+      mockDocument.shutdown(requisition);
     }
   }
 ];

lib/gcli/commands/preflist.js

@@ -22,7 +22,7 @@ var Promise = require('../util/promise').Promise;
 /**
  * Format a list of settings for display
  */
-var prefsData = {
+var prefsViewConverter = {
   item: 'converter',
   from: 'prefsData',
   to: 'view',
@@ -97,6 +97,22 @@ var prefsData = {
   }
 };
 
+/**
+ * Format a list of settings for display
+ */
+var prefsStringConverter = {
+  item: 'converter',
+  from: 'prefsData',
+  to: 'string',
+  exec: function(prefsData, conversionContext) {
+    var reply = '';
+    prefsData.settings.forEach(function(setting) {
+      reply += setting.name + ' -> ' + setting.value + '\n';
+    });
+    return reply;
+  }
+};
+
 /**
  * 'pref list' command
  */
@@ -136,6 +152,8 @@ function PrefList(prefsData, conversionContext) {
   this.search = prefsData.search;
   this.settings = prefsData.settings;
   this.conversionContext = conversionContext;
+
+  this.onLoad = this.onLoad.bind(this);
 }
 
 /**
@@ -194,4 +212,4 @@ PrefList.prototype.onSetClick = function(ev) {
   this.conversionContext.update(typed);
 };
 
-exports.items = [ prefsData, prefList ];
+exports.items = [ prefsViewConverter, prefsStringConverter, prefList ];

lib/gcli/commands/server/firefox.js

@@ -123,33 +123,17 @@ function buildFirefox(destDir) {
  */
 function createCommonJsToJsTestFilter() {
   var filter = function commonJsToJsTestFilter(data, location) {
-    var name = location.path.substring(1);
+    var name = location.path.replace(/\/test/, 'browser_gcli_').toLowerCase()
     var header = '$1' +
-        '\n' +
-        '// <INJECTED SOURCE:START>\n' +
-        '\n' +
+        '\n\n' +
         '// THIS FILE IS GENERATED FROM SOURCE IN THE GCLI PROJECT\n' +
-        '// DO NOT EDIT IT DIRECTLY\n' +
-        '\n' +
-        'var exports = {};\n' +
+        '// PLEASE TALK TO SOMEONE IN DEVELOPER TOOLS BEFORE EDITING IT\n' +
         '\n' +
-        'var TEST_URI = "data:text/html;charset=utf-8,<p id=\'gcli-input\'>gcli-' + name + '</p>";\n' +
+        'const exports = {};\n' +
         '\n' +
         'function test() {\n' +
-        '  return Task.spawn(*function() {\n' +
-        '    let options = yield helpers.openTab(TEST_URI);\n' +
-        '    yield helpers.openToolbar(options);\n' +
-        '    options.requisition.system.addItems(mockCommands.items);\n' +
-        '\n' +
-        '    yield helpers.runTests(options, exports);\n' +
-        '\n' +
-        '    options.requisition.system.removeItems(mockCommands.items);\n' +
-        '    yield helpers.closeToolbar(options);\n' +
-        '    yield helpers.closeTab(options);\n' +
-        '  }).then(finish, helpers.handleError);\n' +
-        '}\n' +
-        '\n' +
-        '// <INJECTED SOURCE:END>';
+        '  helpers.runTestModule(exports, "' + name + '");\n' +
+        '}';
     return data.toString()
         // Inject the header above just after 'use strict'
         .replace(/('use strict';)/, header)
@@ -167,23 +151,13 @@ function createCommonJsToJsTestFilter() {
 
 function commonJsToJsMockFilter(data) {
   var header = '$1' +
-      '\n' +
-      '// <INJECTED SOURCE:START>\n' +
-      '\n' +
+      '\n\n' +
       '// THIS FILE IS GENERATED FROM SOURCE IN THE GCLI PROJECT\n' +
-      '// DO NOT EDIT IT DIRECTLY\n' +
-      '\n' +
-      '// <INJECTED SOURCE:END>\n';
+      '// PLEASE TALK TO SOMEONE IN DEVELOPER TOOLS BEFORE EDITING IT';
 
   return data.toString()
       // Inject the header above just after 'use strict'
       .replace(/('use strict';)/, header)
-      // In mochitests everything is global
-      .replace(/var mockCommands = exports;/, 'var mockCommands = {};')
-      // Comment out test helpers that we define separately
-      .replace(/(var [A-z]* = require\(['"][A-z_\.\/]*\/assert['"]\);)/g, '// $1')
-      .replace(/(var [A-z]* = require\(['"][A-z_\.\/]*\/helpers['"]\);)/g, '// $1')
-      .replace(/(var [A-z]* = require\(['"][A-z_\.\/]*\/mockCommands['"]\);)/g, '// $1')
       // Make the require statements absolute rather than relative.
       // We're ignoring paths that start ../.. or ./ but this works for now
       .replace(/\nvar ([A-z]*) = require\(['"]..\/([A-z_\/]*)['"]\)/g, '\nvar $1 = require(\'gcli/$2\')');

lib/gcli/commands/test.js

@@ -70,10 +70,10 @@ exports.items = [
       }
       else {
         options = {
-          isNode: (typeof(process) !== 'undefined' && process.title === 'node'),
+          isNode: (typeof(process) !== 'undefined' &&
+                   process.title.indexOf('node') != -1),
           isFirefox: false,
           isPhantomjs: false,
-          isNoDom: true,
           requisition: new Requisition(context.system)
         };
         options.automator = createRequisitionAutomator(options.requisition);