GNOME Bugzilla – Bug 327372
Number column for browser
Last modified: 2018-05-24 11:08:38 UTC
Might be nice to have a number column in the browser similar to the one used for "All". This is really helpful in at least the case where you right click on the podcast feed and select Update Feed. If you have more episodes than can fit on one screen it is hard to know if anything was added. If the number column incremented it would be easy.
Created attachment 57545 [details] [review] first attempt This seems to work OK for the library but only shows a single entry for each podcast feed. Any ideas?
Created attachment 57554 [details] [review] patch OK, this should do it. I changed the podcast feed browser to query the SUBTITLEs of POSTs instead of querying the LOCATION of FEEDs. It is the same effect but we get the number of each this way. Look OK?
It looks okay, except that the number after "All" has a different meaning then for the other ones. We should probably make All have the number of tracks as well, and put the number of Genres/Artists/Albums in the column title.
Created attachment 57589 [details] [review] updated patch This seems OK to me. The part where it creates the "All %d [artists...]" is a little strange but I don't know another way to work around ngettext limitations. What do you think?
Does this look OK?
Looks okay to me. "All 1 artist" is a bit odd, but I can't think of any better wording.
I was thinking we could hide the 'all' row when there were fewer than two property rows to display. Or maybe it'd still be visible for zero properties, and the text would be "no matching songs". I think it might work better to have the song count column right-justified, and it'd also be useful/interesting to be able to sort on it.
I committed before I saw your comment Jonathan. Those sound like good ideas so I'll keep the bug open.
Right justifying would be nice because occasionally albums and artists have brackets with numbers in the titles, so it can look strange sometimes, e.g.: Album (2) (10) would look better as: Album (2) (10)
I've just noticed an issue with the committed patch: changing the podcast feed browser from operating on feed LOCATIONs to episode SUBTITLEs causes it not to display feeds that don't have episodes (e.g. the user deleted them all). Not having them displayed is odd, because they will still be updated, but there is no way of removing the feed. Fixing it would probably involve reversing most of http://cvs.gnome.org/viewcvs/rhythmbox/sources/rb-podcast-source.c?r1=1.40&r2=1.41. Comments?
Created attachment 58097 [details] [review] patch to revert podcast browse changes Here is the patch to revert the changes to podcast browsing. So I've thought a bit about this. I think this is starting to show some of the limitations of how we're using the browser in this case. We probably shouldn't be using a property view for the feed selection since there will be only one feed per location. It doesn't really make much sense and it is very limiting. On the other side, the way we are using the browser makes it nearly impossible to work with podcasts without the browser. We have no equivalent functionality when the browser is not shown. It is my view that the browser should be entirely optional. Actually, I would prefer that we start to follow the industry trend away from browsing and toward searching. I think iTunes got this right. They made the feed a parent node to child episodes in the tree view. Is this possible? In the meantime, would you like me to apply this revert patch?
Looks okay to me. Having child nodes in a tree view is certainly possible, however RBEntryView contains several things that wouldn't work too well with this. One similar possibility (that I think someone suggested when podcast support was first being done) was to have feeds as child sources of the main podcast source.
It's not possible to arrange podcasts hierarchically like this at the moment, because RhythmDBQueryModel only handles sequences of entries, rather than full tree structures. I don't think it's a great idea to add this directly to RhythmDBQueryModel, because everything else relies on it working, and not much else would benefit from the changes. My initial impression of what would have to be done: - implement a new GtkTreeModel for grouping entries by some property - modify rhythmdb so it calls an arbitrary callback with query results, rather than calling a RhythmDBQueryModel method (not a bad idea in any case) - modify RBEntryView so it doesn't care what sort of tree model it uses. The only RhythmDBQueryModel methods it uses much are the ones to convert between entries, tree paths, and tree iterators, which shouldn't be hard to deal with. - do something about play orders. We don't need play orders for iradio or podcasts, so we should be able to get RBShellPlayer to not use one. Instead, these sources should just play one selected entry. Otherwise, play orders would need to understand the hierarchical tree model thing too, which would probably be painful. So, this would be a fair bit of work. Making individual feeds appear as child nodes under the main podcast source in the source list would be much easier.
Jonathan, you took off the accepted flag for the patch to revert. Was that intentional? If not, I'll go ahead and commit this. Thanks.
No, it wasn't, sorry. There was a mid-air collision when I added my comment.
Patch committed to cvs.
With the recently-committed chane to fold the extra stuff from RBSimpleView back into RBProperty view, it should be fairly easy to add an extra columne for the track count.
This really should be a separate bug (if it's not already), but: (In reply to comment #13) > - modify rhythmdb so it calls an arbitrary callback with query results, rather > than calling a RhythmDBQueryModel method (not a bad idea in any case) Jonathan has done this, and it is now in cvs. > - modify RBEntryView so it doesn't care what sort of tree model it uses. The > only RhythmDBQueryModel methods it uses much are the ones to convert between > entries, tree paths, and tree iterators, which shouldn't be hard to deal with. They are really only utility functions to save copying the code everywhere. They will work with any GtkTreeModel, as long as the first column contains the pointer to the entry. > - implement a new GtkTreeModel for grouping entries by some property Depending on what exactly we want, this could be fairly easy, or really difficult. If every row has a RhythmDBEntry*, things would mostly Just Work. We would probably want header rows that don't have an associated entry, so we would need to make RBEntryView handle rows without an entry (and do the right thing with them). > - do something about play orders. We don't need play orders for iradio or > podcasts, so we should be able to get RBShellPlayer to not use one. Instead, > these sources should just play one selected entry. Otherwise, play orders > would need to understand the hierarchical tree model thing too, which would > probably be painful. This could probably be an extension of the EOF type - only sources that use RB_SOURCE_EOF_NEXT would use play orders. The easiest solution would be to say that any source using that (and hence play orders) must use a non-heirarchical model.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/rhythmbox/issues/123.