Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align video and music sort types on file listings to each other #8184

Merged
merged 1 commit into from Oct 9, 2015

Conversation

razzeee
Copy link
Member

@razzeee razzeee commented Oct 5, 2015

This removes one redundant search type from video file lists and aligns the music file lists to the ones in video.

As @Hitcher asked for this

@Montellese or @mkortstiege for review probably

@razzeee razzeee added the Type: Cleanup non-breaking change which removes non-working or unmaintained functionality label Oct 5, 2015
@Montellese
Copy link
Member

No idea if anyone is using SortByFile. I'm assuming that the difference is that SortByLabel sorts by the determined label (e.g. the movie title) and SortByFile sorts by the actual filename (which might contain additional stuff/information)?

Apart from that it looks good.

@razzeee
Copy link
Member Author

razzeee commented Oct 8, 2015

Well it might. Haven't thought of that yet.
But I don't think that the file library has a customizable label? So we end up with the filename anyway.
Will need testing.

@Montellese
Copy link
Member

When I browse into the video files node I get the actual movie name for all movies that have been added to the library (with artwork and all the fancy stuff) and the folder/filename for everything that hasn't been added to the library. So there certainly can be differences between SortByLabel and SortByFile.

@Hitcher
Copy link
Contributor

Hitcher commented Oct 8, 2015

That happens when you enable the video library setting - 'Replace file names with library titles'. So are we going to have the same for music?

@razzeee
Copy link
Member Author

razzeee commented Oct 8, 2015

I'm not sure if I like the implementation then, should what is in label really depend on what's the sort type? If yes, why is this the only place we use it like this.

@Montellese
Copy link
Member

I never said that the label changes when you change the sort method. The label always stays the same but when using SortByLabel it uses the visible label (which might be an actual movie title or the filename depending on whether the item is in the library or not) and when using SortByFile it uses the filename independent of whether the item is in the library or not. So IMO SortByFile still has a valid use case even though the listing might look a bit odd with the replaced labels.
I've never used this myself but it certainly is a different behaviour than SortByLabel.

@razzeee
Copy link
Member Author

razzeee commented Oct 9, 2015

@Montellese
How about this?
Just for sense of completeness, pictures should be fine as they are. They have the same as video and music now have + 1 for DateTaken

@Montellese
Copy link
Member

Yup, fine with me. Thanks for checking pictures.

razzeee added a commit that referenced this pull request Oct 9, 2015
Align video and music sort types on file listings to each other
@razzeee razzeee merged commit 71e6002 into xbmc:master Oct 9, 2015
@razzeee razzeee deleted the align-file-sort-types branch October 9, 2015 14:19
@DaveTBlake
Copy link
Member

@razzeee from your recent work in this area, could you tell me what changes can best be made to CGUIViewStateWindowMusicNav creator that will reinstate the music file view sorting and display functionality as we have in Isengard but seem to have inadvertently lost in Jarvis?

Since #8011 we stopped using GUIWindowMusicSongs.cpp, and this was providing sorting (I think) that
CGUIViewStateWindowMusicNav doesn't. Note music files can be sorted in ways that video files can't because we have artist, song name, album etc. data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v16 Jarvis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants