Review Request 110672: Allow multiple selection in archiveview

[Raphael Kubo da Costa]

On Sept. 7, 2014, 12:55 a.m., Alim Gokkaya wrote:
> > Again, thanks a lot for your work on this patch.
> > 
> > I've left some inline comments mostly concerning your unit test, which is still broken (after I fixed it locally all tests passed).
> > 
> > One problem I had when testing the code is that some unnecessary directories are still being created because of the algorithm used in `getSelectionRoot()`. If I take phonon-4.7.2.tar.xz again and drag and drop "phonon-4.7.2/declarative/abstractinitable.h" and "phonon-4.7.2/phonon.pc.cmake", I end up with "declarative/abstractinitable.h" and "phonon.pc.cmake" -- the "declarative/" part should not be there. This is related to bug 208384, which I'm beginning to believe should be addressed together with this one.
> > 
> > This led me to think about other solutions that didn't involve this amount of string manipulation in the first place. It's been a long time since I last had to do any serious ItemView programming with Qt so I'm a bit rusty, but I'm fairly sure we can exploit it some more.
> > 
> > I've started a branch called "multiple-dnd-selection" that attempts to use this approach: instead of processing all selected files as strings, I'm processing the view's `QItemSelectionModel` and grouping all the selected items by their closest unselected parent (which is the root node) and then creating separate `ExtractJob`s for them based on this root node. I think this is the only way to solve bug 208384. Going back to the Phonon example, there would be one `ExtractJob` whose root node is "phonon-4.7.2/declarative" and another one whose root node is "phonon-4.7.2".
> > 
> > This is an investigative work in progress; those jobs need to be properly serialized and only the libarchive backend is working as expected at the moment -- please take a look at my commit there, see if it makes sense and tell me what you think.

Oh and before I forget: if my idea works it won't mean the unit test code will be discarded; I'd like to make sure everything's working, then move the code elsewhere and test it like you do in your patch.


- Raphael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/110672/#review65937
-----------------------------------------------------------


On Sept. 6, 2014, 6:40 p.m., Alim Gokkaya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/110672/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2014, 6:40 p.m.)
> 
> 
> Review request for KDE Utils, Albert Astals Cid and Raphael Kubo da Costa.
> 
> 
> Repository: ark
> 
> 
> Description
> -------
> 
> This patch enables extracting multiple files using drag&drop by removing the code that singles the selection.
> 
> 
> Diffs
> -----
> 
>   part/archiveview.cpp 6b9918d 
>   part/archivemodel.cpp 4326268 
>   part/archivemodel.h 7f8c527 
>   part/CMakeLists.txt 9e38443 
>   part/part.h 5379b9f 
>   part/part.cpp b4ebcd2 
>   part/test/CMakeLists.txt PRE-CREATION 
>   part/test/archivemodeltest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/110672/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alim Gokkaya
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20140906/97412e69/attachment.html>