search

fossfreedom / coverart-browser

Browse your cover-art albums in Rhythmbox v2.96 - 3.0+
http://xpressubuntu.wordpress.com/
GNU General Public License v3.0
74 stars 19 forks source link

Bug or enhancement? - Use Natural/Human sort for sorts #124

Closed fossfreedom closed 11 years ago

fossfreedom commented 11 years ago

from #122

"-> It seems that for your algorithm, "100 album 1" is before "17 album 2" since "0" is smaller than "7". It is not looking at "100" against "17". This is not so important, it is just that Rhythmbox is doing the contrary."

Need to investigate further what sort type rhythmbox is using for sorting strings such as the album / album artist sort functions.

If its a human/natural sort then this Q&A will help:

Do we classify this as a bug to fix in the main branch - or leave this to the refactor branch - or move this as a potential enhancement in a future milestone.

EDIT: Will have a look in a separate branch - I share the concern that this may severely impact performance

fossfreedom commented 11 years ago

@asermax - ok - building on the previous sort bug/enhancement - I've added a new "NaturalString" object that replaces the calculated album string previously created.

How does this fair with your collection?

N.B. branch "naturalsort"

asermax commented 11 years ago

Well, first, it seems like you mixed some code from the refactoring branch into this new branch. You should revert back that last commit, since it'll break master and it will bring a lot of problems when merging refactoring back into master. EDIT: nevermind, I did a bad diff xD

Second, what's the difference with the search_fold approach? Is it better in some aspect or is it the same? I'm not against the change, but I don't know why is that you changed it; using the rhythmbox method seemed fine to me.

asermax commented 11 years ago

Ok, I'm starting to see the slight difference on the sorting of albums with numbers on it d: that part seems to be working alright. No delays neither, maybe it takes a little longer to initialise, but it's not noticeable. On the other side, the new objects breaks the search functionality (since it isn't a string anymore). Also, the call to search_fold could be moved inside the constructor of the new object.

fossfreedom commented 11 years ago

Agree with you - this is a subtle change - I'm surprised I had not noticed before, especially since I've got lots of albums starting with numbers.

Anyway - my bad - you are correct about the search. My intention was for NaturalString to be a direct drop-in replacement for a string - I forgot to actually inherit from str - doh!

Anyway, please double check that the revised changes hasnt inadvertently caused a noticeable performance drop.

asermax commented 11 years ago

Seems to be working alright now :+1:.