Review Request 119607: Support for ".hidden" files

Bruno Nova brunomb.nova at gmail.com
Thu Dec 4 22:01:05 UTC 2014 


> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote:
> > src/core/kcoredirlister.h, line 420
> > <https://git.reviewboard.kde.org/r/119607/diff/2/?file=301214#file301214line420>
> >
> >     I think that you should not make this part of the public API. AFAICS, you're not using it anywhere outside KCoreDirLister, right?
> 
> Bruno Nova wrote:
>     I left this method (and others) public because I thought that, maybe in the future, someone may want to use this method.
>     But you're right, it's not used anywhere else and should probably be private/protected.

I moved this method to KCoreDirListerCache, making it private.


> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote:
> > src/core/kcoredirlister.cpp, line 2799
> > <https://git.reviewboard.kde.org/r/119607/diff/2/?file=301215#file301215line2799>
> >
> >     This will only work for local files. I'm not sure if we would want to support the ".hidden" mechanism also for other protocols, such as sftp or fish?
> >     
> >     Moreover, I think that we might want to cache the contents of the .hidden file, to prevent that we read it, parse it and construct the QSet with the hidden files over and over again if a kioslave reports the UDSEntries in multiple batches.
> 
> David Faure wrote:
>     Supporting .hidden for remote protocols would be much more complex (async API).
>     
>     Caching: it would have to include an mtime, to be able to detect changes. But indeed caching the last one read would be good, because while having a directory open, any change triggers an update job, which will re-read the .hidden file. Or while at it, could be a LRU cache, Qt makes it simple:
>     
>         struct CacheHiddenFile {
>             QDateTime mtime;
>             QSet<QString> listedFiles;
>         }
>         QCache<QString /*dot hidden file*/, CacheHiddenFile> m_cacheHiddenFiles;
>     
>         m_cacheHiddenFiles.setMaxCost(10);
>         
>     This requires making the filesInDotHiddenForDir method not file-static as I previously suggested, but rather a method of KCoreDirLister::Private, which would also have the above member.
> 
> Bruno Nova wrote:
>     Now that you mention remote protocols, I just found out that Nautilus ".hidden" also doesn't support them.
>     For example, ".hidden" is not interpreted when I access a computer through SFTP in Nautilus. However, when I mount it externally using `sshfs`, it works.
>     Haven't checked Thunar.
>     
>     As for the caching, I'll try to take a look at it, but I'll probably need help (I have very little experience with KDE and Qt).
> 
> Bruno Nova wrote:
>     Just found a case where the *.hidden* doesn't work.
>     If a *.hidden* file is in *~/Desktop*, it works correctly when accessed from that path, but it doesn't work when accessed from *desktop:/*.
>     Is *desktop:/* interpreted as a network path, and `dir.toLocalFile()` fails in that path?
>     I tested in Project Neon in Virtualbox using Kwrite's open dialog, plus LD_LIBRARY_PATH to use the patched library.
> 
> Bruno Nova wrote:
>     David, for the caching, where should the struct and filesInDotHiddenForDir be declared? KCoreDirLister::Private or KCoreDirListerCache?
>     filesInDotHiddenForDir is called in two methods of KCoreDirListerCache.
>     
>     Also, you forgot the semi-colon after the struct declaration ;) (and, as usual, the C++ compiler error message was not very informative).
> 
> David Faure wrote:
>     desktop:/ is almost like a remote protocol, except that we can resolve such urls to local files, using KFileItem::localPath(). So using this would be a way to make it work for desktop:/ urls as well as file:/ urls.
>     
>     Both classes are private (i.e. not part of the public API), so it doesn't matter. So KCoreDirListerCache looks like the easiest solution.
>     
>     Sorry about the missing semicolon :-)
> 
> Bruno Nova wrote:
>     From my tests, KFileItem::localPath() does not work for "desktop:/" URLs when the KFileItem is constructed from a QUrl, it just returns an empty string. Worse, when the URL is "file:///" (root folder), localPath() also returns an empty string (strange). And I've also seen it fail randomly in other folders.
>     
>     It would probably work if the KFileItem was constructed/retrieved from a UDSEntry, but how can I do that in slotEntries() and slotUpdateResult() before iterating over the UDSEntryList?

I uploaded a new patch that caches the ".hidden" file, as advised.
Please check if it's correct.


- Bruno


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


On Dez. 4, 2014, 9:55 p.m., Bruno Nova wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119607/
> -----------------------------------------------------------
> 
> (Updated Dez. 4, 2014, 9:55 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/20141204/b90e95dd/attachment-0001.html>

More information about the Kde-frameworks-devel mailing list