Review Request 109369: Bug #254404: Copy files to USB storage devices in display order by sorting tracks in CollectionLocation.cpp

[Konrad Zemek]

> On Aug. 25, 2013, 8:49 a.m., Mark Kretschmann wrote:
> > What's the current status of this patch?
> 
> Matěj Laitl wrote:
>     Well, the question is whether we want such big complication of code for little gain. I fear that not, at not least in the current for of the patch. I'd be also against adding another duplicate of "level sort" to our code. Do I remember correctly that Konrad offered to make the "level sort" from playlist more generic (i.e. using Meta::val*) and reusable? That could help in shrinking this patch.

@Matej, way to hold me to my words. ;-) Sure, I'll give sorting a spin now that I know the codebase better, and then we can revisit the current issue.


- Konrad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109369/#review38520
-----------------------------------------------------------


On March 12, 2013, 6:27 p.m., Anmol Ahuja wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109369/
> -----------------------------------------------------------
> 
> (Updated March 12, 2013, 6:27 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> 1. Made a levelSort() function to sort tracks according to multiple parameters ( Should i just be sorting tracks belonging to the same album? )
> 2. Modified copyUrlToCollection() to use QList instead of QMap
> 3. FileView's files are being copied in the order of selection, thanks to using QLists ( is that acceptable? )
> 
> Updates:
> Implemented changes as suggested by strohel
> 1. Moved levelSort function to the prepareCopy callers (CollectionTreeView)
> 2. Restored CollectionLocation.cpp to the old version, with QLists instead of QMaps
> 
> Still not complete though, working on it.
> 
> 
> Diffs
> -----
> 
>   src/browsers/CollectionTreeView.h 3b2ca80 
>   src/browsers/CollectionTreeView.cpp fd9fe66 
>   src/browsers/collectionbrowser/CollectionWidget.h c281f41 
>   src/core-impl/collections/audiocd/AudioCdCollectionLocation.cpp be13551 
>   src/core-impl/collections/db/sql/SqlCollectionLocation.h 0bcf244 
>   src/core-impl/collections/db/sql/SqlCollectionLocation.cpp 93efe97 
>   src/core-impl/collections/ipodcollection/IpodCollectionLocation.h cc27e19 
>   src/core-impl/collections/ipodcollection/IpodCollectionLocation.cpp f8105f9 
>   src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.h 3c2d9f2 
>   src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp 8a40c6c 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceCollectionLocation.h e40529f 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceCollectionLocation.cpp f60aff6 
>   src/core-impl/collections/mediadevicecollection/handler/MediaDeviceHandler.h 821f1b0 
>   src/core-impl/collections/mediadevicecollection/handler/MediaDeviceHandler.cpp c1b76f5 
>   src/core-impl/collections/mtpcollection/handler/MtpHandler.cpp a8d9f52 
>   src/core-impl/collections/support/PlaylistCollectionLocation.h 10a365f 
>   src/core-impl/collections/support/PlaylistCollectionLocation.cpp c885046 
>   src/core-impl/collections/support/TrashCollectionLocation.h 239a977 
>   src/core-impl/collections/support/TrashCollectionLocation.cpp 61c2e49 
>   src/core-impl/collections/umscollection/UmsCollection.cpp 6bebd98 
>   src/core-impl/collections/umscollection/UmsCollectionLocation.h 45ba596 
>   src/core-impl/collections/umscollection/UmsCollectionLocation.cpp e0ba0ac 
>   src/core/collections/CollectionLocation.h d37ccfb 
>   src/core/collections/CollectionLocation.cpp aecc068 
>   src/services/ServiceCollectionLocation.cpp d1cb0d8 
>   src/services/mp3tunes/Mp3tunesServiceCollectionLocation.h 2b06cb4 
>   src/services/mp3tunes/Mp3tunesServiceCollectionLocation.cpp aa61072 
> 
> Diff: http://git.reviewboard.kde.org/r/109369/diff/
> 
> 
> Testing
> -------
> 
> Seems to be copying tracks in the correct order now
> 
> 
> Thanks,
> 
> Anmol Ahuja
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130825/10032b6d/attachment.html>