On Tue, Jun 20, 2017 at 6:44 AM, Chris Glover via Boost
Hi,
The following is my review of the proposed Boost.Nowide library.
One thing I'm not sure about is if the overlap between the fstream implementations in filesystem and nowide is necessary. It seems to me that if I set filesystem to be nowide aware (via boost::nowide::nowide_filesystem) then there's little reason for the nowide versions of the stream objects to exist, other than possibly eliminating one string copy on non-windows platforms and avoiding the filesystem dependency (and maybe those are good enough reasons).
The reason nowide implemented fstream is because filesystem::fstream uses internally the default std::basic_filebuf thus relays on std library to provide filebuf::open(wchar_t const *). Also it works OK for MSVC, MinGW libstdc++ does not provide wchar_t interface. For filesytem the fix is far from trivial as requires implementation of filebuf so nowide implemented one.
- What is your evaluation of the documentation?
The documentation is clear and complete, but I think it needs to explain at the top that the library does not present a uniform encoding for strings. This is discussed in the documentation near the end, under "Q & A", but I think it should be near the top. A suggestion is to add a section called "What Boost.Nowide is *not*" directly under the section titled "What is Boost.Nowide", otherwise I expect some users to be surprised.
I think it is very good point.
Under MSYS, one test failed (Fail: Error boost::nowide::cin.putback(c) in test_iostream.cpp:17 main), but I haven't had a chance to look into why yet.
All of the tests passed on the other platforms.
Ok I hope once I have full test suite of Boost suite it will be easier to find the stuff.
Yes. I am already using it and expect I will be using it a lot more going forward.
Thank You very much for the review
-- chris
Artyom