> On June 21, 2011, 6:56 a.m., Aleric Inglewood wrote: > > indra/llvfs/lldiriterator.cpp, line 123 > > <http://codereview.secondlife.com/r/313/diff/1/?file=2823#file2823line123> > > > > What is your reasoning to use native() here and not string()? > > Boroondas Gupte wrote: > I tried to stick to a 1:1 replacement. The old > boost::filesystem::path::filename() has return type string_type, and > boost::filesystem::path::native(), too, so that seemed like the natural > replacement to me. As we save the result in a std::string, my reasoning might > not make too much sense, though. > > Aleric Inglewood wrote: > No that makes no sense at all, since > boost::filesystem::path::native::string() also returns a std::string. > So, still the same question: why mIter->path().filename().native() and > not mIter->path().filename().string()? > > > Aleric Inglewood wrote: > I meant boost::filesystem::path::string()
The reason I've given above is the only one I had. If that one doesn't make sense, no reason is left for preferring mIter->path().filename().native() over mIter->path().filename().string(), AFAIK. However, I also see no reason to prefer mIter->path().filename().string() over mIter->path().filename().native(). >From the documentation, it seems they should give the exactly same data: the >path's (here: the filename's) content as std::string with OS-native >directory-separator characters (which shouldn't occur in a path that's just a >filename). What could make sense is to use >mIter->path().filename().generic_string(), so that the (effectless for pure >filenames) '/' --> '\' conversion doesn't have to be executed on Windows. - Boroondas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/313/#review793 ----------------------------------------------------------- On May 25, 2011, 1:25 p.m., Boroondas Gupte wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/313/ > ----------------------------------------------------------- > > (Updated May 25, 2011, 1:25 p.m.) > > > Review request for Viewer. > > > Summary > ------- > > Context: We are currently using Boost 1.45, which already comes with the new > Boost Filesystem Library API (Version 3) but still defaults to the old one > (Version 2). From Boost 1.46 on, V3 will be the default and Boost 1.47 will > be the last one to come with V2. The Boost Filesystem Library documentation > recommends "Existing code should be moved to Version 3 as soon as convenient. > New code should be written for Version 3. Version 2 is deprecated, and will > not be included in Boost releases 1.48 and later." > > This change overrides the default, so that the V3 API is used, and makes the > necessary code changes. (So we can stick to Boost 1.45 and upgrade whenever > we feel like it.) > > Note: I only changed stuff that the compiler complained about. If the new API > also changes semantic of still-compiling library usage, more changes might be > necessary. > > > This addresses bug OPEN-67. > http://jira.secondlife.com/browse/OPEN-67 > > > Diffs > ----- > > doc/contributions.txt 959f9340da92 > indra/llvfs/lldiriterator.cpp 959f9340da92 > > Diff: http://codereview.secondlife.com/r/313/diff > > > Testing > ------- > > * Compiled Viewer (standalone) with Boost 1.45 > * Started Viewer > * Logged in > > * Compiled Viewer (standalone) with Boost 1.46 > * Started Viewer > * Logged in > > Not tested: > * non-standalone > > > Thanks, > > Boroondas > >
_______________________________________________ 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
