:bmarks [filter] works again, now even with -tags=foo filtering

content/bookmarks.js

@@ -99,17 +99,15 @@ function Bookmarks() //{{{
     ////////////////////// PUBLIC SECTION //////////////////////////////////////////
     /////////////////////////////////////////////////////////////////////////////{{{
 
-    // FIXME: add filtering here rather than having to calling get_bookmark_completions()
-    //
     // if "bypass_cache" is true, it will force a reload of the bookmarks database
     // on my PC, it takes about 1ms for each bookmark to load, so loading 1000 bookmarks
     // takes about 1 sec
-    this.get = function(filter, bypass_cache)
+    this.get = function(filter, tags, bypass_cache)
     {
         if (!bookmarks || bypass_cache)
             load();
 
-        return bookmarks;
+        return vimperator.completion.filterURLArray(bookmarks, filter, tags);
     }
 
     this.add = function (title, url, keyword, tags)
@@ -178,6 +176,7 @@ function Bookmarks() //{{{
         return count.value;
     }
 
+    // TODO: add filtering
     // also ensures that each search engine has a Vimperator-friendly alias
     this.getSearchEngines = function()
     {
@@ -210,6 +209,7 @@ function Bookmarks() //{{{
         return search_engines;
     }
 
+    // TODO: add filtering
     // format of returned array:
     // [keyword, helptext, url]
     this.getKeywords = function()
@@ -271,7 +271,7 @@ function Bookmarks() //{{{
             return url; // can be null
     }
 
-    this.list = function(filter, fullmode)
+    this.list = function(filter, tags, fullmode)
     {
         if (fullmode)
         {
@@ -279,8 +279,7 @@ function Bookmarks() //{{{
         }
         else
         {
-            //var items = vimperator.completion.get_bookmark_completions(filter);
+            var items = this.get(filter, tags, false);
-            var items = this.get(filter, false);
 
             if (items.length == 0)
             {
@@ -375,14 +374,12 @@ function History() //{{{
     ////////////////////// PUBLIC SECTION //////////////////////////////////////////
     /////////////////////////////////////////////////////////////////////////////{{{
 
-    // FIXME: add filtering here rather than having to call
+    this.get = function(filter)
-    // get_bookmark_completions()
-    this.get = function()
     {
         if (!history)
             load();
 
-        return history;
+            return vimperator.completion.filterURLArray(history, filter);
     }
 
     // the history is automatically added to the Places global history
@@ -451,7 +448,7 @@ function History() //{{{
         }
         else
         {
-            var items = vimperator.completion.get_history_completions(filter);
+            var items = this.get(filter);
 
             if (items.length == 0)
             {
@@ -463,22 +460,18 @@ function History() //{{{
                 return;
             }
 
+            var list = ":" + vimperator.util.escapeHTML(vimperator.commandline.getCommand()) + "<br/>" +
+                       "<table><tr align=\"left\" class=\"hl-Title\"><th>title</th><th>URL</th></tr>";
             for (var i = 0; i < items.length; i++)
             {
-                var list = ":" + vimperator.util.escapeHTML(vimperator.commandline.getCommand()) + "<br/>" +
+                var title = vimperator.util.escapeHTML(items[i][1]);
-                           "<table><tr align=\"left\" class=\"hl-Title\"><th>title</th><th>URL</th></tr>";
+                if (title.length > 50)
-                for (var i = 0; i < items.length; i++)
+                    title = title.substr(0, 47) + "...";
-                {
+                var url = vimperator.util.escapeHTML(items[i][0]);
-                    var title = vimperator.util.escapeHTML(items[i][1]);
+                list += "<tr><td>" + title + "</td><td><a href=\"#\" class=\"hl-URL\">" + url + "</a></td></tr>";
-                    if (title.length > 50)
-                        title = title.substr(0, 47) + "...";
-                    var url = vimperator.util.escapeHTML(items[i][0]);
-                    list += "<tr><td>" + title + "</td><td><a href=\"#\" class=\"hl-URL\">" + url + "</a></td></tr>";
-                }
-                list += "</table>";
-
-                vimperator.commandline.echo(list, vimperator.commandline.HL_NORMAL, vimperator.commandline.FORCE_MULTILINE);
             }
+            list += "</table>";
+            vimperator.commandline.echo(list, vimperator.commandline.HL_NORMAL, vimperator.commandline.FORCE_MULTILINE);
         }
     }
     //}}}

content/commands.js

@@ -516,7 +516,7 @@ function Commands() //{{{
                     var entry = sh.getEntryAtIndex(i, false);
                     var url = entry.URI.spec;
                     var title = entry.title;
-                    if (vimperator.completion.match(filter, [url, title], false))
+                    if (vimperator.completion.match([url, title], filter, false))
                         completions.push([url, title]);
                 }
                 return completions;
@@ -575,15 +575,27 @@ function Commands() //{{{
         }
     ));
     addDefaultCommand(new Command(["bmarks"],
-        function(args, special) { vimperator.bookmarks.list(args, special); },
+        function(args, special)
+        {
+            var res = parseArgs(args, this.args);
+            if (res.error)
+            {
+                vimperator.echoerr(res.error);
+                return false;
+            }
+            
+            var tags = getOption(res.opts, "-tags", []);
+            vimperator.bookmarks.list(res.args.join(" "), tags, special);
+        },
         {
             usage: ["bmarks [filter]", "bmarks!"],
             short_help: "Show bookmarks",
             help: "Open the message window at the bottom of the screen with all bookmarks which match <code class=\"argument\">[filter]</code> either in the title or URL.<br/>" +
                   "The special version <code class=\"command\">:bmarks!</code> will open the default Firefox bookmarks window.<br/>" +
-                  "The following options WILL be interpreted in the future:<br/>" +
+                  "Filter can also contain the following options:<br/>" +
-                  " -T comma,separated,tag,list<br/>",
+                  "-tags=comma,separated,tag,list<br/>",
-            completer: function(filter) { return vimperator.completion.get_bookmark_completions(filter); }
+            completer: function(filter) { return vimperator.bookmarks.get(filter); },
+            args: [[["-tags", "-T"],     OPTION_LIST]]
         }
     ));
     addDefaultCommand(new Command(["b[uffer]"],
@@ -637,7 +649,7 @@ function Commands() //{{{
                   "The following options WILL be interpreted in the future:<br/>" +
                   " [!] a special version to delete ALL bookmarks <br/>" +
                   " -T comma,separated,tag,list <br/>",
-            completer: function(filter) { return vimperator.completion.get_bookmark_completions(filter); }
+            completer: function(filter) { return vimperator.bookmarks.get(filter); }
         }
     ));
     addDefaultCommand(new Command(["com[mand]"],
@@ -858,7 +870,7 @@ function Commands() //{{{
                     var entry = sh.getEntryAtIndex(i, false);
                     var url = entry.URI.spec;
                     var title = entry.title;
-                    if (vimperator.completion.match(filter, [url, title], false))
+                    if (vimperator.completion.match([url, title], filter, false))
                         completions.push([url, title]);
                 }
                 return completions;
@@ -895,7 +907,7 @@ function Commands() //{{{
             short_help: "Show recently visited URLs",
             help: "Open the message window at the bottom of the screen with all history items which match <code class=\"argument\">[filter]</code> either in the title or URL.<br/>" +
                   "The special version <code class=\"command\">:history!</code> will open the default Firefox history window.",
-            completer: function(filter) { return vimperator.completion.get_history_completions(filter); }
+            completer: function(filter) { return vimperator.history.get(filter); }
         }
     ));
     addDefaultCommand(new Command(["javas[cript]", "js"],
@@ -1853,7 +1865,7 @@ function Commands() //{{{
                     // undoItems[i].image is also available if need for favicons
                     var url = undoItems[i].state.entries[0].url;
                     var title = undoItems[i].title;
-                    if (vimperator.completion.match(filter, [url, title], false))
+                    if (vimperator.completion.match([url, title], filter, false))
                         completions.push([url, title]);
                 }
                 return completions;

content/completion.js

@@ -104,65 +104,6 @@ vimperator.completion = (function() // {{{
         return filtered;
     } //}}}
 
-    /* discard all entries in the 'urls' array, which don't match 'filter */
-    function filter_url_array(urls, filter) //{{{
-    {
-        var filtered = [];
-        // completions which don't match the url but just the description
-        // list them add the end of the array
-        var additional_completions = [];
-
-        if (!filter) return urls.map(function($_) {
-            return [$_[0], $_[1]]
-        });
-
-        var ignorecase = false;
-        if (filter == filter.toLowerCase())
-            ignorecase = true;
-
-        /*
-         * Longest Common Subsequence
-         * This shouldn't use build_longest_common_substring
-         * for performance reasons, so as not to cycle through the urls twice
-         */
-        for (var i = 0; i < urls.length; i++)
-        {
-            var url = urls[i][0] || "";
-            var title = urls[i][1] || "";
-            if (ignorecase)
-            {
-                url = url.toLowerCase();
-                title = title.toLowerCase();
-            }
-
-            if (url.indexOf(filter) == -1)
-            {
-                if (title.indexOf(filter) != -1)
-                    additional_completions.push([ urls[i][0], urls[i][1] ]);
-                continue;
-            }
-            if (g_substrings.length == 0)   // Build the substrings
-            {
-                var last_index = url.lastIndexOf(filter);
-                var url_length = url.length;
-                for (var k = url.indexOf(filter); k != -1 && k <= last_index; k = url.indexOf(filter, k + 1))
-                {
-                    for (var l = k + filter.length; l <= url_length; l++)
-                        g_substrings.push(url.substring(k, l));
-                }
-            }
-            else
-            {
-                g_substrings = g_substrings.filter(function($_) {
-                    return url.indexOf($_) >= 0;
-                });
-            }
-            filtered.push([urls[i][0], urls[i][1]]);
-        }
-
-        return filtered.concat(additional_completions);
-    } //}}}
-
     return {
         /*
          * returns the longest common substring
@@ -201,9 +142,9 @@ vimperator.completion = (function() // {{{
                 if (cpt[i] == 's')
                     completions = completions.concat(this.get_search_completions(filter));
                 else if (cpt[i] == 'b')
-                    completions = completions.concat(this.get_bookmark_completions(filter));
+                    completions = completions.concat(vimperator.bookmarks.get(filter));
                 else if (cpt[i] == 'h')
-                    completions = completions.concat(this.get_history_completions(filter));
+                    completions = completions.concat(vimperator.history.get(filter));
                 else if (cpt[i] == 'f')
                     completions = completions.concat(this.get_file_completions(filter, true));
             }
@@ -224,18 +165,6 @@ vimperator.completion = (function() // {{{
             return build_longest_common_substring(mapped, filter);
         }, //}}}
 
-        get_history_completions: function(filter) //{{{
-        {
-            var items = vimperator.history.get();
-            return filter_url_array(items, filter);
-        }, //}}}
-
-        get_bookmark_completions: function(filter) //{{{
-        {
-            var bookmarks = vimperator.bookmarks.get();
-            return filter_url_array(bookmarks, filter);
-        }, //}}}
-
         // TODO: support file:// and \ or / path separators on both platforms
         get_file_completions: function(filter) //{{{
         {
@@ -540,10 +469,104 @@ vimperator.completion = (function() // {{{
             return build_longest_starting_substring(completions, filter);
         }, // }}}
 
-        // helper function which checks if the given arguments pass "filter"
+        // discard all entries in the 'urls' array, which don't match 'filter
+        // urls must be of type [["url", "title"], [...]] or optionally
+        //                      [["url", "title", keyword, [tags]], [...]]
+        filterURLArray: function(urls, filter, tags) //{{{
+        {
+            var filtered = [];
+            // completions which don't match the url but just the description
+            // list them add the end of the array
+            var additional_completions = [];
+
+            if (urls.length == 0)
+                return [];
+
+            var hasTags = urls[0].length >= 4;
+            // TODO: create a copy of urls?
+            if (!filter && (!hasTags || !tags))
+                return urls;
+
+            tags = tags || [];
+
+            // TODO: use ignorecase and smartcase settings
+            var ignorecase = true;
+            if (filter != filter.toLowerCase() || tags.join(",") != tags.join(",").toLowerCase())
+                ignorecase = false;
+
+            if (ignorecase)
+            {
+                filter = filter.toLowerCase();
+                tags = tags.map(function(t) { return t.toLowerCase(); });
+            }
+
+            /*
+             * Longest Common Subsequence
+             * This shouldn't use build_longest_common_substring
+             * for performance reasons, so as not to cycle through the urls twice
+             */
+            outer:
+            for (var i = 0; i < urls.length; i++)
+            {
+                var url   = urls[i][0] || "";
+                var title = urls[i][1] || "";
+                var tag   = urls[i][3] || [];
+
+                if (ignorecase)
+                {
+                    url = url.toLowerCase();
+                    title = title.toLowerCase();
+                    tag = tag.map(function(t) { return t.toLowerCase(); });
+                }
+
+                // filter on tags
+                for (var j = 0; j < tags.length; j++)
+                {
+                    if (!tags[j])
+                        continue;
+
+                    if (tag.indexOf(tags[j]) == -1)
+                        continue outer;
+                }
+
+                if (url.indexOf(filter) == -1)
+                {
+                    if (title.indexOf(filter) >= 0)
+                        additional_completions.push(urls[i]);
+
+                    continue;
+                }
+
+                // TODO: refactor out? And just build if wildmode contains longest?
+                if (g_substrings.length == 0)   // Build the substrings
+                {
+                    var last_index = url.lastIndexOf(filter);
+                    var url_length = url.length;
+                    if (last_index >= 0 && last_index < url_length) // do not build substrings, if we don't match filter
+                    {
+                        for (var k = url.indexOf(filter); k != -1 && k <= last_index; k = url.indexOf(filter, k + 1))
+                        {
+                            for (var l = k + filter.length; l <= url_length; l++)
+                                g_substrings.push(url.substring(k, l));
+                        }
+                    }
+                }
+                else
+                {
+                    g_substrings = g_substrings.filter(function($_) {
+                        return url.indexOf($_) >= 0;
+                    });
+                }
+
+                filtered.push(urls[i]);
+            }
+
+            return filtered.concat(additional_completions);
+        }, //}}}
+
+        // generic helper function which checks if the given "items" array pass "filter"
         // items must be an array of strings
-        // if case_sensitive == true, be sure to pass filter already in lowercased version
+        match: function(items, filter, case_sensitive)
-        match: function(filter, items, case_sensitive)
         {
             if (typeof(filter) != "string" || !items)
                 return false;
@@ -552,15 +575,16 @@ vimperator.completion = (function() // {{{
             {
                 for (var i = 0; i < items.length; i++)
                 {
-                    if (items[i].toLowerCase().indexOf(filter) > -1)
+                    if (items[i].indexOf(filter) > -1)
                         return true;
                 }
             }
             else
             {
+                filter = filter.toLowerCase();
                 for (var i = 0; i < items.length; i++)
                 {
-                    if (items[i].indexOf(filter) > -1)
+                    if (items[i].toLowerCase().indexOf(filter) > -1)
                         return true;
                 }
             }

content/util.js

@@ -29,9 +29,13 @@ the terms of any one of the MPL, the GPL or the LGPL.
 vimperator.util = {
     escapeHTML: function(str)
     {   
-        var e = window.content.document.createElement("div");
+        // XXX: the following code is _much- slower then a simple .replace()
-        e.appendChild(window.content.document.createTextNode(str));
+        // :history display went down from 2 to 1 second after changing
-        return e.innerHTML;
+        //
+        // var e = window.content.document.createElement("div");
+        // e.appendChild(window.content.document.createTextNode(str));
+        // return e.innerHTML;
+        return str.replace(/</, "&lt;").replace(/>/, "&gt;");
     },
 
     // TODO: use :highlight color groups

skin/vimperator.css

@@ -151,7 +151,7 @@ the terms of any one of the MPL, the GPL or the LGPL.
     text-decoration: none;
 }
 a.hl-URL:hover {
-    background-color: gray;
+    text-decoration: underline;
 }
 
 /* MOW */