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