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

Revision history for this message
methane (songofacandy) wrote :

> Are the mangled filenames going to be in the output of the external tool? If
> so there needs to be some unmangling code.

These filenames are just an input of the external diff tool.

> Can relpath be a string like u"docs/マニュアル/index.html"? If so we risk losing
> path components, should use 'replace' for encoding errors and then do
> .replace("?", "_") to make it windows-happy.

You're right! Adding "_" prefix is not enough.

> Could use `sys.getfilesystemencoding() or 'ascii'` rather than just 'ascii' as
> the encoding. Likewise could use `(path) or 'FILE'` rather than `'_'+(path)`
> to avoid the empty string.

I've made it.
Please review again.

« Back to merge proposal