[boost] Re: [Filesystem] i18n

From: Beman Dawes (bdawes_at_[hidden])
Date: 2005-03-30 15:16:07

"Martin" <adrianm_at_[hidden]> wrote in message 
news:loom.20050330T110611-38_at_post.gmane.org...
> 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'll do that after then next round of updates.

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

Thanks. The #ifdef situation has improved since the last release in that 
there are fewer #ifdefs covering bigger blocks of code, and in some cases 
overloading has replaced #ifdefs. The only other thing I can think of would 
be to have separate POSIX and Windows implementation files. But that brings 
up code duplication issues. Changes to allow the code to work with broken 
compilers is also likely to introduce more #ifdefs. It's messy, that's for 
sure.

> - 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.)

I thought of that, but decided the symplification from removing the name 
checking from the basic_path class was more important.

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

That isn't correct. See lpath.hpp and wide_test.cpp in the test directory 
for an example of using a basic_string<long> as the the underlying string 
type. The docs need more work to make clearer exactly what is required of 
the String argument to the basic_path template.

A programmer wishing to basic_path with something other than std::string or 
std::wstring has to do more work, but it isn't particularly difficult, as 
the lpath example shows.

> - 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'll give that some thought.

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

If the is_* functions return false, rather than throwing, then testing for 
the negative of a is_* function becomes unreliable. Does !is_directory( 
"foo" ) mean that "foo" is a file, or is is just missing? The programmer 
would have to write exists("foo") && is_accessible("foo") && 
!is_directory("foo"). That could be simplified by providing an is_file() 
function (as suggested by Jeff Garland recently), but then we have to define 
exactly what a file is. Is a socket a file? Is a device a file? Or what 
POSIX calls a "regular file"? How does that translate to Windows?

I'm not saying  that approach would be a bad thing. In fact, I was just 
working out examples of how it might work this morning. But it is a 
considerable change. Those changes would have to be worked out in detail so 
we could evaluate them compared to the status quo.

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

While that is an interesting example, note that a user provided function 
would work instead.

> 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).

Interesting. That would cover my example about of wanting to detect a file. 
The expressions would be written !is_directory("foo",true). But that seems a 
bit obscure to me, and likely to be a cause of coding errors. Hum... it 
isn't even correct in the case a directory exists but is not accessible An 
alternative would be named functions. These would be the so called 
"compound" functions which were discussed at one time, but that gets really 
messy. Where do you stop?

> - 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").

There was discussion of a glob directory iterator at one time, but no one 
ever came up with a final design. It is certainly a real need.

> - Why doesn't last_write_time return a boost::ptime

That was a concious decision to limit coupling. That is, to avoid Boost 
libraries which are not part of TR1. I'd like the Boost version to be 
identical to what gets proposed to the standards committee, and unless Boost 
Date-Time gets proposed and accepted, that means it can't be used.

Thanks for your comments. Because of the upcoming standards committee 
meeting, I'll be pretty distracted for the next three weeks, but will try to 
give some more thought at odd moments to the issues you've raised.

--Beman