Make extra backends works more easily

[fasheng]

With the new added elfeed-update-function, elfeed-tag-hooks and
elfeed-untag-hooks, it will be more easily to support extra elfeed
backends.

The ownCloud News in elfeed-backends works well now, you could have a try with the 10 minutes tutorial in README:
https://github.com/fasheng/elfeed-backends

BTW: what do you think about the design in this patch, any advice?

[skeeto]

I'm mostly happy with elfeed-(un)tag-hooks. That's the right way to do this, especially with the way you aggregate some of the changes. If you want to aggregate even more changes without affecting the API, there's a hack in elfeed-db.el you should look at which uses run-at-time. It delays an action until the next event loop turn. This would allow you to accumulate a set of changes made together with multiple calls (in a loop, etc.) into a single "transaction." I'd rather leave elfeed-search.el ignorant of the new hook and just call the database functions like everyone else. Perhaps elfeed-tag and elfeed-untag could optionally accept a list of entries for the first argument, which would take better advantage of the new hook. A note on the name: According to the Elisp manual, "-hooks" is a "deprecated suffix." This is because "hook" describes the variable, not the functions, so it should be singular. Also, the suffix "-hook" is only intended for functions that don't take arguments, despite the name of "run-hook-with-args" (even Emacs itself is inconsistent). Elfeed already eschews these conventions, so this is alright. The elfeed-is-status-error function should be moved to elfeed.el, and the second argument should be optional, defaulting to the value of elfeed-use-curl (necessitating the move to elfeed.el). Otherwise this looks good. I'm not satisfied with elfeed-update-function. If it's a function, it shouldn't be in an eq branch but just called. It's also at the wrong abstraction level. You shouldn't need to change anything in elfeed-search.el or elfeed-show.el unless you want to change how entries are actually displayed. What are you trying to do with elfeed-update-function?

[fasheng]

Thanks for your advice, sir. It's really helpful.

I push another comments now, so you can easily see the changes. All the comments will be merged to one at end.

The elfeed-is-status-error was moved to elfeed.el now and used in elfeed-update-feed, but I prefer don't let argument use-curl as optional and even default value to elfeed-use-curl. For example in elfeed-update-feed, the local variable use-curl wrap the value from global efeed-use-curl, looks there's no reason to use elfeed-use-curl again to change the following logical in elfeed-is-status-error.

And thanks for showing run-at-time for me, I really don't use it before and it's so cool. I read the doc and found that it will not solve the merging multiple tag/untag actions to only one problem which will prevent calling multiple curl processes when user change tags for large number of entries at one time, at least could not solve it directly. So I follow the advice add new function elfeed-tag/untag-multi which accept entries as argument and map original elfeed-tag/untag to it, and adjust the code in elfeed-search.el, things just looks better. If you think elfeed-tag and elfeed-tag-multi should be merged to one, I'd like to do the work and update the rest code, too.

About elfeed-update-function, I really have no good idea. Looks elfeed should provide a full implemtation to support extra backend, but I don't know how to begin it so just add elfeed-update-function to allow user use another method to fetch entries. And the changes in elfeed-search is just to skip call the original update operation like elfeed-update-feed if user customized another one. So any idea, sir?

[fasheng]

How about just drop changes about elfeed-update-function, then elfeed-backends will define advice for it.Tthis will works well mostly, too.

[skeeto]

For example in elfeed-update-feed, the local variable use-curl wrap the value from global efeed-use-curl, looks there's no reason to use elfeed-use-curl again to change the following logical in elfeed-is-status-error.

I see what you mean. It's fine the way you wrote it.

If you think elfeed-tag and elfeed-tag-multi should be merged to one, I'd like to do the work and update the rest code, too.

Rather than add a new pair of functions to the API, I think I just want to stick with my original idea of accepting either a single entry or a list of entries. Like so: (defun elfeed-tag (entry-or-entry-list tags) (if (elfeed-entry-p entry-or-entry-list) (list ((entries (list entry-or-entry-list))) (run-hook-with-args #'elfeed-tag-hooks entries tags) (elfeed-tag-1 entry-or-entry-list tags)) (run-hook-with-args #'elfeed-tag-hooks entry-or-entry-list tags) (apply #'elfeed-tag-1 entry-or-entry-list tags)))

About elfeed-update-function, I really have no good idea. Looks elfeed should provide a full implemtation to support extra backend, but I don't know how to begin it so just add elfeed-update-function to allow user use another method to fetch entries. And the changes in elfeed-search is just to skip call the original update operation like elfeed-update-feed if user customized another one. So any idea, sir?

So would you want ownCloud (or whatever backend) to do all the feed fetching, too? Or is this to pull in additional entries from another source? Or is this replacing the database backend? The latter is how I've interpreted your intentions so far: Rather than have Elfeed write to its own index, it synchronizes with another storage backend. That backend might add its own entries independently, which should appear in Elfeed. In this case you don't want to replace elfeed-update, you want to replace/hook/redirect the database functions. A database basically has to do these things: * db-get-feed: Get a feed object by its url * db-get-entry: Get an entry object by its id * db-db-add: Add/merge entries into the database * db-set/get-update-time: Simple update of the last database change * tag/untag: Duh. * metadata set/get: Additional entry and feed metadata (optional) * feed-entries: Efficiently get all the entries belonging to a feed * ref/deref: Store/load entry content * db-visit: Efficiently visit all entries in time order, descending I wrote that list just walking down elfeed-db.el. If this was abstracted cleanly, then there could be alternate backends to service all these requests. It could be as simple as a plist of "methods" that gets installed as the backend, with the current file-based database as the default installed backend. (defvar owncloud-methods (:get-feed #'owncloud-get-feed :get-entry #'owncloud-get-entry :visit #'owncloud visit :tag #'owncloud-tag :untag #'owncloud-untag ...))

[fasheng]

Cool argument style, I just didn't thought it before, the code updated now.

Maybe I mistake the source and backend. Anyway, for example ownCloud News, I just want make such remote source works like this:

1. Fetch and parse articles from remote, and just save them in the original elfeed database because the parsed entries format are same with others besides they own a special metadata to determine who fetched them like (:source ocnews)
2. Remember necessary configuration like the last modified timestamp. I thinks make the remote source act as a mock feed item and save them in the metadata is just fine
3. Keep the basic features with the help of tags, for example sync unread/starred stats with remote

So looks it's no necessary to replace the database backend at all, and the visit method is in the same, too. And another point is that the remote souce dispatch fetch all and fetch special feeds is a little different. If we define a owncloud-method, it should looks like this:

(defvar elfeed-default-methods
  (:update-all #'elfeed-update-all
   :update-feed #'elfeed-update-feed
   :tag #'elfeed-tag
   :untag #'elfee-untag
   ...))

(defvar elfeed-owncloud-methods
  (:update-all #'elfeed-owncloud-update-all
   :update-feed #'elfeed-owncloud-update-feed
   :tag #'elfeed-owncloud-tag
   :untag #'elfeed-owncloud-untag
   ...))

The new code will be implemented soon.

[fasheng]

Hi sir, the new code updated now, how about this time? The changes are:

1. :pre-tag/untag were used instead of :tag/untag because the extend source may don't want to override the elfeed-tag/untag like :update-all and :update-feed did, they just append some alternate operations. And elfeed-tag/untag-hooks were left along, too, but I believe they will be useful for others, later.
2. the split package was renamed to elfeed-sources

elfeed-methods styles:

(defvar elfeed-default-methods
  '(:update-all #'elfeed-update-all
    :update-feed #'elfeed-update-feed))
(defvar elfeed-sources-ocnews-methods
  '(:update-all 'elfeed-sources-ocnews-update-all
    :update-feed 'elfeed-sources-ocnews-update-feed
    :pre-tag 'elfeed-sources-ocnews-pre-tag
    :pre-untag 'elfeed-sources-ocnews-pre-untag))

BTW: wish the style seems not strange for you, otherwise I really do not have a better way 😭

[fasheng]

Hi sir, looks the source/backend interface design not so clearly. How about merge the elfeed-tag/untag-hooks firstly? And elfeed-sources will works fine with it :)

The commts were rebased, the old code keeps here:
https://github.com/fasheng/elfeed/tree/old/make-extra-backends-easily

[skeeto]

I see what you want now. I thought you were looking to store the database in ownCloud. I think the right way to go about this is with a protocol registry. The feeds would be added as "owncloud://..." and when Elfeed goes to fetch a feed, it first takes a look at the protocol. If it's non-standard, it looks it up in a registry and calls the appropriate function that asynchronously returns a list of entries. For example, your extension would do something like this: (elfeed-register-protocol "owncloud" (:fetch #'owncloud-fetch :tag #'owncloud-tag :untag #'owncloud-untag)) However, I don't understand yet why ownCloud needs to be informed about Elfeed's tags. The function that returns entries could include the tags it wants and Elfeed will merge these into the database. If new tags are added, ownCloud shouldn't have to know about them, unless I'm missing something.

[fasheng]

Sorry not describe it clearly before, my English is not very well..

1. ownCloud News actually is a collection of feed urls, and it owns update and update-feed methods, too. So just define it as a feed with special protocol is not fine. In fact, if ownCloud was enabled, the origin local feed urls should be disabled.

2. Yes, some self-host RSS reader like Tiny Tiny RSS support tags itself, but mostly not. So the tags here were only used to check/sync unread/starred stats with remote. And it will not affect database storage locally, that's why I use :pretag instead of :tag. Of course, we could use :tag symbol, but all the elfeed-tag related code need be hacked, too. I'm afraid this will make the code messy.

And really sorry that didn't point the code was inspired by newsbeuter before, which is a terminal RSS reader that already support ownCloud, Tiny Tiny RSS and Newsblur so far. Here is the abstract interfaces it used:
https://github.com/akrennmair/newsbeuter/blob/master/include/remote_api.h

Now my goal is to continue the work in the separate package with help of hooks and advices, just like elfeed-org did. Once the new interface designed, I'd like to update the code then and it should be a easy work.

[skeeto]

An alternate "protocol" could be richer than fetching a single feed's content over HTTP. The implementation of that protocol could manage multiple feed object creation in addition to adding new entries to the database. When Elfeed sees a feed with the owncloud:// protocol, it calls a function with that URL and a callback, and leaves it up to that function to asynchronously add entries+feeds to the database, then call back with entries when it's done. Here's a really rough idea of what I mean: (defun owncloud-protocol (url callback) (let ((feeds-urls (owncloud-feeds url ...)) (entries ())) ;; this part happens asynchronously: (dolist (feed-url feed-urls) (push (elfeed--entry-create :feed-id feed-url ...) entries)) (funcall callback entries))) This function is free to update any feed and any entry in the database. It also doesn't need to hook elfeed-update, since it gets called naturally like any other URL. Elfeed assumes its busy until it makes the callback. Technically it could even use elfeed-db-add itself rather than pass the entries back out. The only thing this needs is a protocol registry and a non-private way to create entry objects (e.g. a public alternative to elfeed--entry-create).

[fasheng]

Well, this could work, but will fetch each feed one time just like newsbeuter did, and the problem is if there are hundreds of feeds in remote and the user must send hundreds of requests, too. In fact, the self-hosting feed readers always provide API to fetch all articles in one request. That's why I point out to separate update-all and update-feed in different abstract methods before.

And if will be a little strange for user if we put "ownCloud://url" in elfeed-feeds and store the auth info in another variable..