Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/ --- (Updated Set. 14, 2014, 3:32 p.m.) Review request for KDE Frameworks and

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > I've added some comments inline. You should wait for David's feedback > > though. > > > > BTW, do other platforms, which support ".hidden" files already, only > > support full file names in that file, or can you add wildcards (such as

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kcoredirlister.cpp, line 1218 > > > > > > const What do you mean? `filesToHide` should be `const`? (I thought I had added this comment before

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/ --- (Updated Set. 14, 2014, 7:32 p.m.) Review request for KDE Frameworks and

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kcoredirlister.cpp, line 2801 > > > > > > I think that you can remove this line. > > Bruno Nova wrote: > Oops, forgot to remove `QString`

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/#review66777 --- Thanks for the patch. A few comments: Is there any sort of sp

Re: Review Request 119607: Support for ".hidden" files

> On Sept. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kcoredirlister.cpp, line 2799 > > > > > > This will only work for local files. I'm not sure if we would want to > > support the ".hidden" me

Re: Review Request 119607: Support for ".hidden" files

> On Sept. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > I've added some comments inline. You should wait for David's feedback > > though. > > > > BTW, do other platforms, which support ".hidden" files already, only > > support full file names in that file, or can you add wildcards (such a

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/ --- (Updated Set. 18, 2014, 10:06 a.m.) Review request for KDE Frameworks and

Re: Review Request 119607: Support for ".hidden" files

> On Set. 17, 2014, 7:59 p.m., David Faure wrote: > > Thanks for the patch. A few comments: > > > > Is there any sort of spec or reference implementation for ".hidden" files? > > Are they supposed to support wildcards, for instance? > > > > Something else, it would be good to add a unittest to

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kcoredirlister.cpp, line 1218 > > > > > > const > > Bruno Nova wrote: > What do you mean? `filesToHide` should be `const`? > (I thoug

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/#review66872 --- Wait? I was _really_ expecting the .hidden file to support pat

Re: Review Request 119607: Support for ".hidden" files

> On Set. 18, 2014, 9:08 p.m., Christoph Feck wrote: > > Wait? I was _really_ expecting the .hidden file to support patterns. If the > > current specification does not allow it, what is the actual use case then? > > Wouldn't it make sense to discuss adding patterns? > > > > Directory reading i

Re: Review Request 119607: Support for ".hidden" files

> On Sept. 18, 2014, 9:08 p.m., Christoph Feck wrote: > > Wait? I was _really_ expecting the .hidden file to support patterns. If the > > current specification does not allow it, what is the actual use case then? > > Wouldn't it make sense to discuss adding patterns? > > > > Directory reading

Re: Review Request 119607: Support for ".hidden" files

> On Set. 17, 2014, 7:59 p.m., David Faure wrote: > > Thanks for the patch. A few comments: > > > > Is there any sort of spec or reference implementation for ".hidden" files? > > Are they supposed to support wildcards, for instance? > > > > Something else, it would be good to add a unittest to

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kfileitem.h, line 262 > > > > > > Since we probably do not want to make it possible that all users of > > this class can make items "hidden",

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kcoredirlister.cpp, line 2799 > > > > > > This will only work for local files. I'm not sure if we would want to > > support the ".hidden" mec

Re: Review Request 119607: Support for ".hidden" files

> On Sept. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kcoredirlister.cpp, line 2799 > > > > > > This will only work for local files. I'm not sure if we would want to > > support the ".hidden" me

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kcoredirlister.cpp, line 2799 > > > > > > This will only work for local files. I'm not sure if we would want to > > support the ".hidden" mec

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/#review68653 --- The last patch says "I have not tested this yet" - has it been

Re: Review Request 119607: Support for ".hidden" files

> On Out. 18, 2014, 10:02 a.m., David Faure wrote: > > The last patch says "I have not tested this yet" - has it been tested > > meanwhile? Let's not let this rot completely :-) Yes, I have tested it already. It works, but the issues were not all fixed yet. And, as discussed above, the patch d

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/#review70102 --- src/core/kcoredirlister.cpp

Re: Review Request 119607: Support for ".hidden" files

> On Nov. 8, 2014, 10:02 p.m., David Faure wrote: > > src/core/kcoredirlister.cpp, line 1241 > > > > > > After this line, try adding > > > > if (!url.isLocalFile()) { > > const QString l

Re: Review Request 119607: Support for ".hidden" files

> On Nov. 8, 2014, 10:02 p.m., David Faure wrote: > > src/core/kcoredirlister.cpp, line 1241 > > > > > > After this line, try adding > > > > if (!url.isLocalFile()) { > > const QString l

Re: Review Request 119607: Support for ".hidden" files

> On Nov. 8, 2014, 10:02 p.m., David Faure wrote: > > src/core/kcoredirlister.cpp, line 1241 > > > > > > After this line, try adding > > > > if (!url.isLocalFile()) { > > const QString l

Re: Review Request 119607: Support for ".hidden" files

> On Nov. 8, 2014, 10:02 p.m., David Faure wrote: > > src/core/kcoredirlister.cpp, line 1241 > > > > > > After this line, try adding > > > > if (!url.isLocalFile()) { > > const QString l

Re: Review Request 119607: Support for ".hidden" files

> On Nov. 8, 2014, 10:02 p.m., David Faure wrote: > > src/core/kcoredirlister.cpp, line 1241 > > > > > > After this line, try adding > > > > if (!url.isLocalFile()) { > > const QString l

Re: Review Request 119607: Support for ".hidden" files

--- 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 D

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kcoredirlister.h, line 420 > > > > > > I think that you should not make this part of the public API. AFAICS, > > you're not using it anywhere

Re: Review Request 119607: Support for ".hidden" files

> On Nov. 8, 2014, 10:02 p.m., David Faure wrote: > > src/core/kcoredirlister.cpp, line 1241 > > > > > > After this line, try adding > > > > if (!url.isLocalFile()) { > > const QString l

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kfileitem.h, line 262 > > > > > > Since we probably do not want to make it possible that all users of > > this class can make items "hidden",

Re: Review Request 119607: Support for ".hidden" files

> On Sept. 14, 2014, 3:27 nachm., Frank Reininghaus wrote: > > src/core/kfileitem.h, line 262 > > > > > > Since we probably do not want to make it possible that all users of > > this class can make items "hidden

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kfileitem.h, line 262 > > > > > > Since we probably do not want to make it possible that all users of > > this class can make items "hidden",

Re: Review Request 119607: Support for ".hidden" files

> On Sept. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kfileitem.h, line 262 > > > > > > Since we probably do not want to make it possible that all users of > > this class can make items "hidden",

Re: Review Request 119607: Support for ".hidden" files

> On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote: > > src/core/kfileitem.h, line 262 > > > > > > Since we probably do not want to make it possible that all users of > > this class can make items "hidden",

Re: Review Request 119607: Support for ".hidden" files

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

Re: Review Request 119607: Support for ".hidden" files

--- 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 D

Re: Review Request 119607: Support for ".hidden" files

> On Dez. 6, 2014, 11:03 a.m., David Faure wrote: > > src/core/kcoredirlister.cpp, line 51 > > > > > > split on 2 lines, each with their own comment (more modular and precise) Done. > On Dez. 6, 2014, 11:03 a.m

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/ --- (Updated Dez. 11, 2014, 3:23 p.m.) Review request for KDE Frameworks and

Re: Review Request 119607: Support for ".hidden" files

> On Set. 17, 2014, 7:59 p.m., David Faure wrote: > > Thanks for the patch. A few comments: > > > > Is there any sort of spec or reference implementation for ".hidden" files? > > Are they supposed to support wildcards, for instance? > > > > Something else, it would be good to add a unittest to

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/ --- (Updated Jan. 1, 2015, 9:55 p.m.) Status -- This change has been mar

Re: Review Request 119607: Support for ".hidden" files

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/#review72882 --- Ship it! Thanks for the updated patch. I wrote a unittest fo

Re: Review Request 119607: Support for ".hidden" files

> On Jan. 1, 2015, 10:02 p.m., David Faure wrote: > > Thanks for the updated patch. > > > > I wrote a unittest for it, it works, so I pushed both, to wrap this up. > > Feel free to extend the test if you see missing testcases. > > > > I'm not too happy about the sleep(1s) in the unittest, but

Re: Review Request 119607: Support for ".hidden" files

> On Jan. 1, 2015, 10:02 p.m., David Faure wrote: > > Thanks for the updated patch. > > > > I wrote a unittest for it, it works, so I pushed both, to wrap this up. > > Feel free to extend the test if you see missing testcases. > > > > I'm not too happy about the sleep(1s) in the unittest, but

Re: Review Request 119607: Support for ".hidden" files

> On Jan. 1, 2015, 10:02 p.m., David Faure wrote: > > Thanks for the updated patch. > > > > I wrote a unittest for it, it works, so I pushed both, to wrap this up. > > Feel free to extend the test if you see missing testcases. > > > > I'm not too happy about the sleep(1s) in the unittest, but