:bmark -F Folders support.

[Alexander Skurihin]

# HG changeset patch
# User Skurikhin Aleksandr <a.sku...@gmail.com>
# Date 1326181742 -14400
# Node ID f0159c1aca6c5d97c2c6edb9a762b479028aeba3
# Parent 14cf65d4aac2181c5e6722e682a91cfb47accf90
:bmark -F Folder support while adding bookmarks

diff -r 14cf65d4aac2 -r f0159c1aca6c common/content/bookmarks.js
--- a/common/content/bookmarks.js Sun Jan 08 15:25:13 2012 -0500
+++ b/common/content/bookmarks.js Tue Jan 10 11:49:02 2012 +0400
@@ -6,6 +6,25 @@
// given in the LICENSE.txt file included with this file.
/* use strict */

+function getFolderIdByPath(path) {
+
+ /**
+ * Gets id of a folder by given full path in format: path/to/
folder <- root
+ * If folder doesn't exist, creates it.
+ *
+ * @param {string} path Path to a folder in format: path/to/
folder
+ * @returns {int} Id of a founded or created folder
+ */
+
+ for (var b in bookmarkcache)
+ if (b.folder.path === path)
+ return b.folder.id;
+
+ var folderTitle = path.split('/')[0];
+ var parents = path.split('/').slice(1);
+ return
services.bookmarks.createFolder(getFolderIdByPath(parents.join('/')),
folderTitle, -1);
+}
+
// also includes methods for dealing with keywords and search engines
var Bookmarks = Module("bookmarks", {
init: function () {
@@ -64,10 +83,10 @@
* @returns {boolean} True if the bookmark was updated, false if
a
* new bookmark was added.
*/
- add: function add(unfiled, title, url, keyword, tags, force) {
+ add: function add(unfiled, title, url, keyword, tags, force,
folder) {
// FIXME
if (isObject(unfiled))
- var { id, unfiled, title, url, keyword, tags, post,
charset, force } = unfiled;
+ var { id, unfiled, title, url, keyword, tags, post,
charset, force, folder} = unfiled;

let uri = util.createURI(url);
if (id != null)
@@ -85,7 +104,6 @@
PlacesUtils.tagging.untagURI(uri, null);
PlacesUtils.tagging.tagURI(uri, tags);
}
-
let updated = !!bmark;
if (bmark == undefined)
bmark = bookmarkcache.bookmarks[
@@ -107,7 +125,8 @@
bmark.post = post;
if (keyword)
bmark.keyword = keyword;
-
+ if (folder)
+ bmark.folder = folder;
return updated;
},

@@ -404,6 +423,18 @@
}, {
commands: function () {
// TODO: Clean this up.
+
+ const folder = {
+ names: ["-folder", "-F"],
+ description: "A folder where this bookmark will be
added",
+ completer: function folder(context, args) {
+ context.generate = function () array(b.folder.path
for (b in bookmarkcache)).uniq().array;
+ context.keys.text = util.identity;
+ context.keys.description = function () ""
+ },
+ type: CommandOption.STRING
+ };
+
const tags = {
names: ["-tags", "-T"],
description: "A comma-separated list of tags",
@@ -457,8 +488,8 @@
function (args) {
dactyl.assert(!args.bang || args["-id"] == null,
_("bookmark.bangOrID"));
-
let opts = {
+ folder: getFolderIdByPath(args["-folder"]) ||
null,
force: args.bang,
unfiled: false,
id: args["-id"],
@@ -469,7 +500,6 @@
title: args["-title"] || (args.length === 0 ?
buffer.title : null),
url: args.length === 0 ? buffer.uri.spec :
args[0]
};
-
let updated = bookmarks.add(opts);
let action = updated ? "updated" : "added";

@@ -491,7 +521,7 @@
}
completion.bookmark(context, args["-tags"],
{ keyword: args["-keyword"], title: args["-title"] });
},
- options: [keyword, title, tags, post,
+ options: [keyword, title, tags, post, folder,
{
names: ["-charset", "-c"],
description: "The character encoding of the
bookmark",
diff -r 14cf65d4aac2 -r f0159c1aca6c common/modules/bookmarkcache.jsm
--- a/common/modules/bookmarkcache.jsm Sun Jan 08 15:25:13 2012 -0500
+++ b/common/modules/bookmarkcache.jsm Tue Jan 10 11:49:02 2012 +0400
@@ -21,8 +21,29 @@
}
}

-var Bookmark = Struct("url", "title", "icon", "post", "keyword",
"tags", "charset", "id");
+function getParentsForItem(itemId) {
+
+ /**
+ * Looks for parent folders for a given item's id
+ *
+ * @param {int} itemId Item's id which parents we are looking for
+ * @returns {array} List of parent's titles
+ */
+
+ var parents = [];
+ parents.toString = function () this.join("/");
+
+ let id = itemId, parent, title;
+ while ((id = services.bookmarks.getFolderIdForItem(id)) &&
+ (title = services.bookmarks.getItemTitle(id)))
+ parents.push(title);
+
+ return parents;
+}
+
+var Bookmark = Struct("url", "title", "icon", "post", "keyword",
"tags", "charset", "id", "folder");
var Keyword = Struct("keyword", "title", "icon", "url");
+var Folder = Struct("id", "path");
Bookmark.defaultValue("icon", function ()
BookmarkCache.getFavicon(this.url));
update(Bookmark.prototype, {
get extra() [
@@ -46,15 +67,10 @@
},

get folder() {
- let res = [];
- res.toString = function () this.join("/");
+ let folderPath = getParentsForItem(this.id).toString();
+ let folderId =
services.bookmarks.getFolderIdForItem(this.id);

- let id = this.id, parent, title;
- while ((id = services.bookmarks.getFolderIdForItem(id)) &&
- (title = services.bookmarks.getItemTitle(id)))
- res.push(title);
-
- return res.reverse();
+ return Folder(folderId, folderPath);
}
})
Bookmark.prototype.members.uri = Bookmark.prototype.members.url;
@@ -69,6 +85,7 @@
if (val)
services.tagging.tagURI(this.uri, val);
});
+Bookmark.setter("folder", function (val)
{ services.bookmarks.moveItem(this.id, val, -1) });

var name = "bookmark-cache";

@@ -111,7 +128,8 @@

let post = BookmarkCache.getAnnotation(node.itemId,
this.POST);
let charset = BookmarkCache.getAnnotation(node.itemId,
this.CHARSET);
- return Bookmark(node.uri, node.title, node.icon &&
node.icon.spec, post, keyword, tags, charset, node.itemId);
+ let folder = node.folder;
+ return Bookmark(node.uri, node.title, node.icon &&
node.icon.spec, post, keyword, tags, charset, node.itemId, folder);
},

annotate: function (id, key, val, timespan) {

[Kris Maglione]

On Mon, Jan 09, 2012 at 11:51:52PM -0800, Alexander Skurihin wrote:
># HG changeset patch
># User Skurikhin Aleksandr <a.sku...@gmail.com>
># Date 1326181742 -14400
># Node ID f0159c1aca6c5d97c2c6edb9a762b479028aeba3
># Parent 14cf65d4aac2181c5e6722e682a91cfb47accf90
>:bmark -F Folder support while adding bookmarks
>
>diff -r 14cf65d4aac2 -r f0159c1aca6c common/content/bookmarks.js
>--- a/common/content/bookmarks.js Sun Jan 08 15:25:13 2012 -0500
>+++ b/common/content/bookmarks.js Tue Jan 10 11:49:02 2012 +0400
>@@ -6,6 +6,25 @@
> // given in the LICENSE.txt file included with this file.
> /* use strict */
>
>+function getFolderIdByPath(path) {

This needs to be a public method.

>+ for (var b in bookmarkcache)
>+ if (b.folder.path === path)
>+ return b.folder.id;

This won't work. You either need to maintain a separate cache
for paths or walk the bookmark tree directly. Aside from being
inefficient, this will miss empty folders.

>@@ -64,10 +83,10 @@
> * @returns {boolean} True if the bookmark was updated, false if
>a
> * new bookmark was added.
> */
>- add: function add(unfiled, title, url, keyword, tags, force) {
>+ add: function add(unfiled, title, url, keyword, tags, force,
>folder) {

Calling this function with multiple parameters is deprecated.
New parameters should only be added to the object.

>@@ -404,6 +423,18 @@
> }, {
> commands: function () {
> // TODO: Clean this up.
>+
>+ const folder = {
>+ names: ["-folder", "-F"],

Short options should be lower case unless there's a conflicting
short option with the same letter.

>+ completer: function folder(context, args) {
>+ context.generate = function () array(b.folder.path
>for (b in bookmarkcache)).uniq().array;

Again misses empty folders, and this should be a function in the
completion module.

You haven't included any documentation changes or NEWS entries
either.

--
Kris Maglione

The first 90% of the code accounts for the first 90% of the
development time. The remaining 10% of the code accounts for the
other 90% of the development time.
--Tom Cargill

[Alexander Skurihin]

So, I've considered your suggestions, and here what I got.
It's quite big, so i'll split diffs by file, and I put a whole diff
file in the end:

1. bookmarks.js http://dpaste.com/693361/
2. bookmarkcache.jsm http://dpaste.com/693365/
3. completion.jsm http://dpaste.com/693367/
4. foldercache.jsm http://dpaste.com/693369/
5. template.jsm http://dpaste.com/693370/
4. options.xml http://dpaste.com/693364/
5. marks.xml http://dpaste.com/693362/
6. the rest http://dpaste.com/693371/
7. all together http://hpaste.org/56985

This path:
1. adds folder support in bookmark related commands, such
as :bmark :bmarks :delbmarks
1.1 provides an easy way to customize it's output, using css and
foldericon option
2. fixes bug when :bmark didn't filtrate by title, because item.title
wasn't defined
3. fixes bug when :bmark -title showed wrong titles, now it shows
titles of all opend tabs in current window as it should

Hope you find my work useful. Again if you have any suggestions I'm
opened to listen.

During writing this code I met a lot of different problems, most of
them I've managed to solve, but few, the most annoyimg are still here.
It would cool If you or somebody else can answer my questions:

1. when I add a new module it's not loaded.
I temporary solved this problem by adding my module into pentadactyl/
config.json in scripts sections. But i believe there is a better way.

2. when i change a file and run make, it is not added to newly created
xpi. I had to manually add cp lines to make_jar.sh.
I think I lost something.

3. I was tring to add options to my foldercache module, but it didn't
work, so I temporary move them to bookmarks.js.
Options appears in foldercache.INIT, but seems that it was never
called.
I tried to check other modules, but didn't find any points how can I
help this.

4. I didn't quite understand why you use storage.fireEvent(name,
"add", bmark) in bookmarkcache.

By the way, it was quite fun to hack dactyl. But it realy, realy needs
more documentation for developers.
Hope I'll find some time to fix it.

On Jan 16, 10:24 am, Kris Maglione <maglion...@gmail.com> wrote:
> On Mon, Jan 09, 2012 at 11:51:52PM -0800, Alexander Skurihin wrote:
> ># HG changeset patch

> ># User Skurikhin Aleksandr <a.skuri...@gmail.com>