Review Request 125117: Moved check for all mount points excluded out of widget class

[Ovidiu-Florin BOGDAN]

> On Sept. 9, 2015, 9:11 p.m., Martin Klapetek wrote:
> > kcms/baloo/kcm.cpp, line 167
> > <https://git.reviewboard.kde.org/r/125117/diff/1/?file=402363#file402363line167>
> >
> >     const QStorageInfo &si

If I do that I get this:

/home/ovidiu/kde-devel/src/kde/workspace/plasma-desktop/kcms/baloo/kcm.cpp:167:5: error: invalid initialization of reference of type ‘QStorageInfo&’ from expression of type ‘const QStorageInfo’


> On Sept. 9, 2015, 9:11 p.m., Martin Klapetek wrote:
> > kcms/baloo/kcm.cpp, lines 168-169
> > <https://git.reviewboard.kde.org/r/125117/diff/1/?file=402363#file402363line168>
> >
> >     You can do it directly -- mountPoints << si.displayName()

I didn't do it directly because: it's easyer to debug if you keep the value in a variable before adding it to the list. But you are right, you can check the list's elements.


> On Sept. 9, 2015, 9:11 p.m., Martin Klapetek wrote:
> > kcms/baloo/kcm.cpp, lines 174-177
> > <https://git.reviewboard.kde.org/r/125117/diff/1/?file=402363#file402363line174>
> >
> >     This can also stay like it was, just do
> >     
> >     return excluded.toSet() == mountPoints.toSet();
> >     
> >     ...it will return the result of that comparison.
> >     
> >     Or even 
> >     
> >     return m_excludeFolders_FSW->excludeFolders().toSet() == mountPoints.toSet();
> >     
> >     to simplify it completely.

That does not simplify the code. Your last example performs too many operations in the same line. It's debugging difficulty increases with that.
My proposal is more obvious at what it does (especially for new readers).

Also the compiler optimises that.


- Ovidiu-Florin


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


On Sept. 9, 2015, 9:04 p.m., Ovidiu-Florin BOGDAN wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125117/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 9:04 p.m.)
> 
> 
> Review request for Baloo, Plasma and Vishesh Handa.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> And simplified the way it searches for the mount points.
> 
> 
> Diffs
> -----
> 
>   kcms/baloo/folderselectionwidget.h 6430b60 
>   kcms/baloo/folderselectionwidget.cpp 3ad1764 
>   kcms/baloo/kcm.h 6878e89 
>   kcms/baloo/kcm.cpp d85f615 
> 
> Diff: https://git.reviewboard.kde.org/r/125117/diff/
> 
> 
> Testing
> -------
> 
> Compiled and used.
> 
> 
> Thanks,
> 
> Ovidiu-Florin BOGDAN
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150909/05a46585/attachment-0001.html>