Autotagging does not work

madalu commented 4 years ago

First, thanks for this excellent package! I'm finding that there are multiple issues with auto-tagging using the :autotags property and ttrss. Take these example settings:

 (setq elfeed-feeds '(
                      ("ttrss+https://www.example.com/tt-rss/"
                       :password "password"
                       :autotags '(("https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml" news)))))

There are a few problems that prevent the autotags from ever being recognized.

elfeed-protocol-meta-feed does not return the feed object (the string-match syntax in that function is incorrect).

As a result, with the above setting for elfeed-feed, here's what the function returns:

(elfeed-protocol-meta-feed "ttrss") => nil
elfeed-protocol-advice-feed-autotags calls calls elfeed-protocol-host-url when it should call elfeed-protocol-type.

As a result, the proto-id given as an argument to elfeed-protocol-feed-autotags is the entire server address ("ttrss+https://www.example.com/tt-rss/") although it expects the type ("ttrss") as an argument.
elfeed-protocol-advice-feed-autotags expects a feed url in the following format:

"ttrss+https://www.example.com/tt-rss/::https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml"

However, the feed url that elfeed actually sends to elfeed-feed-autotags and thus also to elfeed-protocol-advice-feed-autotags is simply "https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml" If the feed being sent to the elfeed-feed-autotags is just the feed url protocol autotagging in its current form will always fail, since both elfeed-feed-p and elfeed-protocol-subfeed-p will return "nil" on that url.

For all these reasons, the autotags are never returned. I will submit PR's to fix 1 and 2 shortly.

However, I'm not quite sure how to fix 3, as I am less familiar with elfeed's internals. And while 1 & 2 would affect all backends, I don't know whether 3 affects all or just ttrss.

emacs version: 27.1
elfeed version: 3.3.0
elfeed-protocol version: 20201004.156

fasheng commented 4 years ago

Thanks for the detailed description. Looks you misunderstood the format of PROTO-ID, in your example it should be ttrss+https://www.example.com/tt-rss/ instead of ttrss.

And for your issue, it should be caused by the quoted autotags in elfeed-feeds, you could check the different results with the following code. I use elfeed-org all the time so didn't realize this issue before. Thanks your report and PR, I will provide my solution later and please test again.

(let* ((elfeed-feeds '(("ttrss+https://www.example.com/tt-rss/"
                        :password "password"
                        :autotags '(("https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml" news))))))
  (elfeed-protocol-meta-autotags "ttrss+https://www.example.com/tt-rss/"))

(let* ((elfeed-feeds (list (list "ttrss+https://www.example.com/tt-rss/"
                        :password "password"
                        :autotags '(("https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml" news))))))
  (elfeed-protocol-meta-autotags "ttrss+https://www.example.com/tt-rss/"))

fasheng commented 4 years ago

Well, ignore my new commit, how about the result if you change the elfeed-feeds format to:

(setq elfeed-feeds (list
                      (list "ttrss+https://www.example.com/tt-rss/"
                       :password "password"
                       :autotags '(("https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml" news)))))

madalu commented 4 years ago

Thanks! You are right. I misunderstand proto-id before. The following now works:

  (let* ((elfeed-feeds (list (list "ttrss+https://www.example.com/tt-rss/"
                      :password "password"
                      :autotags '(("https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml" news))))))
             (elfeed-protocol-meta-autotags "ttrss+https://www.example.com/tt-rss/"))

Evaluating it returns:

   (("https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml" news))

So feel free to close my earlier pull requests, which rested on a misdiagnosis of the problem. However, I think point 3 may still be an issue.

fasheng commented 4 years ago

Don't worry about point 3, elfeed-protocol didn't parse autotags iteself and will override elfeed-feeds before calling the elfeed-feed-autotags, so send the normal feed url will be fine: https://github.com/fasheng/elfeed-protocol/blob/master/elfeed-protocol-common.el#L191

You could check it with the test code, too:

(let* ((elfeed-feeds (list (list "ttrss+https://www.example.com/tt-rss/"
                                 :password "password"
                                 :autotags '(("https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml" news))))))
  (elfeed-protocol-feed-autotags "ttrss+https://www.example.com/tt-rss/" "https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml")
  (elfeed-feed-autotags "ttrss+https://www.example.com/tt-rss/::https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml"))

fasheng commented 3 years ago

I believe the problem should be solved~

madalu commented 3 years ago

Thanks! Everything is working well now. No problem with auto-tagging, and the tests work.

fasheng / elfeed-protocol

Autotagging does not work #41