[PATCH] Bookmark Tags
|
View:
New views
4 Messages
—
Rating Filter:
Alert me
|
 |
[PATCH] Bookmark Tags
by Michael Zeller
::
Rate this Message:
Hello all,
Let me preface this by saying that this is my first contribution to this amazing project, and in fact, to any open source project. Please let me know if I've made any mistakes in how I communicate my patch, or any feedback you may have. Hopefully this can get accepted =). Additionally, I hope to contribute more in the future to both StumpWM and Conkeror (and Org-mode!), they all rock! ~Michael Zeller * What I've added Support for bookmark tags, in the same way that Firefox supports tags (i.e. you should be able to copy your places.sqlite from Firefox and use the same tags). Essentially, you add the following to your ~/.conkerorrc after this patch is applied. minibuffer_read_tag_enable = true; define_tag("gnu","GNU Not UNIX"); define_tag("uci","University of California, Irvine"); define_tag("r","R language"); define_tag("emacs","Emacs"); define_tag("blog","Interesting Blog"); And you will be prompted for tags when invoking the interactive "bookmark" command. Any number of tags can be added, and the input method works the same as keywords in auto-insert.el in Emacs, i.e. you end input with an empty tag name. To search for tags, just type the tag name, or part of the tag name, in find-url, I didn't change anything there -- it worked out of the box. I've also fixed a typo in minibuffer-read.js. =================== * Notes I'm not sure where I should have put these notes, but here are a few remaining issues: - Not sure what happens if you add a tag twice while bookmarking. - If you add a bookmark twice, it appears twice in the results, this appears to be a general Conkeror bug. I think it should prompt for overwrite if the URI's match, so if this sounds good I can work on this, this would allow adding additional tags. - If you remove a "define_tag" line from your .conkerorrc, the tag will stick around in your places.sqlite, and you can still search for it, but you will not have the option to use it in the minibuffer prompt during "bookmark". - The XULRunner support for Tags is pretty lame, there are a lot of missing methods, for example, there is no way to save a Description for a tag with the tag. And there is no way to get a list of all tags, except to iterate over the "Tags" folder until you hit an Index Out of Bounds error. This is why tags are explicitly defined in the ~/.conkerorrc, with descriptions. - I *don't* think there is any issue with the names of tags, and you could use special characters, if you /really/ wanted to. - I would gladly edit the Wiki Bookmarks page for this, if it gets added. >From 7e69b8cd4d6340d934f2ad96cf16ce216fc3d4d7 Mon Sep 17 00:00:00 2001 From: Michael Zeller <me@...> Date: Tue, 4 Aug 2009 03:41:45 -0700 Subject: [PATCH] Added support for bookmark tags --- modules/commands.js | 12 ++++++++++-- modules/content-buffer.js | 31 +++++++++++++++++++++++++++++++ modules/history.js | 35 ++++++++++++++++++++++++++++++++--- modules/minibuffer-read.js | 2 +- 4 files changed, 74 insertions(+), 6 deletions(-) diff --git a/modules/commands.js b/modules/commands.js index b409dee..cb1e8c3 100644 --- a/modules/commands.js +++ b/modules/commands.js @@ -637,11 +637,19 @@ interactive("bookmark", null, function (I) { element_get_operation_label(element, "Bookmarking"), uri_string]]); try { - var title = yield I.minibuffer.read($prompt = "Bookmark with title:", $initial_value = load_spec_title(spec) || ""); + var title = yield I.minibuffer.read($prompt = "Bookmark with title:", + $initial_value = load_spec_title(spec) || ""); + var tags = []; + while(true) { + tag = yield I.minibuffer.read_tag($prompt = "Bookmark with tag:", + $initial_value = ""); + if (tag == "") break; else tags.push(tag); + } + } finally { panel.destroy(); } - add_bookmark(uri_string, title); + add_bookmark(uri_string, title, tags); I.minibuffer.message("Added bookmark: " + uri_string + " - " + title); }, $browser_object = browser_object_frames); diff --git a/modules/content-buffer.js b/modules/content-buffer.js index 3b8dd9f..c55657b 100644 --- a/modules/content-buffer.js +++ b/modules/content-buffer.js @@ -328,6 +328,37 @@ minibuffer.prototype.try_read_url_handlers = function (input) { return input; } +/** + * This most definitely should not go here + */ + +define_variable("minibuffer_read_tag_select_initial", true, + "Specifies whether a tag presented in the minibuffer for editing "+ + "should be selected. This affects bookmark."); +define_variable("minibuffer_read_tag_enable", false, + "Specifies whether tags should be prompted for during creation "+ + "of bookmarks. This affects bookmark."); + +minibuffer.prototype.read_tag = function () { + keywords(arguments, $prompt = "Tag:", $initial_value = "", $history = "tag"); + var result = minibuffer_read_tag_enable ? yield this.read( + $prompt = arguments.$prompt, + $history = arguments.$history, + $completer = tags_completer(), + $initial_value = arguments.$initial_value, + $auto_complete = "string", + $select = minibuffer_read_tag_select_initial, + $validator = function (x, m) { + if (x in user_tags || x == "") return true; + else { m.message("No match"); return false; } + }) : ""; + yield co_return(result); +}; + +/** + * End the stuff that should not be here + */ + define_variable("url_completion_use_webjumps", true, "Specifies whether URL completion should complete webjumps."); diff --git a/modules/history.js b/modules/history.js index 2ca52de..30793ec 100644 --- a/modules/history.js +++ b/modules/history.js @@ -58,9 +58,38 @@ function url_completer() { return merge_completers(completers); } +/** + * Tags + */ + const nav_bookmarks_service = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].getService(Ci.nsINavBookmarksService); -function add_bookmark(url, title) { - nav_bookmarks_service.insertBookmark(nav_bookmarks_service.unfiledBookmarksFolder, - make_uri(url), -1, title); +var user_tags = {}; + +function define_tag (tag, description) { + var id = nav_bookmarks_service.getChildFolder(nav_bookmarks_service.tagsFolder, tag); + if (!id) id = nav_bookmarks_service.createFolder(nav_bookmarks_service.tagsFolder, tag, -1); + user_tags[tag] = { + default_value: tag, + description: description, + id: id + }; +} + +function tags_completer() { + return prefix_completer($completions = function (visitor) { + [visitor(i, user_tags[i]) for (i in user_tags)]; + }, + $get_string = function (x) x, + $get_description = function (x) user_tags[x].description || "", + $get_value = function (x) x); +} + +function add_bookmark(url, title, tags) { + var uri = make_uri(url); + nav_bookmarks_service.insertBookmark(nav_bookmarks_service.bookmarksMenuFolder, + uri, -1, title); + if (tags && tags.length > 0) + for (i in tags) nav_bookmarks_service.insertBookmark(user_tags[tags[i]].id, + uri, -1, null); } diff --git a/modules/minibuffer-read.js b/modules/minibuffer-read.js index 0d306bf..ffe40e3 100644 --- a/modules/minibuffer-read.js +++ b/modules/minibuffer-read.js @@ -36,7 +36,7 @@ define_variable("minibuffer_history_max_items", 100, * * default_completion only used if match_required is set to true * - * $valiator [optional] + * $validator [optional] * specifies a function */ define_keywords("$history", "$validator", -- 1.5.6.5 _______________________________________________ Conkeror mailing list Conkeror@... https://www.mozdev.org/mailman/listinfo/conkeror |
|
|
Re: [PATCH] Bookmark Tags
by David Kettler-2
::
Rate this Message:
G'day Michael, thanks for your contribution. Take my comments below
as suggestions; normally John or Jeremy will accept patches. > To: Shawn Betts <sabetts@...> I don't think Shawn is very much involved in development any more. > Support for bookmark tags, in the same way that Firefox supports tags I think this is a good idea. I normally kludge something similar by editing the bookmark name to make sure it has a few appropriate keywords. > minibuffer_read_tag_enable = true; > define_tag("gnu","GNU Not UNIX"); > define_tag("uci","University of California, Irvine"); > define_tag("r","R language"); > define_tag("emacs","Emacs"); > define_tag("blog","Interesting Blog"); What use is made of the descriptions? > And you will be prompted for tags when invoking the interactive > "bookmark" command. Any number of tags can be added, and the input > method works the same as keywords in auto-insert.el in Emacs, i.e. you > end input with an empty tag name. That sounds awkward. How about just one input line with multiple space-separated tags (I don't know whether tags may contain spaces, but I don't think it would be sensible to use spaces in tag names anyway). > I've also fixed a typo in minibuffer-read.js. It's better not to mix different changes into one patch. Sometimes it's okay if the trivial change is close to another edit. > * Notes > > I'm not sure where I should have put these notes This is fine, but see also below. > - The XULRunner support for Tags is pretty lame, there are a lot of > missing methods, for example, there is no way to save a Description > for a tag with the tag. And there is no way to get a list of all tags, > except to iterate over the "Tags" folder until you hit an Index Out of > Bounds error. This is why tags are explicitly defined in the > ~/.conkerorrc, with descriptions. That's a shame. > - I would gladly edit the Wiki Bookmarks page for this, if it gets > added. Just an idea; I included the intended wiki text in the notes of a recent patch of mine. > Subject: [PATCH] Added support for bookmark tags It's a bit easier for reviewers if the patch text is included inline rather than attached; that way it gets quoted in the reply. A lot of the explanation above would be useful in the commit message. > --- You can include notes that you don't intend to go in the commit message here, after the three dashes; git will elide those when applying the patch. > modules/commands.js | 12 ++++++++++-- > modules/content-buffer.js | 31 +++++++++++++++++++++++++++++++ > modules/history.js | 35 ++++++++++++++++++++++++++++++++--- > modules/minibuffer-read.js | 2 +- > 4 files changed, 74 insertions(+), 6 deletions(-) > + while(true) { > + tag = yield I.minibuffer.read_tag($prompt = "Bookmark with tag:", > + $initial_value = ""); > + if (tag == "") break; else tags.push(tag); > + } > + See above comment about reading all tags on one line. > + $validator = function (x, m) { > + if (x in user_tags || x == "") return true; > + else { m.message("No match"); return false; } If you use $match_required = true then you won't need the validator. > -function add_bookmark(url, title) { > - nav_bookmarks_service.insertBookmark(nav_bookmarks_service.unfiledBookmarksFolder, > - make_uri(url), -1, title); > + nav_bookmarks_service.insertBookmark(nav_bookmarks_service.bookmarksMenuFolder, > + uri, -1, title); Why this change in folder? regards, David. _______________________________________________ Conkeror mailing list Conkeror@... https://www.mozdev.org/mailman/listinfo/conkeror |
|
|
|
|
|
Re: [PATCH] Bookmark Tags
by John J. Foerch
::
Rate this Message:
On Fri, Aug 07, 2009 at 07:43:45PM -0700, Michael Zeller wrote:
> >> nav_bookmarks_service.insertBookmark(nav_bookmarks_service.unfiledBookmarksFolder, > >>> - make_uri(url), -1, title); > >> > >>> + > >> nav_bookmarks_service.insertBookmark(nav_bookmarks_service.bookmarksMenuFolder, > >>> + uri, -1, title); > >> > >> Why this change in folder? > > > > Oops, could catch. Firefox uses bookmarksMenuFolder when you create a > > bookmark (parent == 1 in places.sqlite), whereas Conkeror is using > > unfiledBookmarksFolder(5). I changed to test if search still worked with > > (1), but I didn't mean to commit this, I'll remove it. > > > > But that brings up a question of my own: why does Conkeror use (5) and > > not (1)? I suppose one advantage to the current way is that (5) would > > allow potential differentiation from Firefox bookmarks and Conkeror > > bookmarks, if they were to be merged somehow? The mozilla bookmarks API is flawed in the sense that its folders correspond to specific Firefox UI elements, rather than generic concepts. The choice of unfiledBookmarksFolder makes sense with our current bookmarks UI because Conkeror offers no way to "file" a bookmark. I am working on a bookmark manager, though, in which this property will be configurable, so if you want to change it for your own purposes, please just change it in your own copy. Unfortunately for Conkeror, my work has picked up a lot and I don't have the time to put into Conkeror right now as I have in the past, so it may be a long time before I can finish this work. -- John Foerch _______________________________________________ Conkeror mailing list Conkeror@... https://www.mozdev.org/mailman/listinfo/conkeror |
| Free embeddable forum powered by Nabble | Forum Help |
