Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEVTOOLS: fix encoding with the dumper companion mac command #3487

Merged
merged 3 commits into from Nov 11, 2021

Conversation

@mistydemeo
Copy link
Contributor

@mistydemeo mistydemeo commented Nov 4, 2021

The mac command in the Python dumper companion tried to handle filename encodings, which is, if I'm being honest, a fool's errand. This led it to error out on certain Japanese filenames, for example:

Resource in ǜǪÇ∑ÇØPPC
Traceback (most recent call last):
  File "/Users/mistydemeo/github/scummvm/devtools/dumper-companion.py", line 488, in <module>
    exit(args.func(args))
  File "/Users/mistydemeo/github/scummvm/devtools/dumper-companion.py", line 404, in collect_forks
    to_file.write(file_to_macbin(file, to_filename.encode("mac_roman")))
  File "/usr/local/Cellar/python@3.9/3.9.1_6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/encodings/mac_roman.py", line 12, in encode
    return codecs.charmap_encode(input,errors,encoding_table)
UnicodeEncodeError: 'charmap' codec can't encode character '\u0327' in position 1: character maps to <undefined>

I've switched the path handling to use pure bytes instead so it can rely on having just the filenames as they are on the filesystem. I've confirmed this lets me use the dumper companion with Japanese Mac discs now. More changes will be needed for punycode encoding to work, since that has to be aware of the underlying string encoding to work.

I've also added a second fix which addresses an overly-aggressive AttributeError catch I added previously, which ended up catching attribute errors not related to the argparse code.

@sev-
Copy link
Member

@sev- sev- commented Nov 4, 2021

Cool stuff! Could you please also add this filename as a test to test_decode_name() (line 537 or so).

Loading

@mistydemeo mistydemeo force-pushed the companion_fix_mac_encoding branch from 94ec049 to ff12b5f Nov 5, 2021
@mistydemeo
Copy link
Contributor Author

@mistydemeo mistydemeo commented Nov 5, 2021

Done!

I'm looking into adding correct-encoding punycoding, but that'll take more work so I'd like to do it as a followup PR.

Loading

@mistydemeo mistydemeo force-pushed the companion_fix_mac_encoding branch from ff12b5f to bd984e2 Nov 5, 2021
@aquadran aquadran merged commit 954e1d6 into scummvm:master Nov 11, 2021
5 of 8 checks passed
Loading
@mistydemeo mistydemeo deleted the companion_fix_mac_encoding branch Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants