Review Request 110672: Allow multiple selection in archiveview

[Alim Gokkaya]

> On Aug. 17, 2014, 1:30 a.m., Raphael Kubo da Costa wrote:
> > part/part.cpp, line 209
> > <https://git.reviewboard.kde.org/r/110672/diff/2/?file=305694#file305694line209>
> >
> >     You can replace this loop with something like this:
> >     
> >         const int lastPathDelimiterPos = rootStr.lastIndexOf(QLatin1Char('/'), rootstr_len);
> >         rootStr.truncate(lastPathDelimiterPos);
> >     
> >     (I'm assuming there will be at least a "/" in the beginning of all nodes, otherwise you have to account for that)

I've noticed that there isn't usually a preceeding "/". So I'd to handle it after rootStr.lastIndexOf(..).


On Aug. 17, 2014, 1:30 a.m., Alim Gokkaya wrote:
> > Thanks for the patch; I still need to take a deeper look at it (and check some assumptions such as whether all nodes always have a common '/' parent or not), so I just left one inline comment so far.
> > 
> > In any case, this is the sort of code I'd expect to find in the model-view part itself, not here in part.cpp (doing this would also make it easier to test). And please adjust the coding style: Ark follows kdelibs', which means ending each sentence in a period, using `fooBar` instead of `foo_bar` in variable and function names, putting a space between `if` and `(`, not needlessly abbreviating variable names etc.

I've carried the code to the ArchiveModel class, I hope it a better place. For the qtest, I had to change part/CMakeLists.txt to generate library executables because I couldn't find a proper way to write qtest to work with independent modules.


- Alim


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


On Aug. 17, 2014, 5:46 p.m., Alim Gokkaya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/110672/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2014, 5:46 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/test/archivemodeltest.cpp PRE-CREATION 
>   part/test/CMakeLists.txt PRE-CREATION 
>   part/CMakeLists.txt 9e38443 
>   part/archivemodel.h 7f8c527 
>   part/archivemodel.cpp 4326268 
>   part/archiveview.cpp 6b9918d 
>   part/part.cpp b4ebcd2 
> 
> 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/20140817/6bd13ca5/attachment.html>