> On Dec. 23, 2010, 7:21 a.m., Oz Linden wrote: > > indra/llvfs/lldiriterator.cpp, lines 152-155 > > <http://codereview.secondlife.com/r/32/diff/2/?file=129#file129line152> > > > > This method of tracking escapes and brackets isn't quite correct, I > > think... consider translating a string input containing an escaped > > backslash or an escaped bracket. > >
Escaping characters like backslash or a square bracket seems to work as expected. For example the pattern "file\\\\[0-9].abc" used as a mask argument for iterator constructor will match files: "file\1.abc", "file\2.abc", etc. The bracket can be escaped by a preceding double backslash "\\[". > On Dec. 23, 2010, 7:21 a.m., Oz Linden wrote: > > indra/llvfs/lldiriterator.cpp, line 99 > > <http://codereview.secondlife.com/r/32/diff/2/?file=129#file129line99> > > > > There should be a unit test for this translation. All unit tests for directory iterator depend on the result of this translation. Looks like most common cases are covered. Does it need some additional test cases? what kind of cases should be added? > On Dec. 23, 2010, 7:21 a.m., Oz Linden wrote: > > indra/llvfs/lldiriterator.cpp, lines 67-80 > > <http://codereview.secondlife.com/r/32/diff/2/?file=129#file129line67> > > > > It would be better to make the boost::regex a member variable - rather > > than translate the mask to a regex on every call to next, translate it once > > in the constructor. Updated in diff r3. > On Dec. 23, 2010, 7:21 a.m., Oz Linden wrote: > > indra/llvfs/lldiriterator.h, lines 55-56 > > <http://codereview.secondlife.com/r/32/diff/2/?file=128#file128line55> > > > > Why not use '^' for the negation qualifier here? It's more familiar > > (at least to unix users) '!' is defined in pattern matching rules on glob(7) man page, but it could be easily replaced with '^' if it is needed. - Seth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/32/#review80 ----------------------------------------------------------- On Jan. 4, 2011, 12:38 p.m., Seth ProductEngine wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/32/ > ----------------------------------------------------------- > > (Updated Jan. 4, 2011, 12:38 p.m.) > > > Review request for Viewer. > > > Summary > ------- > > Fixed LLDir unit test which failed for some complex wildcard combinations. > Added a class implementing directory entries iteration with pattern matching > which is used in unit tests instead of LLDir::getNextFileInDir. > > This code has been run on Linux only. It should be tested under other > platforms and more test cases should be provided. For example changing > directory contents while iterating through it. > > > This addresses bug STORM-550. > http://jira.secondlife.com/browse/STORM-550 > > > Diffs > ----- > > indra/cmake/Boost.cmake 27dae7b01a81 > indra/llvfs/CMakeLists.txt 27dae7b01a81 > indra/llvfs/lldiriterator.h PRE-CREATION > indra/llvfs/lldiriterator.cpp PRE-CREATION > indra/llvfs/tests/lldir_test.cpp 27dae7b01a81 > > Diff: http://codereview.secondlife.com/r/32/diff > > > Testing > ------- > > > Thanks, > > Seth > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
