Bug 1253221 - Sync changes from GCLI web project; r=jryans draft
authorJoe Walker <jwalker@mozilla.com>
Thu, 03 Mar 2016 12:42:52 +0000
changeset 362896 61396447a5cf0ea058e9e17f321fa12eca1b5025
parent 362895 1e8d203b0d6b8908461bf4f6fbf1edb94b2a6aa0
child 362897 56ba922c54ff7c1d1fb1f3ac13d50f4194a7a09a
push id17055
push userbmo:jwalker@mozilla.com
push dateTue, 03 May 2016 14:10:31 +0000
reviewersjryans
bugs1253221
milestone49.0a1
Bug 1253221 - Sync changes from GCLI web project; r=jryans Changes in this patch: * GCLI gave up with testing via phantomjs a while ago, but the docs didn't keep up, so this fixes the docs. * gcli/languages/command isn't used in Firefox, but on the web if a converter wrongly produces a null DOM node, then the UI just looks blank, so this change makes the error more obvious. However we don't use this in Firefox. I'd like to remove the module, but not in this patch. * The delegate type (used when we want to defer the type of a parameter, e.g. with "pref set PREFNAME VALUE" where the type of value depends on PREFNAME) forced children to be prediction-less. This allows them to decide for themselves * The file type assumed that the local filesystem was the same as the remote one which is clearly wrong. The change to gcli/types/file fixes that. * Typo fix to gcli/util/util * On very slow connections gcli/cli.js could get in a pickle where 2 changes happened out of order. We've planned for this, but got in wrong, so this just adds a bit of defensive programming. MozReview-Commit-ID: H88W5UDCikM
devtools/shared/gcli/source/docs/running-tests.md
devtools/shared/gcli/source/docs/writing-tests.md
devtools/shared/gcli/source/lib/gcli/cli.js
devtools/shared/gcli/source/lib/gcli/languages/command.js
devtools/shared/gcli/source/lib/gcli/types/delegate.js
devtools/shared/gcli/source/lib/gcli/types/file.js
devtools/shared/gcli/source/lib/gcli/util/util.js
--- a/devtools/shared/gcli/source/docs/running-tests.md
+++ b/devtools/shared/gcli/source/docs/running-tests.md
@@ -43,27 +43,16 @@ Or, using the `test` command:
     testCompletion: Pass (funcs=1, checks=139)
     testExec: Pass (funcs=1, checks=133)
     testHistory: Pass (funcs=3, checks=13)
     ....
     
     Summary: Pass (951 checks)
 
 
-# Phantom
-
-The GCLI test suite can also be run under PhantomJS as follows:
-
-    $ phantomjs ./phantom-test.js
-    
-    Summary: Pass (4289 checks)
-    
-    Finished running unit tests. (total 3.843s, ave response time 3.36ms, ...)
-
-
 # Travis CI
 
 GCLI check-ins are automatically tested by [Travis CI](https://travis-ci.org/joewalker/gcli).
 
 
 # Test Case Generation
 
 GCLI can generate test cases automagically. Load ```localtest.html```, type a
--- a/devtools/shared/gcli/source/docs/writing-tests.md
+++ b/devtools/shared/gcli/source/docs/writing-tests.md
@@ -2,17 +2,17 @@
 # Writing Tests
 
 There are several sources of GCLI tests and several environments in which they
 are run.
 
 The majority of GCLI tests are stored in
 [this repository](https://github.com/joewalker/gcli/) in files named like
 ```./lib/gclitest/test*.js```. These tests run in Firefox, Chrome, Opera,
-PhantomJS, and NodeJS/JsDom
+and NodeJS/JsDom
 
 See [Running Tests](running-tests.md) for further details.
 
 GCLI comes with a generic unit test harness (in ```./lib/test/```) and a
 set of helpers for creating GCLI tests (in ```./lib/gclitest/helpers.js```).
 
 # GCLI tests in Firefox
 
--- a/devtools/shared/gcli/source/lib/gcli/cli.js
+++ b/devtools/shared/gcli/source/lib/gcli/cli.js
@@ -1163,17 +1163,19 @@ Requisition.prototype.getInputStatusMark
   cursor = cursor === 0 ? 0 : cursor - 1;
   var cTrace = argTraces[cursor];
 
   var markup = [];
   for (var i = 0; i < argTraces.length; i++) {
     var argTrace = argTraces[i];
     var arg = argTrace.arg;
     var status = Status.VALID;
-    if (argTrace.part === 'text') {
+    // When things get very async we can get here while something else is
+    // doing an update, in which case arg.assignment == null, so we check first
+    if (argTrace.part === 'text' && arg.assignment != null) {
       status = arg.assignment.getStatus(arg);
       // Promote INCOMPLETE to ERROR  ...
       if (status === Status.INCOMPLETE) {
         // If the cursor is in the prefix or suffix of an argument then we
         // don't consider it in the argument for the purposes of preventing
         // the escalation to ERROR. However if this is a NamedArgument, then we
         // allow the suffix (as space between 2 parts of the argument) to be in.
         // We use arg.assignment.arg not arg because we're looking at the arg
--- a/devtools/shared/gcli/source/lib/gcli/languages/command.js
+++ b/devtools/shared/gcli/source/lib/gcli/languages/command.js
@@ -481,26 +481,31 @@ var commandLanguage = exports.commandLan
       }
       if (ev.output.error) {
         data.promptEle.classList.add('gcli-row-error');
       }
 
       util.clearElement(data.rowoutEle);
 
       return ev.output.convert('dom', context).then(function(node) {
+        this.terminal.scrollToBottom();
+        data.throbEle.style.display = ev.output.completed ? 'none' : 'block';
+
+        if (node == null) {
+          data.promptEle.classList.add('gcli-row-error');
+          // TODO: show some error to the user
+        }
+
         this._linksToNewTab(node);
         data.rowoutEle.appendChild(node);
 
         var event = document.createEvent('Event');
         event.initEvent('load', true, true);
         event.addedElement = node;
         node.dispatchEvent(event);
-
-        this.terminal.scrollToBottom();
-        data.throbEle.style.display = ev.output.completed ? 'none' : 'block';
       }.bind(this));
     }.bind(this)).catch(console.error);
 
     this.terminal.addElement(data.rowinEle);
     this.terminal.addElement(data.rowoutEle);
     this.terminal.scrollToBottom();
 
     this.focusManager.outputted();
--- a/devtools/shared/gcli/source/lib/gcli/types/delegate.js
+++ b/devtools/shared/gcli/source/lib/gcli/types/delegate.js
@@ -82,16 +82,17 @@ exports.items = [
     // change the function definition to accommodate this right now
     isImportant: false
   },
   {
     item: 'type',
     name: 'remote',
     paramName: undefined,
     blankIsValid: false,
+    hasPredictions: true,
 
     getSpec: function(commandName, paramName) {
       return {
         name: 'remote',
         commandName: commandName,
         paramName: paramName,
         blankIsValid: this.blankIsValid
       };
--- a/devtools/shared/gcli/source/lib/gcli/types/file.js
+++ b/devtools/shared/gcli/source/lib/gcli/types/file.js
@@ -58,25 +58,21 @@ exports.items = [
       }
 
       if (this.existing !== 'yes' && this.existing !== 'no' &&
           this.existing !== 'maybe') {
         throw new Error('existing must be one of [yes|no|maybe]');
       }
     },
 
-    getSpec: function() {
-      var matches = (typeof this.matches === 'string' || this.matches == null) ?
-                    this.matches :
-                    this.matches.source; // Assume RegExp
+    getSpec: function(commandName, paramName) {
       return {
-        name: 'file',
-        filetype: this.filetype,
-        existing: this.existing,
-        matches: matches
+        name: 'remote',
+        commandName: commandName,
+        paramName: paramName
       };
     },
 
     stringify: function(file) {
       if (file == null) {
         return '';
       }
 
--- a/devtools/shared/gcli/source/lib/gcli/util/util.js
+++ b/devtools/shared/gcli/source/lib/gcli/util/util.js
@@ -171,17 +171,17 @@ exports.createEvent = function(name) {
     return new Promise(function(resolve, reject) {
       var handler = function(arg) {
         event.remove(handler);
         resolve(arg);
       };
 
       event.add(handler);
     });
-  },
+  };
 
   /**
    * Temporarily prevent this event from firing.
    * @see resumeFire(ev)
    */
   event.holdFire = function() {
     if (eventDebug) {
       console.group('Holding fire: ' + name);