Annotate/mark downloaded podcasts in the episode list

[nbastin]

It would be nice if you could see which episodes were available for offline listening without having to read the metadata pane for each episode (particularly since there doesn't appear to be any way to scroll that pane, so sometimes you can't see that information at all).

[nbastin]

I have an implementation that I am using that works, but is a little bit cumbersome, at master...nbastin:20190101-save-status which allows for flexible addition of metadata flags to the episode list.

This is also an example of where it starts to get annoying to pass the config around, and creates a maintainability problem with future changes that might alter the arguments to the base Menu class where we will have to search and find callers that we might not otherwise care about.

[xgi]

I'm in favor of this, and I agree that it should be flexible since in the future there may be other metadata we'd like to display in this fashion.

Looking at your implementation, I'm in favor of making Menu subclasses, but I think I would prefer having the subclass override _draw_item to modify the output (maybe making another method with the generic behavior). That might be bad for performance, though.

Also, I think I'm leaning more in favor of the Config changes, but could you elaborate on the maintainability issues here? If anything, looking back at the Menu class now I'd be inclined to replace its config parameter with a reference to the Display class that created it, through which we could access the config.

[nbastin]

I started out trying to override the _draw_item functionality, as that seemed like the logical place, but the problem there is that the Menu doesn't have the actual objects - they've already been resolved to strings by that point. I would prefer that the Menu has the actual objects (or no objects) and resolves them as late as possible to strings for display, but that was a much bigger change.

Also really I think maybe it needs a "lighter" abstract call than _draw_item for subclasses to override, otherwise they're all going to have to be calling _pad_item_str and such which seems overkill - I'd prefer to leave all the actual layout/alignment/padding in base code that is solid. Maybe there could simply be a call that gets the "intended display string" for a row, and the return value of that is still passed through whatever necessary grist mill exists to make it actually fit the display. This would allow for resizing the width of the menus without subclasses having to know anything about that.

I would definitely be in favor of some kind of lazier-evaluation of the contents of the display. The fact that the menu holds all the possible items you could ever display, for every feed, causes me some memory issues already (I hadn't tracked those down yet, but identified the problem by accident when making this change).

<long-musing-on-architecture-and-maintainability-and-the-unknowable-future>

As for the maintainability issue with passing things around, this is a little bit of a spidey-senses-tingling thing about land mines I've laid for myself in past experience and it was painful to work out of later. If we simply decided that display owned everything, instead of singletons managed by sys.modules, and then every object that was ever created was also passed (and held) a display object, then that would at least solve the basic problem. What I really don't want is to have to pass an object through a bunch of code I'm not modifying in order to get it into the code that I am, because the last frame that had the data I want is 5 calls back in the stack, which is sort of what happens now. On the other hand, if we put everything in display that means that we're implicitly making a for-all-time architecture decision that things can't be detached from a display object.

Even in the case where that seems mostly ok, I would actually hold out the Config object as still being different. It is currently a leaf object - it can be instantiated and used without Castero doing anything with your display, and I think it should stay that way. Nothing about maintaining a single instance inside of display changes that, but it makes it easier to accidentally end up requiring a display at some point in the future without realizing it. Making leaf singletons maintained by sys.modules means that the moment you break the abstraction you immediately realize it (because the code breaks).

Speaking of leaf objects and musings...clearly this is mostly true of Feed and Episode as well, although of course they are not singletons. They're leaf objects and can be really clean of most anything Castero-UI-implementation-specific. I mention this because it might be nice to keep them clean so that you could say, run a non-display tool that at least ran through all your feeds and downloaded new episodes as a cron job or something. But this goes to the question of what Castero is really intended to do, and what the direction should be. One practical question of this is whether they should really have download methods - should they know how to download themselves? Or should they be passed to a thing that knows how to do downloads, and they only maintain the data that exists locally and the pointers to where to get them?

To give that at least some practical color - I've been pondering replacing some of the threaded downloading with either libcurl or callouts to subprocesses, and it might be nice if the Feed and Episode objects didn't control their downloading, so they could be passed to a configurable choice of downloader. This would allow us to tinker with implementations that were configuration options without affecting the mechanism most users were already using (and also support multiple options so if you have say a 4 core but slow machine, like an N3160, you can farm out to subprocesses instead of threads, but if you have a 2 core but fast machine, like an m5, you can stick with threads if that works better for you).

[xgi]

I can relate to your concerns about the variance of Menu items. If we commit to having separate Menu subclasses for all/most menus, it could be reasonable to have them each store their items in a context-appropriate way and just use the base class for generic "put this string on this line" functionality. However I'm not quite sure how this would affect the hierarchical structure of the menus.

I also really appreciate your write-up re: maintainability; I am leaning towards merging #10 so I'll address some of those arguments on that PR (in a day or two). In regards to downloading architecture I can say that as a general rule I'm open to abstracting offline download logic out of individual data classes (Episode, Feed). In fact I'm not particularly attached to having the client handle this downloading at all; I mostly just wanted to avoid making too many assumptions about the user's system. However I think its totally reasonable to have support for other downloading utilities, and I'd generally be in favor of architecture changes to delegate the task of handling downloads outside of the downloadable objects themselves.

[xgi]

Forgot to close this; properly implemented as of d06eed7.