> 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
