passing in a feed as a flag requires a manual feed refresh

[guyfedwards]

-f/--feed requires an additional r (otherwise shows items from the subscribed feeds or "No items found")

from #42

[kanielrkirby]

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?

[yonas]

@kanielrkirby That's been my experience.

[kanielrkirby]

Hm, I'll open a separate issue for this, it seems to be a regression.

[kanielrkirby]

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.

[yonas]

@kanielrkirby It works for me!

[guyfedwards]

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.

[kanielrkirby]

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.

[yonas]

@guyfedwards Which PR / commit was this fixed in?

[guyfedwards]

I thought it had been fixed in the above fix but have now seen that that is a fix on your fork. Are you able to submit that fix as a PR? yonasBSD@5b8fbc7