From: Martin (adrianm_at_[hidden])
Date: 2005-03-30 13:18:34
Didn't find any comments on the new filesystem implementation so here are mine
- Would it be possible to store the new files in the vault? I find it
inconvenient to check out a special branch just to read the documentation.
- I haven't compiled the new files but I have had a good look at the
documentation and the source. My impression is that it is a very nice
implementation. (All the #ifdefs make the source hard to follow but I'm not
the maintainer)
- Specially I like the switch from default portable syntax to default native
syntax. It means that I no longer need to change the default name checker in
each application and that the "native_" prefix from the access functions are
gone.
(Maybe the name checker could be a template parameter (with default "none")
for those who still want to use it. Would complicate implementation of
operations.)
The comments I have on the new implementation is:
- Operations are limited to std::string (and std::wstring on win32) as
external type. Why not allow any range of characters? Current implementation
will not work with const_string, flex_string and a basic_string with a custom
allocator.
- Why not have a static locale member in the class instead of using the utf-8
facet. The locale could then be used both for internal/external conversion via
the codecvt facet and get operator< to work with a custom locale.
(for posix the locale would default to locale::global() with the utf-8 facet
added)
I also had hopes for some new things that didn't appear in the new
implementation.
- non-throwing is_* functions. I find it very inconvenient and error prune to
put try/catch around each is_directory call. Calling is_accessible before
every other is_* function is an option but not much easier and still leaves a
risk for an exception if another process is accessing the file.
The throwing is_* functions also makes it impossible(?) to use the directory
iterator with algorithms like remove_copy_if(directory_iterator(path),
directory_iterator(), back_inserter(filelist), is_directory)
My suggestion is to add non-throwing overloads where a second parameter tells
what the function should return in case of error. e.g. is_directory(path,
true).
- The directory iterator is still very limited since you can't specify a
filter (e.g. "*.txt"). If used on a win32 system with a mounted posix disk or
a posix system with a CD problems might arise. (There is no portable way to
tell if "FILE.TXT" matches "*.txt").
- Why doesn't last_write_time return a boost::ptime