----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/32/#review80 -----------------------------------------------------------
As noted below, it would be more efficient to do the translation from the glob expression just once in the constructor rather than on each call to the next method. The constructor should then throw an exception for an invalid glob expression. indra/llvfs/lldiriterator.h <http://codereview.secondlife.com/r/32/#comment49> Why not use '^' for the negation qualifier here? It's more familiar (at least to unix users) indra/llvfs/lldiriterator.cpp <http://codereview.secondlife.com/r/32/#comment50> 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. indra/llvfs/lldiriterator.cpp <http://codereview.secondlife.com/r/32/#comment51> There should be a unit test for this translation. indra/llvfs/lldiriterator.cpp <http://codereview.secondlife.com/r/32/#comment52> 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. - Oz On 2010-12-20 13:47:20, Seth ProductEngine wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/32/ > ----------------------------------------------------------- > > (Updated 2010-12-20 13:47:20) > > > 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 794ad1fc71d1 > indra/llvfs/CMakeLists.txt 794ad1fc71d1 > indra/llvfs/lldiriterator.h PRE-CREATION > indra/llvfs/lldiriterator.cpp PRE-CREATION > indra/llvfs/tests/lldir_test.cpp 794ad1fc71d1 > > 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
