Bug 866680 - gcli string params should allow empty values; r=mgoodwin,harth
authorJoe Walker <jwalker@mozilla.com>
Tue, 21 May 2013 10:18:54 +0100
changeset 143910 97892f7b402a05d57a47afc48dcb3c06d0abc346
parent 143909 e353f92a63bd8ecac1ab86a4c82473730ea2ba07
child 143911 873bf55af0cdab648bc85945d02e3bbc23304f26
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgoodwin, harth
bugs866680
milestone24.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 866680 - gcli string params should allow empty values; r=mgoodwin,harth
browser/devtools/commandline/BuiltinCommands.jsm
browser/devtools/commandline/test/browser_cmd_cookie.js
browser/devtools/commandline/test/browser_gcli_string.js
browser/devtools/commandline/test/mockCommands.js
toolkit/devtools/gcli/gcli.jsm
--- a/browser/devtools/commandline/BuiltinCommands.jsm
+++ b/browser/devtools/commandline/BuiltinCommands.jsm
@@ -970,17 +970,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
   /**
    * 'cookie list' command
    */
   gcli.addCommand({
     name: "cookie list",
     description: gcli.lookup("cookieListDesc"),
     manual: gcli.lookup("cookieListManual"),
     returnType: "cookies",
-    exec: function Command_cookieList(args, context) {
+    exec: function(args, context) {
       let host = context.environment.document.location.host;
       if (host == null || host == "") {
         throw new Error(gcli.lookup("cookieListOutNonePage"));
       }
 
       let enm = cookieMgr.getCookiesFromHost(host);
 
       let cookies = [];
@@ -1013,17 +1013,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
     manual: gcli.lookup("cookieRemoveManual"),
     params: [
       {
         name: "name",
         type: "string",
         description: gcli.lookup("cookieRemoveKeyDesc"),
       }
     ],
-    exec: function Command_cookieRemove(args, context) {
+    exec: function(args, context) {
       let host = context.environment.document.location.host;
       let enm = cookieMgr.getCookiesFromHost(host);
 
       let cookies = [];
       while (enm.hasMoreElements()) {
         let cookie = enm.getNext().QueryInterface(Ci.nsICookie);
         if (isCookieAtHost(cookie, host)) {
           if (cookie.name == args.name) {
@@ -1052,17 +1052,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
         type: "string",
         description: gcli.lookup("cookieSetValueDesc")
       },
       {
         group: gcli.lookup("cookieSetOptionsDesc"),
         params: [
           {
             name: "path",
-            type: "string",
+            type: { name: "string", allowBlank: true },
             defaultValue: "/",
             description: gcli.lookup("cookieSetPathDesc")
           },
           {
             name: "domain",
             type: "string",
             defaultValue: null,
             description: gcli.lookup("cookieSetDomainDesc")
@@ -1086,17 +1086,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
             name: "expires",
             type: "string",
             defaultValue: "Jan 17, 2038",
             description: gcli.lookup("cookieSetExpiresDesc")
           },
         ]
       }
     ],
-    exec: function Command_cookieSet(args, context) {
+    exec: function(args, context) {
       let host = context.environment.document.location.host;
       let time = Date.parse(args.expires) / 1000;
 
       cookieMgr.add(args.domain ? "." + args.domain : host,
                     args.path ? args.path : "/",
                     args.name,
                     args.value,
                     args.secure,
--- a/browser/devtools/commandline/test/browser_cmd_cookie.js
+++ b/browser/devtools/commandline/test/browser_cmd_cookie.js
@@ -73,16 +73,31 @@ function test() {
           args: {
             name: { value: 'fruit' },
             value: { value: 'ban' },
             secure: { value: false },
           }
         },
       },
       {
+        setup:    'cookie set fruit ban --path ""',
+        check: {
+          input:  'cookie set fruit ban --path ""',
+          hints:                                ' [options]',
+          markup: 'VVVVVVVVVVVVVVVVVVVVVVVVVVVVVV',
+          status: 'VALID',
+          args: {
+            name: { value: 'fruit' },
+            value: { value: 'ban' },
+            path: { value: '' },
+            secure: { value: false },
+          }
+        },
+      },
+      {
         setup: "cookie list",
         exec: {
           output: [ /zap=zep/, /zip=zop/, /Edit/ ]
         }
       },
       {
         setup: "cookie set zup banana",
         check: {
--- a/browser/devtools/commandline/test/browser_gcli_string.js
+++ b/browser/devtools/commandline/test/browser_gcli_string.js
@@ -137,16 +137,157 @@ exports.testEscape = function(options) {
           p2: { value: 'asd', arg: ' asd', status: 'VALID', message: '' },
           p3: { value: 'asd', arg: ' asd', status: 'VALID', message: '' },
         }
       }
     }
   ]);
 };
 
+exports.testBlank = function(options) {
+  helpers.audit(options, [
+    {
+      setup:    'tsrsrsr a "" c',
+      check: {
+        input:  'tsrsrsr a "" c',
+        hints:                '',
+        markup: 'VVVVVVVVVVVVVV',
+        cursor: 14,
+        current: 'p3',
+        status: 'ERROR',
+        message: '',
+        args: {
+          command: { name: 'tsrsrsr' },
+          p1: {
+            value: 'a',
+            arg: ' a',
+            status: 'VALID',
+            message: ''
+          },
+          p2: {
+            value: undefined,
+            arg: ' ""',
+            status: 'INCOMPLETE',
+            message: ''
+          },
+          p3: {
+            value: 'c',
+            arg: ' c',
+            status: 'VALID',
+            message: ''
+          }
+        }
+      }
+    },
+    {
+      setup:    'tsrsrsr a b ""',
+      check: {
+        input:  'tsrsrsr a b ""',
+        hints:                '',
+        markup: 'VVVVVVVVVVVVVV',
+        cursor: 14,
+        current: 'p3',
+        status: 'VALID',
+        message: '',
+        args: {
+          command: { name: 'tsrsrsr' },
+          p1: {
+            value: 'a',
+            arg: ' a',
+            status:'VALID',
+            message: '' },
+          p2: {
+            value: 'b',
+            arg: ' b',
+            status: 'VALID',
+            message: ''
+          },
+          p3: {
+            value: '',
+            arg: ' ""',
+            status: 'VALID',
+            message: ''
+          }
+        }
+      }
+    }
+  ]);
+};
+
+exports.testBlankWithParam = function(options) {
+  helpers.audit(options, [
+    {
+      setup:    'tsrsrsr  a --p3',
+      check: {
+        input:  'tsrsrsr  a --p3',
+        hints:                 ' <string> <p2>',
+        markup: 'VVVVVVVVVVVVVVV',
+        cursor: 15,
+        current: 'p3',
+        status: 'ERROR',
+        message: '',
+        args: {
+          command: { name: 'tsrsrsr' },
+          p1: { value: 'a', arg: '  a', status: 'VALID', message: '' },
+          p2: { value: undefined, arg: '', status: 'INCOMPLETE', message: '' },
+          p3: { value: '', arg: ' --p3', status: 'VALID', message: '' },
+        }
+      }
+    },
+    {
+      setup:    'tsrsrsr  a --p3 ',
+      check: {
+        input:  'tsrsrsr  a --p3 ',
+        hints:                  '<string> <p2>',
+        markup: 'VVVVVVVVVVVVVVVV',
+        cursor: 16,
+        current: 'p3',
+        status: 'ERROR',
+        message: '',
+        args: {
+          command: { name: 'tsrsrsr' },
+          p1: { value: 'a', arg: '  a', status: 'VALID', message: '' },
+          p2: { value: undefined, arg: '', status: 'INCOMPLETE', message: '' },
+          p3: { value: '', arg: ' --p3 ', status: 'VALID', message: '' },
+        }
+      }
+    },
+    {
+      setup:    'tsrsrsr  a --p3 "',
+      check: {
+        input:  'tsrsrsr  a --p3 "',
+        hints:                   ' <p2>',
+        markup: 'VVVVVVVVVVVVVVVVV',
+        cursor: 17,
+        current: 'p3',
+        status: 'ERROR',
+        message: '',
+        args: {
+          command: { name: 'tsrsrsr' },
+          p1: { value: 'a', arg: '  a', status: 'VALID', message: '' },
+          p2: { value: undefined, arg: '', status: 'INCOMPLETE', message: '' },
+          p3: { value: '', arg: ' --p3 "', status: 'VALID', message: '' },
+        }
+      }
+    },
+    {
+      setup:    'tsrsrsr  a --p3 ""',
+      check: {
+        input:  'tsrsrsr  a --p3 ""',
+        hints:                    ' <p2>',
+        markup: 'VVVVVVVVVVVVVVVVVV',
+        cursor: 18,
+        current: 'p3',
+        status: 'ERROR',
+        message: '',
+        args: {
+          command: { name: 'tsrsrsr' },
+          p1: { value: 'a', arg: '  a', status: 'VALID', message: '' },
+          p2: { value: undefined, arg: '', status: 'INCOMPLETE', message: '' },
+          p3: { value: '', arg: ' --p3 ""', status: 'VALID', message: '' },
+        }
+      }
+    }
+  ]);
+};
 
 
-/*
-
-
-
-*/
 // });
--- a/browser/devtools/commandline/test/mockCommands.js
+++ b/browser/devtools/commandline/test/mockCommands.js
@@ -97,17 +97,17 @@ var tsr = {
   exec: createExec('tsr')
 };
 
 var tsrsrsr = {
   name: 'tsrsrsr',
   params: [
     { name: 'p1', type: 'string' },
     { name: 'p2', type: 'string' },
-    { name: 'p3', type: 'string' },
+    { name: 'p3', type: { name: 'string', allowBlank: true} },
   ],
   exec: createExec('tsrsrsr')
 };
 
 var tso = {
   name: 'tso',
   params: [ { name: 'text', type: 'string', defaultValue: null } ],
   exec: createExec('tso')
--- a/toolkit/devtools/gcli/gcli.jsm
+++ b/toolkit/devtools/gcli/gcli.jsm
@@ -227,18 +227,22 @@ exports.shutdown = function() {
 /**
  * 'string' the most basic string type where all we need to do is to take care
  * of converting escaped characters like \t, \n, etc. For the full list see
  * https://developer.mozilla.org/en-US/docs/JavaScript/Guide/Values,_variables,_and_literals
  *
  * The exception is that we ignore \b because replacing '\b' characters in
  * stringify() with their escaped version injects '\\b' all over the place and
  * the need to support \b seems low)
+ *
+ * @param typeSpec Options object, currently obeys only one parameter:
+ * - allowBlank: Allow a blank string to be counted as valid
  */
 function StringType(typeSpec) {
+  this._allowBlank = !!typeSpec.allowBlank;
 }
 
 StringType.prototype = Object.create(Type.prototype);
 
 StringType.prototype.stringify = function(value, context) {
   if (value == null) {
     return '';
   }
@@ -255,17 +259,17 @@ StringType.prototype.stringify = functio
        .replace(/ /g, '\\ ')
        .replace(/'/g, '\\\'')
        .replace(/"/g, '\\"')
        .replace(/{/g, '\\{')
        .replace(/}/g, '\\}');
 };
 
 StringType.prototype.parse = function(arg, context) {
-  if (arg.text == null || arg.text === '') {
+  if (!this._allowBlank && (arg.text == null || arg.text === '')) {
     return Promise.resolve(new Conversion(undefined, arg, Status.INCOMPLETE, ''));
   }
 
   // The string '\\' (i.e. an escaped \ (represented here as '\\\\' because it
   // is double escaped)) is first converted to a private unicode character and
   // then at the end from \uF000 to a single '\' to avoid the string \\n being
   // converted first to \n and then to a <LF>
 
@@ -11511,17 +11515,17 @@ Completer.prototype._getCompleterTemplat
             arrowTabText = '\u21E5 ' + tabText;
           }
         }
       }
       else {
         // There's no prediction, but if this is a named argument that needs a
         // value (that is without any) then we need to show that one is needed
         // For example 'git commit --message ', clearly needs some more text
-        if (cArg.type === 'NamedArgument' && cArg.text === '') {
+        if (cArg.type === 'NamedArgument' && cArg.valueArg == null) {
           emptyParameters.push('<' + current.param.type.name + '>\u00a0');
         }
       }
     }
 
     // Add a space between the typed text (+ directTabText) and the hints,
     // making sure we don't add 2 sets of padding
     if (directTabText !== '') {