Slightly refactor the marks code. Make :delmarks, :marks and :qmarks accept ranges.

common/content/marks.js

@@ -39,7 +39,7 @@ var Marks = Module("marks", {
      * Add a named mark for the current buffer, at its current position.
      * If mark matches [A-Z], it's considered a URL mark, and will jump to
      * the same position at the same URL no matter what buffer it's
-     * selected from. If it matches [a-z'"], it's a local mark, and can
+     * selected from. If it matches [a-z], it's a local mark, and can
      * only be recalled from a buffer with a matching URL.
      *
      * @param {string} mark The mark name.
@@ -69,18 +69,20 @@ var Marks = Module("marks", {
      * Remove all marks matching *filter*. If *special* is given, removes all
      * local marks.
      *
-     * @param {string} filter A string containing one character for each
+     * @param {string} filter The list of marks to delete, e.g. "aA b C-I"
-     *     mark to be removed.
      * @param {boolean} special Whether to delete all local marks.
      */
     remove: function (filter, special) {
         if (special)
             this._localMarks.remove(this.localURI);
         else {
+            let pattern = util.charListToRegexp(filter, "a-zA-Z");
             let local = this._localMarks.get(this.localURI);
-            Array.forEach(filter, function (mark) {
+            this.all.forEach(function ([k, ]) {
-                delete local[mark];
+                if (pattern.test(k)) {
-                this.urlMarks.remove(mark);
+                    local && delete local[k];
+                    marks._urlMarks.remove(k);
+                }
             });
             try {
                 Iterator(local).next();
@@ -98,8 +100,6 @@ var Marks = Module("marks", {
      * @param {string} mark The mark to jump to.
      */
     jumpTo: function (mark) {
-        let ok = false;
-
         if (Marks.isURLMark(mark)) {
             let slice = this._urlMarks.get(mark);
             let tab = slice && slice.tab && slice.tab.get();
@@ -122,26 +122,29 @@ var Marks = Module("marks", {
                     }
                     dactyl.log("Jumping to URL mark: " + Marks.markToString(mark, slice), 5);
                     buffer.scrollToPercent(slice.position.x * 100, slice.position.y * 100);
-                    ok = true;
+                    return;
                 }
             }
+            // FIXME This is stupid, but perhaps better than the current
+            // behaviour (persisting URL marks that will just signal an error).
+            else
+                this._pendingJumps.push(slice);
+                dactyl.open(slice.location, dactyl.NEW_TAB);
+                return;
         }
-        else if (Marks.isLocalMark(mark)) {
+        let mobj = Marks.isLocalMark(mark) && (this._localMarks.get(this.localURI) || {})[mark];
-            ok = (this._localMarks.get(this.localURI) || {})[mark];
+        if (mobj) {
-            if (ok) {
+            dactyl.log("Jumping to local mark: " + Marks.markToString(mark, mobj), 5);
-                dactyl.log("Jumping to local mark: " + Marks.markToString(mark, ok), 5);
+            buffer.scrollToPercent(mobj.position.x * 100, mobj.position.y * 100);
-                buffer.scrollToPercent(ok.position.x * 100, ok.position.y * 100);
+            return;
-            }
         }
-
+        dactyl.echoerr("E20: Mark not set");
-        if (!ok)
-            dactyl.echoerr("E20: Mark not set");
     },
 
     /**
      * List all marks matching *filter*.
      *
-     * @param {string} filter
+     * @param {string} filter List of marks to show, e.g. "ab A-I".
      */
     list: function (filter) {
         let marks = this.all;
@@ -149,13 +152,14 @@ var Marks = Module("marks", {
         dactyl.assert(marks.length > 0, "No marks set");
 
         if (filter.length > 0) {
-            marks = marks.filter(function (mark) filter.indexOf(mark[0]) >= 0);
+            let pattern = util.charListToRegexp(filter, "a-zA-Z");
+            marks = marks.filter(function ([k, ]) pattern.test(k));
             dactyl.assert(marks.length > 0, "E283: No marks matching " + filter.quote());
         }
 
         commandline.commandOutput(
             template.tabular(
-                ["Mark",   "Line",              "Column",            "File"],
+                ["Mark",   "HPos",              "VPos",              "File"],
                 ["",       "text-align: right", "text-align: right", "color: green"],
                 ([mark[0],
                   Math.round(mark[1].position.x * 100) + "%",
@@ -183,9 +187,9 @@ var Marks = Module("marks", {
                 (tab ? ", tab: " + tabs.index(tab) : "");
     },
 
-    isLocalMark: function isLocalMark(mark) /^['`a-z]$/.test(mark),
+    isLocalMark: function isLocalMark(mark) /^[a-z]$/.test(mark),
 
-    isURLMark: function isURLMark(mark) /^[A-Z0-9]$/.test(mark)
+    isURLMark: function isURLMark(mark) /^[A-Z]$/.test(mark)
 }, {
     events: function () {
         let appContent = document.getElementById("appcontent");
@@ -198,14 +202,15 @@ var Marks = Module("marks", {
         mappings.add(myModes,
             ["m"], "Set mark at the cursor position",
             function ({ arg }) {
-                dactyl.assert(/^[a-zA-Z]$/.test(arg));
+                dactyl.assert(/^[a-zA-Z]$/.test(arg),
+                    "E191: Argument must be an ASCII letter");
                 marks.add(arg);
             },
             { arg: true });
 
         mappings.add(myModes,
             ["'", "`"], "Jump to the mark in the current buffer",
-            function (args) { marks.jumpTo(args.arg); },
+            function ({ arg }) { marks.jumpTo(arg); },
             { arg: true });
     },
 
@@ -214,27 +219,13 @@ var Marks = Module("marks", {
             "Delete the specified marks",
             function (args) {
                 let special = args.bang;
-                args = args[0] || "";
+                let arg = args[0] || "";
 
                 // assert(special ^ args)
-                dactyl.assert( special ||  args, "E471: Argument required");
+                dactyl.assert( special ||  arg, "E471: Argument required");
-                dactyl.assert(!special || !args, "E474: Invalid argument");
+                dactyl.assert(!special || !arg, "E474: Invalid argument");
 
-                let matches = args.match(/(?:(?:^|[^a-zA-Z0-9])-|-(?:$|[^a-zA-Z0-9])|[^a-zA-Z0-9 -]).*/);
+                marks.remove(arg, special);
-                // NOTE: this currently differs from Vim's behavior which
-                // deletes any valid marks in the arg list, up to the first
-                // invalid arg, as well as giving the error message.
-                dactyl.assert(!matches, "E475: Invalid argument: " + (matches && matches[0]));
-
-                // check for illegal ranges - only allow a-z A-Z 0-9
-                if ((matches = args.match(/[a-zA-Z0-9]-[a-zA-Z0-9]/g))) {
-                    for (let match in values(matches))
-                        dactyl.assert(/[a-z]-[a-z]|[A-Z]-[A-Z]|[0-9]-[0-9]/.test(match) &&
-                                         match[0] <= match[2],
-                            "E475: Invalid argument: " + args.match(match + ".*")[0]);
-                }
-
-                marks.remove(args, special);
             },
             {
                 bang: true,
@@ -248,24 +239,18 @@ var Marks = Module("marks", {
                 let mark = args[0] || "";
                 dactyl.assert(mark.length <= 1, "E488: Trailing characters");
                 dactyl.assert(/[a-zA-Z]/.test(mark),
-                    "E191: Argument must be a letter or forward/backward quote");
+                    "E191: Argument must be an ASCII letter");
 
                 marks.add(mark);
             },
             { argCount: "1" });
 
         commands.add(["marks"],
-            "Show all location marks of current web page",
+            "Show the specified marks",
             function (args) {
-                args = args[0] || "";
+                marks.list(args[0] || "");
-
-                // ignore invalid mark characters unless there are no valid mark chars
-                dactyl.assert(!args || /[a-zA-Z]/.test(args),
-                    "E283: No marks matching " + args.quote());
-
-                let filter = args.replace(/[^a-zA-Z]/g, "");
-                marks.list(filter);
             }, {
+                completer: function (context) completion.mark(context),
                 literal: 0
             });
     },
@@ -275,8 +260,8 @@ var Marks = Module("marks", {
             function percent(i) Math.round(i * 100);
 
             // FIXME: Line/Column doesn't make sense with %
-            context.title = ["Mark", "Line Column File"];
+            context.title = ["Mark", "HPos VPos File"];
-            context.keys.description = function ([, m]) percent(m.position.y) + "% " + percent(m.position.x) + "% " + m.location;
+            context.keys.description = function ([, m]) percent(m.position.x) + "% " + percent(m.position.y) + "% " + m.location;
             context.completions = marks.all;
         };
     },

common/content/quickmarks.js

@@ -61,7 +61,7 @@ var QuickMarks = Module("quickmarks", {
      *
      */
     remove: function remove(filter) {
-        let pattern = RegExp("[" + filter.replace(/\s+/g, "") + "]");
+        let pattern = util.charListToRegexp(filter, "a-zA-Z0-9");
 
         for (let [qmark, ] in this._qmarks) {
             if (pattern.test(qmark))
@@ -95,10 +95,8 @@ var QuickMarks = Module("quickmarks", {
     /**
      * Lists all quickmarks matching *filter* in the message window.
      *
-     * @param {string} filter The list of quickmarks to display. Eg. "abc"
+     * @param {string} filter The list of quickmarks to display, e.g. "a-c i O-X".
-     * Ranges are not supported.
      */
-    // FIXME: filter should match that of quickmarks.remove or vice versa
     list: function list(filter) {
         let marks = [k for ([k, v] in this._qmarks)];
         let lowercaseMarks = marks.filter(function (x) /[a-z]/.test(x)).sort();
@@ -110,7 +108,8 @@ var QuickMarks = Module("quickmarks", {
         dactyl.assert(marks.length > 0, "No QuickMarks set");
 
         if (filter.length > 0) {
-            marks = marks.filter(function (qmark) filter.indexOf(qmark) >= 0);
+            let pattern = util.charListToRegexp(filter, "a-zA-Z0-9");
+            marks = marks.filter(function (qmark) pattern.test(qmark));
             dactyl.assert(marks.length >= 0, "E283: No QuickMarks matching " + filter.quote());
         }
 
@@ -142,9 +141,9 @@ var QuickMarks = Module("quickmarks", {
         commands.add(["qma[rk]"],
             "Mark a URL with a letter for quick access",
             function (args) {
-                if (!/^[a-zA-Z0-9]$/.test(args[0]))
+                dactyl.assert(/^[a-zA-Z0-9]$/.test(args[0]),
-                    dactyl.echoerr("E488: Trailing characters");
+                    "E191: Argument must be an ASCII letter or digit");
-                else if (!args[1])
+                if (!args[1])
                     quickmarks.add(args[0], buffer.URL.spec);
                 else
                     quickmarks.add(args[0], args[1]);
@@ -168,16 +167,11 @@ var QuickMarks = Module("quickmarks", {
             });
 
         commands.add(["qmarks"],
-            "Show all QuickMarks",
+            "Show the specified QuickMarks",
             function (args) {
-                args = args[0] || "";
+                quickmarks.list(args[0] || "");
-
-                // ignore invalid qmark characters unless there are no valid qmark chars
-                dactyl.assert(!args || /[a-zA-Z0-9]/.test(args), "E283: No QuickMarks matching " + args.quote());
-
-                let filter = args.replace(/[^a-zA-Z0-9]/g, "");
-                quickmarks.list(filter);
             }, {
+                completer: function (context) completion.quickmark(context),
                 literal: 0
             });
     },
@@ -192,18 +186,18 @@ var QuickMarks = Module("quickmarks", {
 
         mappings.add(myModes,
             ["go"], "Jump to a QuickMark",
-            function (args) { quickmarks.jumpTo(args.arg, dactyl.CURRENT_TAB); },
+            function ({ arg }) { quickmarks.jumpTo(arg, dactyl.CURRENT_TAB); },
             { arg: true });
 
         mappings.add(myModes,
             ["gn"], "Jump to a QuickMark in a new tab",
-            function (args) { quickmarks.jumpTo(args.arg, { from: "quickmark", where: dactyl.NEW_TAB }); },
+            function ({ arg }) { quickmarks.jumpTo(arg, { from: "quickmark", where: dactyl.NEW_TAB }); },
             { arg: true });
 
         mappings.add(myModes,
             ["M"], "Add new QuickMark for current URL",
             function ({ arg }) {
-                dactyl.assert(/^[a-zA-Z0-9]$/.test(arg));
+                dactyl.assert(/^[a-zA-Z0-9]$/.test(arg), "E191: Argument must be an ASCII letter or digit");
                 quickmarks.add(arg, buffer.URL.spec);
             },
             { arg: true });

common/modules/util.jsm

@@ -134,6 +134,31 @@ var Util = Module("Util", XPCOM([Ci.nsIObserver, Ci.nsISupportsWeakReference]),
             throw FailedAssertion(message, 1);
     },
 
+    /**
+     * Returns a RegExp object that matches characters specified in the range
+     * expression *list*, or signals an appropriate error if *list* is invalid.
+     *
+     * @param {string} list Character list, e.g., "a b d-xA-Z" produces /[abd-xA-Z]/.
+     * @param {string} accepted Character range(s) to accept, e.g. "a-zA-Z" for
+     *     ASCII letters. Used to validate *list*.
+     */
+    charListToRegexp: function charListToRegexp(list, accepted) {
+        let list = list.replace(/\s+/g, "");
+
+        // check for chars not in the accepted range
+        this.assert(RegExp("^[" + accepted + "-]+$").test(list),
+                "Character list outside the range " + accepted.quote());
+
+        // check for illegal ranges
+        let matches = list.match(RegExp("[" + accepted + "]-[" + accepted + "]", "g"));
+        if (matches) {
+            for (let match in values(matches))
+                this.assert(match[0] <= match[2],
+                    "Invalid character range: " + list.match(match + ".*")[0]);
+        }
+        return RegExp("[" + list + "]");
+    },
+
     get chromePackages() {
         // Horrible hack.
         let res = {};

pentadactyl/NEWS

@@ -78,6 +78,8 @@
         directory.
       - Added :write !cmd and :write >>file.
       - Added :yank command.
+      - :delmarks, :marks and :qmarks now also accept ranges, same as
+        :delqmarks.
     * Improvements to :style and :highlight:
       - Added -agent flag to :style.
       - The -append flag now updates existing properties rather than