New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] bookmark integration #110

Closed
groks opened this Issue Nov 21, 2015 · 5 comments

Comments

Projects
None yet
3 participants
@groks
Contributor

groks commented Nov 21, 2015

Emacs has a bookmark feature: C-x r m to add a bookmark to the current file, C-x r b to jump to the file where a bookmark points.

You can define new types of bookmarks for things other than files, so here's one for elfeed-search:

(defun my-elfeed-bookmark-make-record-search ()
  "Return a bookmark record for the current elfeed-search buffer."
  `(,(format "elfeed %s" elfeed-search-filter)
    (location . ,elfeed-search-filter)
    (tags ,@(mapcar #'symbol-name
                           (nth 1 (elfeed-search-parse-filter elfeed-search-filter))))
    (handler . my-elfeed-bookmark-handler-search)))

(defun my-elfeed-bookmark-handler-search (bmk-record)
  "Jump to an elfeed-search bookmarked location."
  (switch-to-buffer (elfeed-search-buffer))
  (unless (eq major-mode 'elfeed-search-mode)
    (elfeed-search-mode))
  (elfeed-search-set-filter (bookmark-prop-get bmk-record 'location)))

(defun my-elfeed-search-mode-hook ()
  (setq bookmark-make-record-function #'my-elfeed-bookmark-make-record-search))

(add-hook 'elfeed-search-mode-hook #'my-elfeed-search-mode-hook)

Emacs bookmarks do not support tags by default, but bookmark+ (in melpa) does, and the code above pre-populates the tags field.

Bookmarks could be a quick way to support more than the default elfeed-search-filter without having to create a whole new front-end.

@skeeto

This comment has been minimized.

Show comment
Hide comment
@skeeto

skeeto Nov 23, 2015

Owner

Not a bad idea. It would certainly simplify the process of creating easy-access "views" on the listing buffer. However, I don't want to create a dependency on bookmark+ just for this feature. If you can get it to work with just Emacs' built-in bookmark.el, I'd like to integrate this (and without the add-hook, since it would be part of the elfeed-search-mode itself).

Owner

skeeto commented Nov 23, 2015

Not a bad idea. It would certainly simplify the process of creating easy-access "views" on the listing buffer. However, I don't want to create a dependency on bookmark+ just for this feature. If you can get it to work with just Emacs' built-in bookmark.el, I'd like to integrate this (and without the add-hook, since it would be part of the elfeed-search-mode itself).

@groks

This comment has been minimized.

Show comment
Hide comment
@groks

groks Nov 23, 2015

Contributor

There's no dependency on bookmark+. The Emacs bookmark system allows storing extra meta-data with each bookmark. Rather than use the key 'elfeed-tags' I used 'tags' because bookmark+ takes advantage of it. Without bookmark+ the tags meta-data is ignored.

This also wants the ability to bookmark items as well as searches (like #34), I'll have a look at that.

Contributor

groks commented Nov 23, 2015

There's no dependency on bookmark+. The Emacs bookmark system allows storing extra meta-data with each bookmark. Rather than use the key 'elfeed-tags' I used 'tags' because bookmark+ takes advantage of it. Without bookmark+ the tags meta-data is ignored.

This also wants the ability to bookmark items as well as searches (like #34), I'll have a look at that.

skeeto added a commit that referenced this issue Nov 23, 2015

Add support for bookmarks (#110).
Elfeed now naturally supports bookmark-set (C-x r m) and
bookmark-jump (C-x r b). Thanks, groks!
@skeeto

This comment has been minimized.

Show comment
Hide comment
@skeeto

skeeto Nov 23, 2015

Owner

Ah, gotcha! I tried your code and it didn't work, so I assumed it was
from missing bookmark+. It was only because I forgot to restart
elfeed-search-mode in my current session. This works great! It's been
committed.

Great feature, thanks!

Owner

skeeto commented Nov 23, 2015

Ah, gotcha! I tried your code and it didn't work, so I assumed it was
from missing bookmark+. It was only because I forgot to restart
elfeed-search-mode in my current session. This works great! It's been
committed.

Great feature, thanks!

@skeeto skeeto closed this Nov 23, 2015

@baudilio

This comment has been minimized.

Show comment
Hide comment
@baudilio

baudilio Sep 28, 2017

If I'm not mistaken, I think I found a bug in the bookmark/bookmark+/elfeed integration.
I followed skeeto's procedure. When I try to create a new bookmark, I get the following error:

Debugger entered--Lisp error: (wrong-type-argument sequencep 604800)
mapcar(symbol-name 604800)
elfeed-search-bookmark-make-record()
bookmark-make-record()
bookmark-set-internal("Set bookmark unconditionally" nil overwrite)
bookmark-set(nil nil)
funcall-interactively(bookmark-set nil nil)
call-interactively(bookmark-set nil nil)
command-execute(bookmark-set)

I suppose you'll need more information and details on the issue (OS, packages installed, emacs version, etc., etc. Before sending irrelevant data, I'll await for your directions.

baudilio commented Sep 28, 2017

If I'm not mistaken, I think I found a bug in the bookmark/bookmark+/elfeed integration.
I followed skeeto's procedure. When I try to create a new bookmark, I get the following error:

Debugger entered--Lisp error: (wrong-type-argument sequencep 604800)
mapcar(symbol-name 604800)
elfeed-search-bookmark-make-record()
bookmark-make-record()
bookmark-set-internal("Set bookmark unconditionally" nil overwrite)
bookmark-set(nil nil)
funcall-interactively(bookmark-set nil nil)
call-interactively(bookmark-set nil nil)
command-execute(bookmark-set)

I suppose you'll need more information and details on the issue (OS, packages installed, emacs version, etc., etc. Before sending irrelevant data, I'll await for your directions.

@skeeto

This comment has been minimized.

Show comment
Hide comment
@skeeto

skeeto Sep 29, 2017

Owner
Owner

skeeto commented Sep 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment