Comments (5)
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.
from castero.
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.
from castero.
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).
from castero.
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.
from castero.
Forgot to close this; properly implemented as of d06eed7.
from castero.
Related Issues (20)
- New feature: status/progress bar HOT 2
- Do not clear queue when manually playing next episode HOT 4
- adding "shut up a second" podcast crashes castero HOT 1
- crashes after press the h key HOT 2
- Handle current episode url with script to enable copy to clipboard HOT 2
- Freeze upon wake from suspend HOT 4
- Request: make default layout configurable HOT 1
- cannot change selection color
- New release request (with width fix) HOT 1
- Support speed increase over 2.0x HOT 1
- Client crashing after adding some feeds
- Episode marked as undone after refreshing the feed
- Custom (per-podcast) start position [Feature Request]
- Option to change metadata display format
- Feature Request: autoplay queue HOT 2
- FeedParseError: Unable to parse text as xml documents HOT 1
- TypeError: argument of type 'NoneType' is not iterable HOT 3
- Future request: ALSA channels volume control
- [Feature Request]: Add MPRIS Support For Linux Users HOT 1
- Using cron to update the feeds periodically
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from castero.