better args parser, plugin writers maybe need to change there

liberator.commands.add() slightly if they used args: as an extraInfo

Donators

@@ -2,6 +2,8 @@
 <b>Note:</b> If you don't wish to appear on this list when making a donation, please tell me.
 
 2008:
+* Anton Kovalenko
+* Paulo Tanimoto
 * Paul Sobey
 * Olivier Guéry
 * Dotan Cohen

NEWS

@@ -6,6 +6,8 @@
     * IMPORTANT: input fields are not blured anymore by default after a page has loaded
       use :set [no]focuscontent to control the behavior (thanks Paul Sobey for the 
       generous donation which made this behavior possible)
+    * new argument parser for ex commands, should tell better error messages when
+      you do things like :bmark -tag=a,b instead of :bmark -tags=a,b
     * :bdelete accepts an optional argument now
     * renamed some :autocmd, mainly BrowserStartup -> Startup and BrowserExit -> Quit
     * don't pass any ctrl- or alt- prefixed keys to firefox in insert mode

TODO

@@ -47,6 +47,7 @@ FEATURES:
 4 y and Y could maybe changed to, but probably not: Y, yy and yl=yank location, ys=yank selection,
   yd=yank domain name, yt=yank title, yw=yank current word, yf=yank filename, (other things to yank?)
 4 Add -nargs, -complete, etc. to :command
+4 :set urlseparators=,\s+
 4 } { should jump to the next paragraph of the page (maybe impossible)
 3 Splitting Windows with [:sp :vsp ctrl-w,s ctrl-w,v] and closing with [ctrl-w,q], moving with [ctrl-w,w or tab]
   have a look into the split browser extension

content/addressbook.js

@@ -112,20 +112,10 @@ liberator.Addressbook = function () //{{{
         "Add an address book entry",
         function (args)
         {
-            var res = liberator.commands.parseArgs(args, this.args);
+            var mailAddr =    args.arguments[0]; // TODO: support more than one email address
-            if (!res)
+            var firstName =   args["-firstname"] || null;
-                return;
+            var lastName =    args["-lastname"] || null;
-
+            var displayName = args["-name"] || null;
-            if (res.args.length == 0)
-            {
-                liberator.echoerr("E474: Invalid argument");
-                return;
-            }
-
-            var mailAddr = res.args[0]; // TODO: support more than one email address
-            var firstName = liberator.commands.getOption(res.opts, "-firstname", null);
-            var lastName = liberator.commands.getOption(res.opts, "-lastname", null);
-            var displayName = liberator.commands.getOption(res.opts, "-name", null);
             if (!displayName)
                 displayName = generateDisplayName(firstName, lastName);
 
@@ -136,9 +126,10 @@ liberator.Addressbook = function () //{{{
 
         },
         {
-            args: [[["-firstname", "-f"], liberator.commands.OPTION_STRING],
+            options: [[["-firstname", "-f"], liberator.commands.OPTION_STRING],
                       [["-lastname", "-l"],  liberator.commands.OPTION_STRING],
-                   [["-name", "-n"],      liberator.commands.OPTION_STRING]]
+                      [["-name", "-n"],      liberator.commands.OPTION_STRING]],
+            argCount: "+"
         });
 
     liberator.commands.add(["contacts", "addr[essbook]"],

content/bookmarks.js

@@ -133,16 +133,12 @@ liberator.Bookmarks = function () //{{{
         "Add a bookmark",
         function (args)
         {
-            var res = liberator.commands.parseArgs(args, this.args);
+            var url = args.arguments.length == 0 ? liberator.buffer.URL : args.arguments[0];
-            if (!res)
+            var title = args["-title"] || (args.arguments.length == 0 ? liberator.buffer.title : null);
-                return;
-
-            var url = res.args.length == 0 ? liberator.buffer.URL : res.args[0];
-            var title = liberator.commands.getOption(res.opts, "-title", res.args.length == 0 ? liberator.buffer.title : null);
             if (!title)
                 title = url;
-            var keyword = liberator.commands.getOption(res.opts, "-keyword", null);
+            var keyword = args["-keyword"] || null;
-            var tags = liberator.commands.getOption(res.opts, "-tags", []);
+            var tags =    args["-tags"] || [];
 
             if (liberator.bookmarks.add(false, title, url, keyword, tags))
             {
@@ -155,25 +151,21 @@ liberator.Bookmarks = function () //{{{
                 liberator.echoerr("Exxx: Could not add bookmark `" + title + "'", liberator.commandline.FORCE_SINGLELINE);
         },
         {
-            args: [[["-title", "-t"],    liberator.commands.OPTION_STRING],
+            options: [[["-title", "-t"],    liberator.commands.OPTION_STRING],
                       [["-tags", "-T"],     liberator.commands.OPTION_LIST],
-                   [["-keyword", "-k"],  liberator.commands.OPTION_STRING, function (arg) { return /\w/.test(arg); }]]
+                      [["-keyword", "-k"],  liberator.commands.OPTION_STRING, function (arg) { return /\w/.test(arg); }]],
+            argCount: "?"
         });
 
     liberator.commands.add(["bmarks"],
         "List or open multiple bookmarks",
         function (args, special)
         {
-            var res = liberator.commands.parseArgs(args, this.args);
+            liberator.bookmarks.list(args.arguments.join(" "), args["-tags"] || [], special);
-            if (!res)
-                return;
-
-            var tags = liberator.commands.getOption(res.opts, "-tags", []);
-            liberator.bookmarks.list(res.args.join(" "), tags, special);
         },
         {
             completer: function (filter) { return [0, liberator.bookmarks.get(filter)]; },
-            args: [[["-tags", "-T"], liberator.commands.OPTION_LIST]]
+            options: [[["-tags", "-T"], liberator.commands.OPTION_LIST]]
         });
 
     liberator.commands.add(["delbm[arks]"],

content/commands.js

@@ -75,7 +75,8 @@ liberator.Command = function (specs, description, action, extraInfo) //{{{
     this.description   = description || "";
     this.action        = action;
     this.completer     = extraInfo.completer || null;
-    this.args          = extraInfo.args || [];
+    this.options       = extraInfo.options || [];
+    this.argCount      = extraInfo.argCount || "";
     this.isUserCommand = extraInfo.isUserCommand || false;
 };
 
@@ -83,6 +84,15 @@ liberator.Command.prototype = {
 
     execute: function (args, special, count, modifiers)
     {
+        // whenever the user specifies special options or fixed number of arguments
+        // we use our args parser instead of passing a string to the callback
+        if (this.options.length > 0 || this.argCount)
+        {
+            args = liberator.commands.parseArgs(args, this.options, this.argCount);
+            if (args == null)
+                return false;
+        }
+
         return this.action.call(this, args, special, count, modifiers);
     },
 
@@ -128,10 +138,133 @@ liberator.Commands = function () //{{{
 
     var exCommands = [];
 
+    function getUserCommands(name)
+    {
+        var matches = [];
+        for (var i = 0; i < exCommands.length; i++)
+        {
+            if (exCommands[i].isUserCommand)
+            {
+                if (name)
+                {
+                    if (exCommands[i].name.match("^" + name))
+                        matches.push(exCommands[i]);
+                }
+                else
+                {
+                    matches.push(exCommands[i]);
+                }
+            }
+        }
+        return matches;
+    }
+
+    function removeUserCommand(name)
+    {
+        for (var i = 0; i < exCommands.length; i++)
+        {
+            if (exCommands[i].isUserCommand && exCommands[i].name == name)
+            {
+                exCommands.splice(i, 1);
+                break;
+            }
+        }
+    }
+
+    /////////////////////////////////////////////////////////////////////////////}}}
+    ////////////////////// PUBLIC SECTION //////////////////////////////////////////
+    /////////////////////////////////////////////////////////////////////////////{{{
+
+    var commandManager = {
+
+        // FIXME: remove later, when our option handler is better
+        //        Idea: If v.commands.add() specifies args or opts in extraInfo, don't call the function
+        //        with args as a string, but already pass an object like:
+        //        args = { -option: value, -anotheroption: true, arguments: [] }
+        OPTION_ANY: 0, // can be given no argument or an argument of any type,
+                       // caller is responsible for parsing the return value
+        OPTION_NOARG: 1,
+        OPTION_BOOL:2,
+        OPTION_STRING: 3,
+        OPTION_INT: 4,
+        OPTION_FLOAT: 5,
+        OPTION_LIST: 6,
+
+        __iterator__: function ()
+        {
+            var sorted = exCommands.sort(function (cmd1, cmd2) { return cmd1.name > cmd2.name; });
+            for (var i = 0; i < sorted.length; i++)
+                yield sorted[i];
+
+            throw StopIteration;
+        },
+
+        add: function (names, description, action, extra)
+        {
+            var command = new liberator.Command(names, description, action, extra);
+            if (!command)
+                return false;
+
+            for (var i = 0; i < exCommands.length; i++)
+            {
+                if (exCommands[i].name == command.name)
+                {
+                    // never replace for now
+                    liberator.log("Warning: :" + names[0] + " already exists, NOT replacing existing command.", 1);
+                    return false;
+                }
+            }
+
+            exCommands.push(command);
+            return true;
+        },
+
+        addUserCommand: function (names, description, action, extra, replace)
+        {
+            var extra = extra || {};
+            extra.isUserCommand = true;
+            description = description || "User defined command";
+
+            var command = new liberator.Command(names, description, action, extra);
+            if (!command)
+                return false;
+
+            for (var i = 0; i < exCommands.length; i++)
+            {
+                if (exCommands[i].name == command.name)
+                {
+                    if (!replace)
+                    {
+                        return false;
+                    }
+                    else
+                    {
+                        removeUserCommand(command.name);
+                        break;
+                    }
+                }
+            }
+
+            exCommands.push(command);
+            return true;
+        },
+
+        get: function (name)
+        {
+            for (var i = 0; i < exCommands.length; i++)
+            {
+                if (exCommands[i].hasName(name))
+                    return exCommands[i];
+            }
+
+            return null;
+        },
+
         // in '-quoted strings, only ' and \ itself are escaped
         // in "-quoted strings, also ", \n and \t are translated
         // in non-quoted strings everything is taken literally apart from "\ " and "\\"
         //
+        // @param str: something like "-x=foo -opt=bar arg1 arg2"
         // "options" is an array [name, type, validator, completions] and could look like:
         //  options = [[["-force"], OPTION_NOARG],
         //             [["-fullscreen", "-f"], OPTION_BOOL],
@@ -140,9 +273,14 @@ liberator.Commands = function () //{{{
         //             [["-acceleration"], OPTION_FLOAT],
         //             [["-accessories"], OPTION_LIST, null, ["foo", "bar"]],
         //             [["-other"], OPTION_ANY]];
+        // argCount can be:
+        //            "0": no arguments
+        //            "1": exactly one argument
+        //            "+": one or more aguments
+        //            "*": zero or more arguments
+        //            "?": zero or one arguments
         // TODO: should it handle comments?
-    // TODO: should it return an error, if it contains arguments which look like options (beginning with -)?
+        parseArgs: function(str, options, argCount)
-    function parseArgs(str, options)
         {
             // returns [count, parsed_argument]
             function getNextArg(str)
@@ -244,12 +382,17 @@ liberator.Commands = function () //{{{
                     return [str.length, arg];
             }
 
-        var args = []; // parsed arguments
-        var opts = []; // parsed options
             if (!options)
                 options = [];
 
+            if (!argCount)
+                argCount = "*";
+
+            var args = {};       // parsed options
+            args.arguments = []; // remaining arguments
+
             var invalid = false;
+            var onlyArgumentsRemaining = false; // after a -- has been found
             var arg = null;
             var count = 0; // the length of the argument
             var i = 0;
@@ -264,7 +407,17 @@ liberator.Commands = function () //{{{
                 }
 
                 var sub = str.substr(i);
+                // dump(i + ": " + sub + " - " + onlyArgumentsRemaining + "\n");
+                if ((!onlyArgumentsRemaining) && /^--(\s|$)/.test(sub))
+                {
+                    onlyArgumentsRemaining = true;
+                    i+=2;
+                    continue;
+                }
+
                 var optname = "";
+                if (!onlyArgumentsRemaining)
+                {
                     for (var opt = 0; opt < options.length; opt++)
                     {
                         for (var name = 0; name < options[opt][0].length; name++)
@@ -290,7 +443,9 @@ liberator.Commands = function () //{{{
 
                                     count++; // to compensate the "=" character
                                 }
-                        else if (options[opt][1] != commandManager.OPTION_NOARG && /\s/.test(sub[optname.length]))
+                                else if (/\s/.test(sub[optname.length]))
+                                {
+                                    if (options[opt][1] != this.OPTION_NOARG)
                                     {
                                         [count, arg] = getNextArg(sub.substr(optname.length + 1));
                                         if (count == -1)
@@ -305,6 +460,9 @@ liberator.Commands = function () //{{{
 
                                         count++; // to compensate the " " character
                                     }
+                                    else
+                                        count = 1; // the space
+                                }
                                 else
                                 {
                                     // this isn't really an option as it has trailing characters, parse it as an argument
@@ -315,14 +473,14 @@ liberator.Commands = function () //{{{
                                 {
                                     switch (options[opt][1]) // type
                                     {
-                            case commandManager.OPTION_NOARG:
+                                    case this.OPTION_NOARG:
                                         if (arg != null)
                                         {
                                             liberator.echoerr("No argument allowed for option: " + optname);
                                             return null;
                                         }
                                         break;
-                            case commandManager.OPTION_BOOL:
+                                    case this.OPTION_BOOL:
                                         if (arg == "true" || arg == "1" || arg == "on")
                                             arg = true;
                                         else if (arg == "false" || arg == "0" || arg == "off")
@@ -333,14 +491,14 @@ liberator.Commands = function () //{{{
                                             return null;
                                         }
                                         break;
-                            case commandManager.OPTION_STRING:
+                                    case this.OPTION_STRING:
                                         if (arg == null)
                                         {
                                             liberator.echoerr("Argument required for string option: " + optname);
                                             return null;
                                         }
                                         break;
-                            case commandManager.OPTION_INT:
+                                    case this.OPTION_INT:
                                         arg = parseInt(arg, 10);
                                         if (isNaN(arg))
                                         {
@@ -348,7 +506,7 @@ liberator.Commands = function () //{{{
                                             return null;
                                         }
                                         break;
-                            case commandManager.OPTION_FLOAT:
+                                    case this.OPTION_FLOAT:
                                         arg = parseFloat(arg);
                                         if (isNaN(arg))
                                         {
@@ -356,7 +514,7 @@ liberator.Commands = function () //{{{
                                             return null;
                                         }
                                         break;
-                            case commandManager.OPTION_LIST:
+                                    case this.OPTION_LIST:
                                         if (arg == null)
                                         {
                                             liberator.echoerr("Argument required for list option: " + optname);
@@ -376,7 +534,7 @@ liberator.Commands = function () //{{{
                                         }
                                     }
 
-                            opts.push([options[opt][0][0], arg]); // always use the first name of the option
+                                    args[options[opt][0][0]] = arg; // always use the first name of the option
                                     i += optname.length + count;
                                     continue outer;
                                 }
@@ -384,6 +542,7 @@ liberator.Commands = function () //{{{
                             }
                         }
                     }
+                }
 
                 // if not an option, treat this token as an argument
                 var [count, arg] = getNextArg(sub);
@@ -392,155 +551,30 @@ liberator.Commands = function () //{{{
                     liberator.echoerr("Error parsing arguments: " + arg);
                     return null;
                 }
+                else if (!onlyArgumentsRemaining && /^-/.test(arg))
+                {
+                    liberator.echoerr("Invalid option: " + arg);
+                    return null;
+                }
 
                 if (arg != null)
-                args.push(arg);
+                    args.arguments.push(arg);
 
                 i += count; // hopefully count is always >0, otherwise we get an endless loop
             }
 
-        return { opts: opts, args: args };
+            // check for correct number of arguments
-    }
+            if ((args.arguments.length == 0 && (argCount == "1" || argCount == "+")) ||
-
+                (args.arguments.length == 1 && (argCount == "0"))                    ||
-    function getOption(opts, option, def)
+                (args.arguments.length > 1  && (argCount == "0" || argCount == "1" || argCount == "?")))
             {
-        for (var i = 0; i < opts.length; i++)
+                liberator.echoerr("Invalid number of arguments: " + args.arguments.length);
-        {
-            if (opts[i][0] == option)
-                return opts[i][1];
-        }
-
-        // no match found, return default
-        return def;
-    }
-
-    function getUserCommands(name)
-    {
-        var matches = [];
-        for (var i = 0; i < exCommands.length; i++)
-        {
-            if (exCommands[i].isUserCommand)
-            {
-                if (name)
-                {
-                    if (exCommands[i].name.match("^" + name))
-                        matches.push(exCommands[i]);
-                }
-                else
-                {
-                    matches.push(exCommands[i]);
-                }
-            }
-        }
-        return matches;
-    }
-
-    function removeUserCommand(name)
-    {
-        for (var i = 0; i < exCommands.length; i++)
-        {
-            if (exCommands[i].isUserCommand && exCommands[i].name == name)
-            {
-                exCommands.splice(i, 1);
-                break;
-            }
-        }
-    }
-
-    /////////////////////////////////////////////////////////////////////////////}}}
-    ////////////////////// PUBLIC SECTION //////////////////////////////////////////
-    /////////////////////////////////////////////////////////////////////////////{{{
-
-    var commandManager = {
-
-        // FIXME: remove later, when our option handler is better
-        //        Idea: If v.commands.add() specifies args or opts in extraInfo, don't call the function
-        //        with args as a string, but already pass an object like:
-        //        args = { -option: value, -anotheroption: true, arguments: [] }
-        getOption: function (opts, option, def) { return getOption(opts, option, def); },
-        parseArgs: function (str, options) { return parseArgs(str, options); },
-
-        OPTION_ANY: 0, // can be given no argument or an argument of any type,
-                       // caller is responsible for parsing the return value
-        OPTION_NOARG: 1,
-        OPTION_BOOL:2,
-        OPTION_STRING: 3,
-        OPTION_INT: 4,
-        OPTION_FLOAT: 5,
-        OPTION_LIST: 6,
-
-        __iterator__: function ()
-        {
-            var sorted = exCommands.sort(function (cmd1, cmd2) { return cmd1.name > cmd2.name; });
-            for (var i = 0; i < sorted.length; i++)
-                yield sorted[i];
-
-            throw StopIteration;
-        },
-
-        add: function (names, description, action, extra)
-        {
-            var command = new liberator.Command(names, description, action, extra);
-            if (!command)
-                return false;
-
-            for (var i = 0; i < exCommands.length; i++)
-            {
-                if (exCommands[i].name == command.name)
-                {
-                    // never replace for now
-                    liberator.log("Warning: :" + names[0] + " already exists, NOT replacing existing command.", 1);
-                    return false;
-                }
-            }
-
-            exCommands.push(command);
-            return true;
-        },
-
-        addUserCommand: function (names, description, action, extra, replace)
-        {
-            var extra = extra || {};
-            extra.isUserCommand = true;
-            description = description || "User defined command";
-
-            var command = new liberator.Command(names, description, action, extra);
-            if (!command)
-                return false;
-
-            for (var i = 0; i < exCommands.length; i++)
-            {
-                if (exCommands[i].name == command.name)
-                {
-                    if (!replace)
-                    {
-                        return false;
-                    }
-                    else
-                    {
-                        removeUserCommand(command.name);
-                        break;
-                    }
-                }
-            }
-
-            exCommands.push(command);
-            return true;
-        },
-
-        get: function (name)
-        {
-            for (var i = 0; i < exCommands.length; i++)
-            {
-                if (exCommands[i].hasName(name))
-                    return exCommands[i];
-            }
-
                 return null;
+            }
+
+            return args;
         },
 
-        // TODO: generalized 0 count handling -> "Zero count"
-        // FIXME: doesn't really belong here...
         // return [null, null, null, null, heredoc_tag || false];
         //        [count, cmd, special, args] = match;
         parseCommand: function (str, tag)
@@ -629,7 +663,7 @@ liberator.Commands = function () //{{{
             }
         },
         {
-            /*args: [[["-nargs"],    OPTION_STRING, function (arg) { return /^(0|1|\*|\?|\+)$/.test(arg); }],
+            /*options: [[["-nargs"],    OPTION_STRING, function (arg) { return /^(0|1|\*|\?|\+)$/.test(arg); }],
                    [["-bang"],     OPTION_NOARG],
                    [["-bar"],      OPTION_NOARG]] */
         });

content/liberator.js

@@ -733,7 +733,7 @@ const liberator = (function () //{{{
             // liberator.options does not exist at the very beginning
             if (liberator.options)
                 verbose = liberator.options["verbose"];
-dump("level: " + level + " - verbose: " + verbose + "\n");
+
             if (level > verbose)
                 return;
 

content/mail.js

@@ -651,7 +651,7 @@ liberator.Mail = function () //{{{
             try
             {
                 if (liberator.mail.currentAccount.server.type == "rss")
-                    OpenBrowserWithMessageId(gDBView.hdrForFirstSelectedMessage.messageId)
+                    messenger.launchExternalURL(getRSSUrl());
                 // TODO: what to do for non-rss message?
             }
             catch (e) { liberator.beep(); }
@@ -684,20 +684,15 @@ liberator.Mail = function () //{{{
         "Write a new message",
         function (args, special, count)
         {
-            var res = liberator.commands.parseArgs(args, this.args);
-            if (!res)
-                return;
-
             var mailargs = new Object();
-            mailargs.subject = liberator.commands.getOption(res.opts, "-subject", undefined);
+            mailargs.to =          args.arguments.join(", ");
-            mailargs.bcc = liberator.commands.getOption(res.opts, "-bcc", undefined);
+            mailargs.subject =     args["-subject"];
-            mailargs.cc = liberator.commands.getOption(res.opts, "-cc", undefined);
+            mailargs.bcc =         args["-bcc"];
-            mailargs.body = liberator.commands.getOption(res.opts, "-text", undefined);
+            mailargs.cc =          args["-cc"];
-            mailargs.attachments = liberator.commands.getOption(res.opts, "-attachment", []);
+            mailargs.body =        args["-text"];
+            mailargs.attachments = args["-attachment"] || [];
 
-            var addresses = [];
+            var addresses = args.arguments;
-            if (res.args)
-                addresses = addresses.concat(res.args);
             if (mailargs.bcc)
                 addresses = addresses.concat(mailargs.bcc);
             if (mailargs.cc)
@@ -710,15 +705,10 @@ liberator.Mail = function () //{{{
                 return;
             }
 
-            if (res.args.length > 0)
-            {
-                mailargs.to = res.args.join(", ");
-            }
-
             liberator.mail.composeNewMail(mailargs);
         },
         {
-            args: [[["-subject", "-s"],    liberator.commands.OPTION_STRING],
+            options: [[["-subject", "-s"],    liberator.commands.OPTION_STRING],
                       [["-attachment", "-a"], liberator.commands.OPTION_LIST],
                       [["-bcc", "-b"],        liberator.commands.OPTION_STRING],
                       [["-cc", "-c"],         liberator.commands.OPTION_STRING],