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

[Vaclav Slavik]

Hi,

On Sat, 2010-05-15 at 11:49 +0000, Greg Chicares wrote:
> >  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?

In practical terms, it's just a requirement to be able to compile with
VC++ (because of its wide string extensions on top of the standard).

But it's also good hygiene with wx3, where wxString::c_str() is
ambiguous w.r.t. wide vs. narrow strings. When you use mb_str() or
wc_str() explicitly you make it clear whether you need to get narrow or
wide string from wxString. Because you work with narrow-string std:: API
here, you should arguably use mb_str() explicitly. 

> characters, so deliberately introducing them via mb_str() where
> not strictly necessary doesn't seem best.

Using mb_str() doesn't introduce any new issues. "Multibyte" is the same
as "encoding into char* string" (aka "narrow string"). In wx-2.8 ANSI
build that you use, c_str() is strictly equivalent to mb_str().

In wx3, it is more complicated because we attempt to be compatible with
both 2.8's ANSI and Unicode modes, but the essence is the same: LMI
limits itself to ASCII strings and uses char* or std::string all over
place, so all wxString::c_str() data are eventually converted into char*
-- and so are still mostly equivalent to using mb_str() directly.

Put another way, in LMI's case, c_str() is implicit conversion to char*
while mb_str() is explicit.

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

Because the part where the errors are reported:

        warning() << "Unable to save '" << filename << "'." <<
        LMI_FLUSH;
        
still creates char* ("multibyte") error message and with your locale
being C, non-ASCII 'filename' cannot be represented as such. I'm not
sure about this code's behavior with wx2.9 off-hand, but with 2.8, you
would have to use wide strings to get usable error message:

        warning() << L"Unable to save '" << filename.wc_str() << L"'." << 
LMI_FLUSH;
        
And of course, warning_alert() would have to take std::wstring too.

>  - MinGW gcc, IIRC, doesn't support wide strings, at least not for
>    gcc-3.x (which is what we still use for production). 

I'm not aware of such limitations. Looking at Poedit VCS history, its
version 1.3.5 required Unicode build of wx and was compiled with MinGW
3.4.

> Can we make the diagnostics say something more than
>   Unable to save ''.

Yes, by either using wide strings for warnings, or by formatting
filenames specially before passing them to operator<< (e.g. by doing a
custom conversion that replaces unconvertible characters with '?').

Vaclav