Better list option parsing and serialization.

common/content/commandline.js

@@ -1071,6 +1071,7 @@ const CommandLine = Module("commandline", {
          */
         select: function (backward, matchCurrent) {
             // always reset the tab completion if we use up/down keys
+            if (commandline._completions)
                 commandline._completions.reset();
 
             let diff = backward ? -1 : 1;

common/content/commands.js

@@ -189,7 +189,15 @@ const Command = Class("Command", {
      * @returns {Args}
      * @see Commands#parseArgs
      */
-    parseArgs: function (args, complete, extra) commands.parseArgs(args, this.options, this.argCount, false, this.literal, complete, extra),
+    parseArgs: function (args, complete, extra) commands.parseArgs(args, {
+        allowUnknownOptions: !!this.allowUnknownOptions,
+        argCount: this.argCount,
+        complete: complete,
+        extra: extra,
+        keepQuotes: !!this.keepQuotes,
+        literal: this.literal,
+        options: this.options
+    }),
 
     /**
      * @property {string[]} All of this command's name specs. e.g., "com[mand]"
@@ -405,16 +413,15 @@ const Commands = Module("commands", {
      */
     commandToString: function (args) {
         let res = [args.command + (args.bang ? "!" : "")];
-        function quote(str) Commands.quoteArg[/[\s"'\\]|^$/.test(str) ? "'" : ""](str);
 
         for (let [opt, val] in Iterator(args.options || {})) {
             let chr = /^-.$/.test(opt) ? " " : "=";
             if (val != null)
-                opt += chr + quote(val);
+                opt += chr + Commands.quote(val);
             res.push(opt);
         }
         for (let [, arg] in Iterator(args.arguments || []))
-            res.push(quote(arg));
+            res.push(Commands.quote(arg));
 
         let str = args.literalArg;
         if (str)
@@ -556,7 +563,7 @@ const Commands = Module("commands", {
      */
     parseArgs: function (str, options, argCount, allowUnknownOptions, literal, complete, extra) {
         function getNextArg(str) {
-            let [count, arg, quote] = Commands.parseArg(str);
+            let [count, arg, quote] = Commands.parseArg(str, null, keepQuotes);
             if (quote == "\\" && !complete)
                 return [,,,"Trailing \\"];
             if (quote && !complete)
@@ -564,6 +571,11 @@ const Commands = Module("commands", {
             return [count, arg, quote];
         }
 
+        let keepQuotes;
+
+        if (isObject(options))
+            ({ allowUnknownOptions, argCount, complete, extra, literal, options, keepQuotes }) = options;
+
         if (!options)
             options = [];
 
@@ -581,7 +593,7 @@ const Commands = Module("commands", {
 
         var invalid = false;
         // FIXME: best way to specify these requirements?
-        var onlyArgumentsRemaining = allowUnknownOptions || options.length == 0 || false; // after a -- has been found
+        var onlyArgumentsRemaining = allowUnknownOptions || options.length == 0; // after a -- has been found
         var arg = null;
         var count = 0; // the length of the argument
         var i = 0;
@@ -866,19 +878,26 @@ const Commands = Module("commands", {
     }
 }, {
     // returns [count, parsed_argument]
-    parseArg: function (str) {
+    parseArg: function (str, sep, keepQuotes) {
         let arg = "";
         let quote = null;
         let len = str.length;
 
-        while (str.length && !/^\s/.test(str)) {
+        // Fix me.
+        if (isString(sep))
+            sep = RegExp(sep);
+        sep = sep != null ? sep : /\s/;
+        let re1 = RegExp("^" + (sep.source === "" ? "(?!)" : sep.source));
+        let re2 = RegExp(/^()((?:[^\\S"']|\\.)+)((?:\\$)?)/.source.replace("S", sep.source));
+
+        while (str.length && !re1.test(str)) {
             let res;
-            if ((res = str.match = str.match(/^()((?:[^\\\s"']|\\.)+)((?:\\$)?)/)))
+            if ((res = re2.exec(str)))
-                arg += res[2].replace(/\\(.)/g, "$1");
+                arg += keepQuotes ? res[0] : res[2].replace(/\\(.)/g, "$1");
-            else if ((res = str.match(/^(")((?:[^\\"]|\\.)*)("?)/)))
+            else if ((res = /^(")((?:[^\\"]|\\.)*)("?)/.exec(str)))
-                arg += eval(res[0] + (res[3] ? "" : '"'));
+                arg += keepQuotes ? res[0] : eval(res[0] + (res[3] ? "" : '"'));
-            else if ((res = str.match(/^(')((?:[^']|'')*)('?)/)))
+            else if ((res = /^(')((?:[^']|'')*)('?)/.exec(str)))
-                arg += res[2].replace("''", "'", "g");
+                arg += keepQuotes ? res[0] : res[2].replace("''", "'", "g");
             else
                 break;
 
@@ -890,7 +909,9 @@ const Commands = Module("commands", {
         }
 
         return [len - str.length, arg, quote];
-    }
+    },
+
+    quote: function quote(str) Commands.quoteArg[/[\s"'\\]|^$/.test(str) ? "'" : ""](str)
 }, {
     completion: function () {
         completion.command = function command(context) {
@@ -1155,8 +1176,9 @@ const Commands = Module("commands", {
 (function () {
 
     Commands.quoteMap = {
-        "\n": "n",
+        "\n": "\\n",
-        "\t": "t"
+        "\t": "\\t",
+        "'":  "''"
     };
     function quote(q, list) {
         let re = RegExp("[" + list + "]", "g");
@@ -1166,14 +1188,14 @@ const Commands = Module("commands", {
     };
     Commands.complQuote = {
         '"': ['"', quote("", '\n\t"\\\\'), '"'],
-        "'": ["'", quote("", "\\\\'"), "'"],
+        "'": ["'", quote("", "'"), "'"],
         "":  ["", quote("",  "\\\\ '\""), ""]
     };
 
     Commands.quoteArg = {
         '"': quote('"', '\n\t"\\\\'),
-        "'": quote("'", "\\\\'"),
+        "'": quote("'", "'"),
-        "":  quote("",  "\\\\ '\"")
+        "":  quote("",  "\\\\\\s'\"")
     };
 
     Commands.parseBool = function (arg) {

common/content/dactyl.js

@@ -1291,7 +1291,7 @@ const Dactyl = Module("dactyl", {
 
         options.add(["loadplugins", "lpl"],
             "A regex list that defines which plugins are loaded at startup and via :loadplugins",
-            "regexlist", "\\.(js|vimp)$");
+            "regexlist", "'\\.(js|vimp)$'");
 
         options.add(["titlestring"],
             "Change the title of the window",

common/content/finder.js

@@ -211,7 +211,7 @@ const RangeFinder = Module("rangefinder", {
 
         options.add(["hlsearch", "hls"],
             "Highlight previous search pattern matches",
-            "boolean", "false", {
+            "boolean", false, {
                 setter: function (value) {
                     try {
                         if (value)

common/content/options.js

@@ -50,8 +50,11 @@ const Option = Class("Option", {
 
         this._op = Option.ops[this.type];
 
-        if (arguments.length > 3)
+        if (arguments.length > 3) {
-            this.defaultValue = defaultValue;
+            if (this.type == "string")
+                defaultValue = Commands.quote(defaultValue);
+            this.defaultValue = this.joinValues(this.parseValues(defaultValue));
+        }
 
         if (extraInfo)
             update(this, extraInfo);
@@ -75,7 +78,7 @@ const Option = Class("Option", {
      * @param {value} value The option value.
      * @returns {value|string[]}
      */
-    parseValues: function (value) value,
+    parseValues: function (value) Option.dequote(value),
 
     /**
      * Returns <b>values</b> packed in the appropriate format for the option
@@ -84,7 +87,7 @@ const Option = Class("Option", {
      * @param {value|string[]} values The option value.
      * @returns {value}
      */
-    joinValues: function (vals) vals,
+    joinValues: function (vals) Commands.quote(vals),
 
     /** @property {value|string[]} The option value or array of values. */
     get values() this.getValues(this.scope),
@@ -376,8 +379,8 @@ const Option = Class("Option", {
         re.toString = function () Option.unparseRegex(this);
         return re;
     },
-    unparseRegex: function (re) re.bang + re.source.replace(/\\(.)/g, function (m, n1) n1 == "/" ? n1 : m) +
+    unparseRegex: function (re) re.bang + Option.quote(re.source.replace(/\\(.)/g, function (m, n1) n1 == "/" ? n1 : m)) +
-        (typeof re.result == "string" ? ":" + re.result : ""),
+        (typeof re.result == "string" ? ":" + Option.quote(re.result) : ""),
 
     getKey: {
         stringlist: function (k) this.values.indexOf(k) >= 0,
@@ -393,24 +396,46 @@ const Option = Class("Option", {
     },
 
     joinValues: {
-        charlist:    function (vals) vals.join(""),
+        charlist:    function (vals) Commands.quote(vals.join("")),
-        stringlist:  function (vals) vals.join(","),
+        stringlist:  function (vals) vals.map(Option.quote).join(","),
-        stringmap:   function (vals) [k + ":" + v for ([k, v] in Iterator(vals))].join(","),
+        stringmap:   function (vals) [Option.quote(k) + ":" + Option.quote(v) for ([k, v] in Iterator(vals))].join(","),
-        regexlist:   function (vals) vals.map(Option.unparseRegex).join(","),
+        regexlist:   function (vals) vals.join(","),
         get regexmap() this.regexlist
     },
 
     parseValues: {
-        number:     function (value) Number(value),
+        number:     function (value) Number(Option.dequote(value)),
-        boolean:    function (value) value == "true" || value == true ? true : false,
+        boolean:    function (value) Option.dequote(value) == "true" || value == true ? true : false,
-        charlist:   function (value) Array.slice(value),
+        charlist:   function (value) Array.slice(Option.dequote(value)),
-        stringlist: function (value) (value === "") ? [] : value.split(","),
+        stringlist: function (value) (value === "") ? [] : Option.splitList(value),
-        stringmap:  function (value) array(util.split(v, /:/g, 2) for (v in values(value.split(",")))).toObject(),
+        stringmap:  function (value) array(util.split(v, /:/g, 2) for (v in values(Option.splitList(value)))).toObject(),
-        regexlist:  function (value) (value === "") ? [] : value.split(",").map(Option.parseRegex),
+        regexlist:  function (value) (value === "") ? [] : Option.splitList(value).map(Option.parseRegex),
-        regexmap:   function (value) value.split(",").map(function (v) util.split(v, /:/g, 2))
+        regexmap:   function (value) Option.splitList(value)
+                                           .map(function (v) util.split(v, /:/g, 2))
                                            .map(function ([k, v]) v != null ? Option.parseRegex(k, v) : Option.parseRegex(".?", k))
     },
 
+    dequote: function (value) {
+        let arg;
+        [, arg, Option._quote] = Commands.parseArg(String(value), "");
+        Option._splitAt = 0;
+        return arg;
+    },
+    splitList: function (value) {
+        let res = [];
+        Option._splitAt = 0;
+        do {
+            if (count !== undefined)
+                Option._splitAt += count + 1;
+            var [count, arg, quote] = Commands.parseArg(value, /,/);
+            Option._quote = quote; // FIXME
+            res.push(arg);
+            value = value.slice(count + 1);
+        } while (value.length);
+        return res;
+    },
+    quote: function quote(str) Commands.quoteArg[/[\s"'\\,]|^$/.test(str) ? "'" : ""](str),
+
     ops: {
         boolean: function (operator, values, scope, invert) {
             if (operator != "=")
@@ -640,7 +665,7 @@ const Options = Module("options", {
         if (!scope)
             scope = Option.SCOPE_BOTH;
 
-        if (name in this._optionMap && (this._optionMap[name].scope & scope))
+        if (this._optionMap[name] && (this._optionMap[name].scope & scope))
             return this._optionMap[name];
         return null;
     },
@@ -661,7 +686,7 @@ const Options = Module("options", {
         function opts(opt) {
             for (let opt in Iterator(options)) {
                 let option = {
-                    isDefault: opt.value == opt.defaultValue,
+                    isDefault: opt.value === opt.defaultValue,
                     name:      opt.name,
                     default:   opt.defaultValue,
                     pre:       "\u00a0\u00a0", // Unicode nonbreaking space.
@@ -1261,7 +1286,7 @@ const Options = Module("options", {
                     serialize: function () [
                         {
                             command: this.name,
-                            arguments: [opt.type == "boolean" ? (opt.value ? "" : "no") + opt.name
+                            literalArg: [opt.type == "boolean" ? (opt.value ? "" : "no") + opt.name
                                                                : opt.name + "=" + opt.value]
                         }
                         for (opt in options)
@@ -1276,6 +1301,9 @@ const Options = Module("options", {
                 },
                 update({
                     bang: true,
+                    completer: function (context, args) {
+                        return setCompleter(context, args);
+                    },
                     domains: function (args) array.flatten(args.map(function (spec) {
                         try {
                             let opt = options.parseOpt(spec);
@@ -1287,9 +1315,7 @@ const Options = Module("options", {
                         }
                         return [];
                     })),
-                    completer: function (context, args) {
+                    keepQuotes: true,
-                        return setCompleter(context, args);
-                    },
                     privateData: function (args) args.some(function (spec) {
                         let opt = options.parseOpt(spec);
                         return opt.option && opt.option.privateData &&
@@ -1347,33 +1373,30 @@ const Options = Module("options", {
                 return;
             }
 
-            let len = context.filter.length;
             switch (opt.type) {
             case "boolean":
                 if (!completer)
                     completer = function () [["true", ""], ["false", ""]];
                 break;
             case "regexlist":
-                newValues = context.filter.split(",");
+                newValues = Option.splitList(context.filter);
                 // Fallthrough
             case "stringlist":
-                let target = newValues.pop() || "";
+                var target = newValues.pop() || "";
-                len = target.length;
                 break;
             case "stringmap":
             case "regexmap":
-                let vals = context.filter.split(",");
+                let vals = Option.splitList(context.filter);
                 target = vals.pop() || "";
-                len = target.length - (target.indexOf(":") + 1);
+                Option._splitAt += target.indexOf(":") ? target.indexOf(":") + 1 : 0;
-                break;
-            case "charlist":
-                len = 0;
                 break;
             }
             // TODO: Highlight when invalid
-            context.advance(context.filter.length - len);
+            context.advance(Option._splitAt);
+            context.filter = target != null ? target : Option.dequote(context.filter);
 
             context.title = ["Option Value"];
+            context.quote = Commands.complQuote[Option._quote] || Commands.complQuote[""]
             // Not Vim compatible, but is a significant enough improvement
             // that it's worth breaking compatibility.
             if (isArray(newValues)) {

common/locale/en-US/options.xml

@@ -30,14 +30,21 @@
     <dt>number</dt>        <dd>A numeric value</dd>
     <dt>string</dt>        <dd>A string value</dd>
     <dt>charlist</dt>      <dd tag="charlist">A string containing a discrete set of distinct characters</dd>
-    <dt>stringlist</dt>    <dd tag="stringlist">A comma-separated list of strings</dd>
+    <dt>stringlist</dt>
+    <dd tag="stringlist">
+        A comma-separated list of strings. Any comma appearing within single
+        or double quotes, or prefixed with a <em>\</em>, will not be treated
+        as an item separator.
+    </dd>
     <dt>stringmap</dt>     <dd tag="stringmap">A comma-separated list of key-value pairs, e.g., <str>key:val,foo:bar</str></dd>
     <dt>regexlist</dt>
     <dd tag="regexlist">
         A comma-separated list of regular expressions. Expressions may be
         prefixed with a <em>!</em>, in which case the match will be negated. A
         literal <em>!</em> at the begining of the expression may be matched with
-        <em>[!]</em>.  Generally, the first matching regular expression is used.
+        <em>[!]</em>.  Generally, the first matching regular expression is
+        used. Any comma appearing within single or double quotes, or prefixed
+        with a <em>\</em>, will not be treated as an item separator.
     </dd>
     <dt>regexmap</dt>
     <dd tag="regexmap">
@@ -1452,10 +1459,6 @@
         <code><ex>:set wildignore=<str delim="'">\.o$</str>,<str delim="'">^\..*\.s[a-z]<a>2</a>$</str></ex></code>
 
         <note>Unlike Vim, each pattern is a regular expression rather than a glob.</note>
-        <note>
-            The only way to include a literal comma in a pattern is with the
-            escape sequence <str>\u0044</str>.
-        </note>
     </description>
 </item>
 

common/modules/base.jsm

@@ -474,8 +474,9 @@ function call(fn) {
  * value of the property.
  */
 function memoize(obj, key, getter) {
-    obj.__defineGetter__(key, function ()
+    obj.__defineGetter__(key, function () (
-        Class.replaceProperty(this, key, getter.call(this, key)));
+        Class.replaceProperty(this, key, null),
+        Class.replaceProperty(this, key, getter.call(this, key))));
 }
 
 /**

pentadactyl/NEWS

@@ -26,6 +26,11 @@
       I.e., 'fo\o''bar' ≡ fo\o'bar
     * IMPORTANT: 'cdpath' and 'runtimepath' no longer treat ‘,,’
       specially. Use ‘.’ instead.
+    * IMPORTANT: Option value quoting has changed. List options will
+      no longer be split at quoted commas and the option name,
+      operators, and = sign may no longer be quoted. This will break
+      certain automatically-generated configuration files.
+      See :help stringlist
     * Added 'altwildmode' and <A-Tab> commandline key binding.
     * Added 'autocomplete' option for specifying which completion
       groups should be auto-completed.