Code review comment for lp:~songofacandy/bzr/fix-523746-dev

>> 1. osutils.get_user_encoding() is not good in rare case.
>> user_encoding is not equals to filesystem encoding.
>
>My assumption was that subprocess.Popen should receive its arguments as if
>they were coming from the command line: in user encoding.

OK, I don't know about when get_user_encoding() returns encoding that is not
suitable for commandline arguments.

>> 2. path.encode(enc) can raise UnicodeEncodeError.
>> So bzr should use "encode(enc, 'replace')"
>
>That will mean some path can't be encoded in user encoding which in turn means
>it could not be user as a parameter to bzr no ?

No. bzr uses unicode arguments in Windows. So users can pass unicode filename to bzr.
And "bzr diff --using=winmergeu.exe" doesn't requires file path but shows diff on
such a file.

>> 3. _write_file() writes file with unicode path.
>> But if bzr can't encode the path collectly, external command can't
>> open the file.
>
> We rely on python to do the right thing from unicode paths there.

On windows, unicode path is too perfect.
If unicode path contains chars that is not be able to encode,
we can't use the path in encoded string.
But we can create a file with such a name when using unicode.
So we should replace such a char before _write_file().