Re: [lmi] patch: compilation fix for wx 2.9

[Greg Chicares]

[...and a Happy New Year, as I crawl through long-neglected messages...]

On 2009-12-30 12:08Z, Vadim Zeitlin wrote:
> On Wed, 30 Dec 2009 11:13:49 +0100 Vaclav Slavik <address@hidden> wrote:
> 
> VS> On Wed, 2009-12-30 at 00:43 +0100, Vadim Zeitlin wrote:
> VS> > It fixes compilation of this code with wxWidgets 2.9 as without it an
> VS> > ambiguity arises when passing wxCStrData, which is the proxy object
> VS> > convertible to both "const char*" and "const wchar_t*" returned by
> VS> > wxString::c_str(), to std::ifstream ctor which is overloaded for both
> VS> > narrow and wide string types.
> VS> 
> VS> This is a Microsoftism -- Visual C++ has overloaded ctor, but the
> VS> standard only defines the const char* one (see 27.8.1.5).
> 
>  Oops, thanks for pointing it out, I didn't realize (or forgot?) it at all.
> 
> VS> We had this issue with VC++ before, in other document classes, and
> VS> solved it by using mb_str() (see rev. dcfcd9 in your git repo).
> 
>  Yes, using mb_str() is definitely the right thing to do then and the
> proposed patch becomes

See the patch below[0], which is extended to two additional files.
I think this is what you intend, but I don't think it does what
you expect.

Is this change a prerequisite for other GUI enhancements that depend
on updating wx? Even with gcc?

Or is it just an msvc workaround that we could conditionalize?
As shown below, we seem to have multiple problems with multibyte
characters, so deliberately introducing them via mb_str() where
not strictly necessary doesn't seem best.

> The potential data loss I mentioned in my initial message is still
> possible, of course, but we'd probably need to use wxWidgets classes to
> avoid it and it would be a far less trivial change. It still seems wrong to
> me that not only LMI currently will fail to open a file with non-ASCII
> characters in name but it won't even give any proper error message for it.
> Maybe we should at least correct the latter?

With the patch, I still don't see a helpful diagnostic. I tried:

  File | New | Illustration
  OK
  File | Save as | 新年快乐

and here's the result:

  Unable to save ''.
  [file /lmi/src/lmi/illustration_document.cpp, line 182]

Figuring that maybe Chinese is exceptionally difficult, I tried
  С Новым Годом
and
  Šťastný nový rok
with the same result. But it accepts "Bonne année" and "Prospero Año",
and even the Volapük "Läbukolöd o nulayel" greeting, which also uses
only characters that I guess msw regards as more "western".

I renamed an already-saved file to 'С Новым Годом.ill' and tried to
reopen it; result:

  boost::filesystem::is_directory: "C:\opt\lmi\data\? ????? ?????.ill":
  The filename, directory name, or volume label syntax is incorrect.

so apparently multibyte characters pose problems even outside
the source files included in the patch.

Furthermore, even if we got past those difficulties:

 - MinGW gcc, IIRC, doesn't support wide strings, at least not for
   gcc-3.x (which is what we still use for production). Wait...
   you've deliberately chosen a NTMBS and not a NTWCS, so it's okay:
   it's still a narrow string. However...

 - For printing, we use apache 'fop', which seems to restrict us
   to a subset of the ASCII filenames that msw would accept. See the
   documentation for orthodox_filename() in lmi's 'path_utility.cpp',
   or click here:
     
http://www.tt-solutions.com/vz/lmi/docs/path__utility_8cpp.html#a6a72c37f6e4f2b09366aa92a55d1e23a

 - lmi's problem domain really is quite USA-specific because life
   insurance is governed by national regulation. I doubt it would
   be useful even in Canada. No end user has ever asked us to
   support non-ASCII filenames.

Can we make the diagnostics say something more than
  Unable to save ''.
? If not, then is there a way to constrain filenames to strings
that we can at least print in diagnostics?

---------

[0] "the patch below"

Index: census_document.cpp
===================================================================
--- census_document.cpp (revision 4950)
+++ census_document.cpp (working copy)
@@ -70,7 +70,7 @@
         }
     else
         {
-        std::ifstream ifs(filename.c_str());
+        std::ifstream ifs(filename.mb_str());
         if(!ifs)
             {
             warning()
@@ -117,7 +117,7 @@

 bool CensusDocument::DoSaveDocument(wxString const& filename)
 {
-    std::ofstream ofs(filename.c_str(), ios_out_trunc_binary());
+    std::ofstream ofs(filename.mb_str(), ios_out_trunc_binary());
     doc_.write(ofs);
     if(!ofs)
         {
Index: illustration_document.cpp
===================================================================
--- illustration_document.cpp   (revision 4950)
+++ illustration_document.cpp   (working copy)
@@ -98,7 +98,7 @@
         }
     else
         {
-        std::ifstream ifs(filename.c_str());
+        std::ifstream ifs(filename.mb_str());
         if(!ifs)
             {
             warning()
@@ -175,7 +175,7 @@
         return false;
         }

-    std::ofstream ofs(filename.c_str(), ios_out_trunc_binary());
+    std::ofstream ofs(filename.mb_str(), ios_out_trunc_binary());
     doc_.write(ofs);
     if(!ofs)
         {
Index: mec_document.cpp
===================================================================
--- mec_document.cpp    (revision 4950)
+++ mec_document.cpp    (working copy)
@@ -69,7 +69,7 @@
         }
     else
         {
-        std::ifstream ifs(filename.c_str());
+        std::ifstream ifs(filename.mb_str());
         if(!ifs)
             {
             warning()
@@ -118,7 +118,7 @@

 bool mec_document::DoSaveDocument(wxString const& filename)
 {
-    std::ofstream ofs(filename.c_str(), ios_out_trunc_binary());
+    std::ofstream ofs(filename.mb_str(), ios_out_trunc_binary());
     doc_.write(ofs);
     if(!ofs)
         {