runat-1128988: Update gcli/index API to not include system

Now systems are created explicitly with createSystem, and the startup
sequence is less magic.

Signed-off-by: Joe Walker <jwalker@mozilla.com>

Author: joewalker

index.html

@@ -27,17 +27,15 @@
 
     var modules = [ 'gcli/index', 'gcli/demo', 'gcli/test/index' ];
     require(modules, function(gcli, demo, test) {
-      // Add demo commands. You'll probably want to replace this with
-      // your own set of commands
-      gcli.addItems(demo.items);
+      // Add the commands/types/converters as required
+      var system = gcli.createSystem();
+      system.addItems(gcli.items); // The basic set that most people need
+      system.addItems(demo.items); // Demo commands
 
-      // To run commands remotely, 
-      // Hook GCLI into this page, using gcli-root above
-      var options = {};
-      gcli.createTerminal(options).then(function() {
-        // Run the unit test at each startup.
-        test.run(options);
-      }).then(null, console.error);
+      gcli.createTerminal(system).then(function(terminal) {
+        terminal.language.showIntro();   // Intro text
+        test.run(terminal);              // Run the unit test at each startup
+      }).then(null, console.error.bind(console));
     });
   </script>
 

lib/gcli/connectors/direct.js

@@ -16,7 +16,7 @@
 
 'use strict';
 
-var system = require('../api').createSystem();
+var createSystem = require('../api').createSystem;
 var Commands = require('../commands/commands').Commands;
 var Types = require('../types/types').Types;
 var Requisition = require('../cli').Requisition;
@@ -63,10 +63,6 @@ var items = [
 
 ].reduce(function(prev, curr) { return prev.concat(curr); }, []);
 
-system.addItems(items);
-
-var requisition = new Requisition(system);
-
 exports.items = [
   {
     // Communicate with a 'remote' server that isn't remote at all
@@ -81,6 +77,12 @@ exports.items = [
 
 function DirectConnection() {
   this._emit = this._emit.bind(this);
+
+  // This is the 'server'
+  var system = createSystem();
+  system.addItems(items);
+  var requisition = new Requisition(system);
+
   this.remoter = new Remoter(requisition);
   this.remoter.addListener(this._emit);
 }

lib/gcli/connectors/index.js

@@ -16,7 +16,7 @@
 
 'use strict';
 
-var api = require('../api');
+var createSystem = require('../api').createSystem;
 var Commands = require('../commands/commands').Commands;
 var Types = require('../types/types').Types;
 
@@ -81,7 +81,8 @@ var items = [
 
 /**
  * These are the commands stored on the remote side that have converters which
- * we'll need to present the data
+ * we'll need to present the data. Ideally connection.specs would transfer
+ * these, that doesn't happen yet so we add them manually
  */
 var requiredConverters = [
   require('../cli').items,
@@ -107,21 +108,20 @@ var requiredConverters = [
 exports.connect = function(options) {
   options = options || {};
 
-  var system = api.createSystem();
-
+  var system = createSystem();
   system.addItems(items);
   system.addItems(requiredConverters);
 
   var connector = system.connectors.get(options.connector);
   return connector.connect(options.url).then(function(connection) {
-    options.connection = connection;
+    system.connection = connection;
     connection.on('commandsChanged', function(specs) {
       exports.addItems(system, specs, connection);
     });
 
     return connection.call('specs').then(function(specs) {
       exports.addItems(system, specs, connection);
-      return connection;
+      return system;
     });
   });
 };
@@ -144,8 +144,11 @@ exports.addLocalFunctions = function(specs, connection) {
     // in removeRemoteItems() below
     commandSpec.connection = connection;
 
+    // TODO: removeRemoteItems() doesn't remove types, so do we need this?
     commandSpec.params.forEach(function(param) {
-      param.type.connection = connection;
+      if (typeof param.type !== 'string') {
+        param.type.connection = connection;
+      }
     });
 
     if (!commandSpec.isParent) {

lib/gcli/index.js

@@ -16,7 +16,7 @@
 
 'use strict';
 
-var api = require('./api');
+var createSystem = require('./api').createSystem;
 var Terminal = require('./ui/terminal').Terminal;
 
 // Patch-up old browsers
@@ -76,32 +76,6 @@ exports.items = [
   // No commands in the basic set
 ].reduce(function(prev, curr) { return prev.concat(curr); }, []);
 
-var system = api.createSystem();
+exports.createSystem = createSystem;
 
-// Export the system API by adding it to our exports
-Object.keys(system).forEach(function(key) {
-  exports[key] = system[key];
-});
-
-system.addItems(exports.items);
-
-/**
- * createTerminal() calls 'Terminal.create()' but returns an object which
- * exposes a much restricted set of functions rather than all those exposed
- * by Terminal.
- * This allows for robust testing without exposing too many internals.
- * @param options See Terminal.create() for a description of the available
- * options.
- */
-exports.createTerminal = function(options) {
-  options = options || {};
-  if (options.settings != null) {
-    system.settings.setDefaults(options.settings);
-  }
-
-  return Terminal.create(system, options).then(function(terminal) {
-    options.terminal = terminal;
-    terminal.language.showIntro();
-    return terminal;
-  });
-};
+exports.createTerminal = Terminal.create.bind(Terminal);

lib/gcli/test/index.js

@@ -69,17 +69,18 @@ var addDebugAids = exports.addDebugAids = function(options) {
  * - Runs the unit tests automatically on startup
  * - Registers a 'test' command to re-run the unit tests
  */
-exports.run = function(options) {
-  var requisition = options.terminal.language.requisition;
-
-  options.window = window;
-  options.automator = createTerminalAutomator(options.terminal);
-  options.requisition = requisition;
-  options.isNode = false;
-  options.isFirefox = false;
-  options.isPhantomjs = (window.navigator.userAgent.indexOf('hantom') !== -1);
-  options.isRemote = (options.connection != null);
-  options.hideExec = true;
+exports.run = function(terminal) {
+  var options = {
+    terminal: terminal,
+    window: window,
+    automator: createTerminalAutomator(terminal),
+    requisition: terminal.language.requisition,
+    isNode: false,
+    isFirefox: false,
+    isPhantomjs: (window.navigator.userAgent.indexOf('hantom') !== -1),
+    isRemote: (terminal.system.connection != null),
+    hideExec: true
+  };
 
   addDebugAids(options);
 
@@ -92,7 +93,7 @@ exports.run = function(options) {
 
   // PhantomJS may tell us to tell the server to shutdown
   if (window.location.href.indexOf('shutdown=true') > 0) {
-    shutdownServer(requisition).then(closeIfPhantomJs, closeIfPhantomJs);
+    shutdownServer(terminal.language.requisition).then(closeIfPhantomJs, closeIfPhantomJs);
     return;
   }
 

lib/gcli/ui/terminal.js

@@ -47,6 +47,7 @@ function Terminal() {
  * - document
  */
 Terminal.create = function(system, options) {
+  options = options || {};
   if (resourcesPromise == null) {
     resourcesPromise = Promise.all([
       host.staticRequire(module, './terminal.css'),

remote.html

@@ -28,9 +28,9 @@
 
     require([ 'gcli/index', 'gcli/test/index', 'gcli/connectors/index' ], function(gcli, test, cnx) {
       var options = { connector: 'websocket' };
-      cnx.connect(options).then(function() {
-        gcli.createTerminal(options).then(function() {
-          test.run(options);
+      cnx.connect(options).then(function(system) {
+        gcli.createTerminal(system).then(function(terminal) {
+          test.run(terminal);
         });
       }).then(null, console.error.bind(console));
     });