Open guyfedwards opened 10 months ago
Hmm, was looking at this issue thinking I could look into it, but I actually can't get RSS feeds to work from --feed
in general. Refreshing just results in the same subscribed feeds for me. Is this expected at the moment?
@kanielrkirby That's been my experience.
Hm, I'll open a separate issue for this, it seems to be a regression.
So what I'm seeing is that the issue is this, in Commands#TUI():
its, err := c.GetAllFeeds()
if err != nil {
return fmt.Errorf("commands List: %w", err)
}
var errorItems []ErrorItem
// if no feeds in store or we have preview feeds, fetchAllFeeds
if len(its) == 0 || len(c.config.PreviewFeeds) > 0 {
_, errorItems, err = c.fetchAllFeeds()
if err != nil {
return fmt.Errorf("[commands.go] TUI: %w", err)
}
// refetch for consistent data across calls
its, err = c.GetAllFeeds()
if err != nil {
return fmt.Errorf("[commands.go] TUI: %w", err)
}
}
What ends up happening is that you'll fetchAllFeeds
, which DOES return preview feeds correctly, but it's passed into _
. GetAllFeeds
doesn't seem to take into account preview feeds at all, which complicates matters.
There's a few options here, and what seems simplest is to just break up the logic. For previewing (passing in --feed
), you don't need to GetAllFeeds
at all, as you're only really concerned about seeing which feeds you'll get from the preview, not from what's in the store.
Example change that seems to fix both issues at first glance:
var errorItems []ErrorItem
// if no feeds in store or we have preview feeds, fetchAllFeeds
if len(c.config.PreviewFeeds) > 0 {
its, errorItems, err = c.fetchAllFeeds()
if err != nil {
return fmt.Errorf("[commands.go] TUI: %w", err)
}
} else if len(its) == 0 {
_, errorItems, err = c.fetchAllFeeds()
if err != nil {
return fmt.Errorf("[commands.go] TUI: %w", err)
}
// refetch for consistent data across calls
its, err = c.GetAllFeeds()
if err != nil {
return fmt.Errorf("[commands.go] TUI: %w", err)
}
}
I'll hold off on a PR until I get some feedback from @guyfedwards, as I'm not sure of the implications of this change by itself, if any.
@kanielrkirby It works for me!
Only thing to ensure here is taht we aren't saving previewFeeds to the db, they should be cleaned up by cleanDB
when nom
is loaded again without previewFeeds
but worth checking that is working as expected, otherwise just fetch and keep in memory for the preview.
I have considered removing the previewFeeds feature as wasn't really sure of it's benefit over just adding an entry in the yaml and removing it, might be worth discussing but seems like there are some users using it at least.
I think I like preview, as it provides a little flexibility, "one-shot" checking. But maybe it would good to separate that logic completely from the current logic for the store. Just a thought I'm having.
With that said, I think it's working as expected, as fetchAllFeeds
never saves previews to the DB (and checks against it), and GetAllFeeds
explicitly pulls from the store.
from https://github.com/guyfedwards/nom/issues/42