Review Request 119607: Support for ".hidden" files

[Bruno Nova]

> On Dez. 6, 2014, 11:03 a.m., David Faure wrote:
> > src/core/kcoredirlister.cpp, line 51
> > <https://git.reviewboard.kde.org/r/119607/diff/5/?file=331915#file331915line51>
> >
> >     split on 2 lines, each with their own comment (more modular and precise)

Done.


> On Dez. 6, 2014, 11:03 a.m., David Faure wrote:
> > src/core/kcoredirlister.cpp, line 1811
> > <https://git.reviewboard.kde.org/r/119607/diff/5/?file=331915#file331915line1811>
> >
> >     (as per latter comment, this would then have to be done with the QFileInfo API)
> >     
> >     Also, did you test the case where the first item is "."? localPath might point to the dir then, so this would look into the *parent* dir, one level too high.

OK, I changed this part. Now I'm using `QFileInfo::canonicalPath()` to get the parent dir.

In both `slotEntries()` and `slotUpdateResult()`, this code should be executed on the first item that isn't "." or ".." (according to the `if`s and `else`s and `continue`s).
Nevertheless, I put this code in this method inside an `else` to make things clearer.


> On Dez. 6, 2014, 11:03 a.m., David Faure wrote:
> > src/core/kcoredirlister.cpp, line 2816
> > <https://git.reviewboard.kde.org/r/119607/diff/5/?file=331915#file331915line2816>
> >
> >     this method assumes the url is local, so why not just take a local path as input?

Done.


> On Dez. 6, 2014, 11:03 a.m., David Faure wrote:
> > src/core/kcoredirlister.cpp, line 2824
> > <https://git.reviewboard.kde.org/r/119607/diff/5/?file=331915#file331915line2824>
> >
> >     triple lookup in the hash
> >      (contains, [] , [])
> >     
> >     Use a const_iterator instead.

OK, fixed this!
Though I didn't use a `const_iterator`. I call `QCache::object()` before the `if`, and then use the returned pointer.
I hope this (and the rest of the patch) doesn't have problems with concurrent access.


> On Dez. 6, 2014, 11:03 a.m., David Faure wrote:
> > src/core/kcoredirlister.cpp, line 2836
> > <https://git.reviewboard.kde.org/r/119607/diff/5/?file=331915#file331915line2836>
> >
> >     hmm shouldn't this be mtime (of the file) rather than currentDateTime? Not sure I can make a case where it would make a difference though. OK, maybe ignore this. Well it would save a system call :-)

You're right, it should be `mtime`! Fixed.


> On Dez. 6, 2014, 11:03 a.m., David Faure wrote:
> > src/core/kfileitem.h, line 263
> > <https://git.reviewboard.kde.org/r/119607/diff/5/?file=331917#file331917line263>
> >
> >     yes, make private, and friend the caller

Done.


- Bruno


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


On Dez. 9, 2014, 2:14 p.m., Bruno Nova wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119607/
> -----------------------------------------------------------
> 
> (Updated Dez. 9, 2014, 2:14 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 64740 and 246260
>     https://bugs.kde.org/show_bug.cgi?id=64740
>     https://bugs.kde.org/show_bug.cgi?id=246260
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> This adds support for *.hidden* files to KDE.
> 
> When listing a directory, the files/folders listed in the *.hidden* file will be hidden, unless the user has chosen to show hidden files.
> 
> This patch was initially based on the patch provided by Mark in Bug #246260.
> 
> 
> Diffs
> -----
> 
>   src/core/kcoredirlister.cpp a31d629 
>   src/core/kcoredirlister_p.h dce7dbc 
>   src/core/kfileitem.h bebc241 
>   src/core/kfileitem.cpp 74dc069 
> 
> Diff: https://git.reviewboard.kde.org/r/119607/diff/
> 
> 
> Testing
> -------
> 
> Built and tested the patch in Project Neon.
> Dolphin was still using KDE4/Qt4 version of the library, so I only tested using the desktop folder widget, and "folder desktop".
> It worked correctly when I pointed it to "~" and "~/Desktop" (I added ".hidden" there).
> However, it didn't work when I pointed it to the "Desktop folder" (the default option, not the custom location "~/Desktop").
> More testing is required.
> 
> The version for KDE4/Qt4 submitted to Bug #246260 was tested in Kubuntu 14.04, and it worked everywhere I tested (Dolphin, open/save dialogs, folder widget and detailed/tree view in Dolphin).
> It wasn't an intensive test, though.
> 
> 
> Thanks,
> 
> Bruno Nova
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141209/ad5fc0dd/attachment-0001.html>