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] selectively update feeds #109

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

Comments

Projects
None yet
2 participants
@groks
Contributor

groks commented Nov 20, 2015

I'd like to be able to update some subset of the feeds in elfeed-feeds, say only the feeds which are currently providing entries in elfeed-search. Here's a quick mockup:

(defun my-elfeed-update-selectively (orig-fun)
  "Only update the feeds currently providing entries"
  (let ((elfeed-feeds (delete-dups (cl-loop for entry in elfeed-search-entries
                                            collect (elfeed-feed-url (elfeed-entry-feed entry))))))
    (call-interactively orig-fun)))

(advice-add 'elfeed-update :around #'my-elfeed-update-selectively)

I think it needs to be a little bit fancier. It should act as if no 'unread' tag is present, to bring new entries into the results. And it should only activate selective update on C-u G.

How about something like this? Much faster to update a small subset of a large collection of feeds.

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

Add prefix argument to G in search mode (#109).
With a prefix, only visible feeds are updated.
@skeeto

This comment has been minimized.

Show comment
Hide comment
@skeeto

skeeto Nov 21, 2015

Owner

This is a great idea. Way back I considered a feature for updating feeds
that have entries matching a specific tag, but I could never figure out
a good interface. Should it prompt the user to enter tags? That's
awkward, clunky, and doesn't compose with anything else. It also
requires a full database scan.

However, your idea builds directly on top of the existing filter
interface. It's a perfectly natural way to narrow to the desired
selection, and it leverages all the features of the filter. Why didn't I
think of this before?

I made a fetch-visible feature branch, which you should see associated
with this issue. Does that commit do what you had in mind? I didn't use
any of your code. Firstly, because there's an elfeed-update-feed for
selectively updating an individual feed. No need to wrap elfeed-update
with advice. Secondly, Emacs' advice is really flaky and should only be
used as a last resort for extensions not otherwise possible. It's
definitely not something to use within a package.

I regret using the word "update" in the search buffer to refer to
redrawing since it's different from the meaning of "update" for the core
of Elfeed. I've chosen "fetch" for describing this action within the
search buffer context.

Owner

skeeto commented Nov 21, 2015

This is a great idea. Way back I considered a feature for updating feeds
that have entries matching a specific tag, but I could never figure out
a good interface. Should it prompt the user to enter tags? That's
awkward, clunky, and doesn't compose with anything else. It also
requires a full database scan.

However, your idea builds directly on top of the existing filter
interface. It's a perfectly natural way to narrow to the desired
selection, and it leverages all the features of the filter. Why didn't I
think of this before?

I made a fetch-visible feature branch, which you should see associated
with this issue. Does that commit do what you had in mind? I didn't use
any of your code. Firstly, because there's an elfeed-update-feed for
selectively updating an individual feed. No need to wrap elfeed-update
with advice. Secondly, Emacs' advice is really flaky and should only be
used as a last resort for extensions not otherwise possible. It's
definitely not something to use within a package.

I regret using the word "update" in the search buffer to refer to
redrawing since it's different from the meaning of "update" for the core
of Elfeed. I've chosen "fetch" for describing this action within the
search buffer context.

@groks

This comment has been minimized.

Show comment
Hide comment
@groks

groks Nov 21, 2015

Contributor

Looks good!

I'm still wondering about this case though:

You have some search defined and it selects entries from two feeds. You read some of the entries and leave some longer ones for later, and it just so happens that all entries from one of the feeds have been read, or they fall outside your time restriction, and therefore none show up in the search.

Your intention now when you press C-u G is "show me more like the ones I've just read". ie: show me entries that would be visible, if the database was up to date. But this doesn't work.

One way around this gotcha is to document it and say: you probably want to extend any short dated time restriction and remove the +unread tag from your search, press C-u G, then re-add those restrictions to get what you probably expect.

Another way would be to automatically modify those search terms before running fetch-visible, although ideally this would happen off-screen to prevent a flash of old entries.

Anyway, fetch-visible is a neat improvement so far!

Contributor

groks commented Nov 21, 2015

Looks good!

I'm still wondering about this case though:

You have some search defined and it selects entries from two feeds. You read some of the entries and leave some longer ones for later, and it just so happens that all entries from one of the feeds have been read, or they fall outside your time restriction, and therefore none show up in the search.

Your intention now when you press C-u G is "show me more like the ones I've just read". ie: show me entries that would be visible, if the database was up to date. But this doesn't work.

One way around this gotcha is to document it and say: you probably want to extend any short dated time restriction and remove the +unread tag from your search, press C-u G, then re-add those restrictions to get what you probably expect.

Another way would be to automatically modify those search terms before running fetch-visible, although ideally this would happen off-screen to prevent a flash of old entries.

Anyway, fetch-visible is a neat improvement so far!

@skeeto

This comment has been minimized.

Show comment
Hide comment
@skeeto

skeeto Nov 23, 2015

Owner

One of my core guidelines for Elfeed's interface is that it shouldn't be
surprising. Given the gist of a new feature, the user should be able to
guess how it works. Or if two different people were to implement it, the
resulting interface would be identical, even if different approaches
were taken to implement it. For example, the live search filter should
make sense almost immediately, with predictable results.

"Update exactly what I currently see" is obvious. With the way I
implemented it so far, it will update feeds for entries not currently
passing the filter so long as they're still listed. You know, when
they're in limbo after modifying the tags but before the search buffer
has been refreshed (g). Quietly adjusting the filter in the background
just for fetching is surprising and non-obvious. There's also the
question of how much the time component is adjusted.

Another core guideline is composability and extensibility. Given
something more complicated or specialized, particularly with some
non-obvious magic behavior, how difficult would it be to implement as an
extension? If it's your own feature, nothing is surprising about it. For
example, many users (including me) have different "views" of the search
listing bound to certain keys. These keys run a command that
sets/adjusts the filter. By adding and removing "-emacs" in the filter,
entries about Emacs are hidden and revealed (assuming they're
appropriately tagged). It's very easy to do as an extension.

In your case, I'd like to first try implementing the idea as an
extension, just as a regular user would, to see in what way Elfeed
currently causes friction, fixing those issues in ways that follow the
two guidelines. For example, I foresee the problem that you might need
to parse the time component yourself. The parsed filter data structure
is currently an internal detail, and the solution might be to formalize
it.

Owner

skeeto commented Nov 23, 2015

One of my core guidelines for Elfeed's interface is that it shouldn't be
surprising. Given the gist of a new feature, the user should be able to
guess how it works. Or if two different people were to implement it, the
resulting interface would be identical, even if different approaches
were taken to implement it. For example, the live search filter should
make sense almost immediately, with predictable results.

"Update exactly what I currently see" is obvious. With the way I
implemented it so far, it will update feeds for entries not currently
passing the filter so long as they're still listed. You know, when
they're in limbo after modifying the tags but before the search buffer
has been refreshed (g). Quietly adjusting the filter in the background
just for fetching is surprising and non-obvious. There's also the
question of how much the time component is adjusted.

Another core guideline is composability and extensibility. Given
something more complicated or specialized, particularly with some
non-obvious magic behavior, how difficult would it be to implement as an
extension? If it's your own feature, nothing is surprising about it. For
example, many users (including me) have different "views" of the search
listing bound to certain keys. These keys run a command that
sets/adjusts the filter. By adding and removing "-emacs" in the filter,
entries about Emacs are hidden and revealed (assuming they're
appropriately tagged). It's very easy to do as an extension.

In your case, I'd like to first try implementing the idea as an
extension, just as a regular user would, to see in what way Elfeed
currently causes friction, fixing those issues in ways that follow the
two guidelines. For example, I foresee the problem that you might need
to parse the time component yourself. The parsed filter data structure
is currently an internal detail, and the solution might be to formalize
it.

@groks

This comment has been minimized.

Show comment
Hide comment
@groks

groks Nov 23, 2015

Contributor

Yup, I agree. The corner cases occurred to me and the solution above was more or less me just talking out loud.

Anyway, I've been poking at it a bit more and came up with these two manual toggles:

(defun my-elfeed-search-toggle-unread ()
  "Toggle the '+unread' tag on the current `elfeed-search-filter'."
  (interactive)
  (let* ((filter elfeed-search-filter)
         (tags (nth 1 (elfeed-search-parse-filter filter))))
    (elfeed-search-set-filter (if (memq 'unread tags)
                                  (replace-regexp-in-string " ?\\(\\+unread\\)" "" filter)
                                (replace-regexp-in-string " ?\\'" " +unread" filter)))))

(defvar my-elfeed-search-previous-date ""
  "The @date to toggle back into `elfeed-search-filter'.")

(defun my-elfeed-search-toggle-date ()
  "Toggle the @date on the current `elfeed-search-filter'."
  (interactive)
  (save-match-data
    (let ((filter elfeed-search-filter)
          (date my-elfeed-search-previous-date))
      (elfeed-search-set-filter (if (string-match "@[0-9a-zA-Z-]+" filter)
                                    (progn
                                      (setq my-elfeed-search-previous-date (match-string 0 filter))
                                      (replace-regexp-in-string "@[0-9a-zA-Z-]+ *" "" filter))
                                  (concat date
                                          (if (eq date "") "" " ")
                                          filter))))))

(my-eval-after-load elfeed-search
    (define-key elfeed-search-mode-map "U" #'my-elfeed-search-toggle-unread)
    (define-key elfeed-search-mode-map "@" #'my-elfeed-search-toggle-date))

With them you can quickly toggle the unread tag and date restriction. Feels pretty good, and no need for weird auto-magic behaviour in the updating.

Contributor

groks commented Nov 23, 2015

Yup, I agree. The corner cases occurred to me and the solution above was more or less me just talking out loud.

Anyway, I've been poking at it a bit more and came up with these two manual toggles:

(defun my-elfeed-search-toggle-unread ()
  "Toggle the '+unread' tag on the current `elfeed-search-filter'."
  (interactive)
  (let* ((filter elfeed-search-filter)
         (tags (nth 1 (elfeed-search-parse-filter filter))))
    (elfeed-search-set-filter (if (memq 'unread tags)
                                  (replace-regexp-in-string " ?\\(\\+unread\\)" "" filter)
                                (replace-regexp-in-string " ?\\'" " +unread" filter)))))

(defvar my-elfeed-search-previous-date ""
  "The @date to toggle back into `elfeed-search-filter'.")

(defun my-elfeed-search-toggle-date ()
  "Toggle the @date on the current `elfeed-search-filter'."
  (interactive)
  (save-match-data
    (let ((filter elfeed-search-filter)
          (date my-elfeed-search-previous-date))
      (elfeed-search-set-filter (if (string-match "@[0-9a-zA-Z-]+" filter)
                                    (progn
                                      (setq my-elfeed-search-previous-date (match-string 0 filter))
                                      (replace-regexp-in-string "@[0-9a-zA-Z-]+ *" "" filter))
                                  (concat date
                                          (if (eq date "") "" " ")
                                          filter))))))

(my-eval-after-load elfeed-search
    (define-key elfeed-search-mode-map "U" #'my-elfeed-search-toggle-unread)
    (define-key elfeed-search-mode-map "@" #'my-elfeed-search-toggle-date))

With them you can quickly toggle the unread tag and date restriction. Feels pretty good, and no need for weird auto-magic behaviour in the updating.

@skeeto

This comment has been minimized.

Show comment
Hide comment
@skeeto

skeeto May 8, 2018

Owner

Closing since this was mostly addressed in daa3030.

Owner

skeeto commented May 8, 2018

Closing since this was mostly addressed in daa3030.

@skeeto skeeto closed this May 8, 2018

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