fix: always use parameter UTF-8 fallback#1468
Conversation
|
This seems ok, but I can't tell what it might affect. Can you add a test case? |
Even when the filesystem encoding and sys.argv encoding are the same we should still fall back to trying UTF-8 for decoding command line arguments.
cfe9117 to
f6710e5
Compare
|
I've made the test case commit come first. That way it's easier to verify that it fails without the fix commit. For reference, the monkeypatched values I inserted are those that Python 2 and 3 respectively set when starting with all of the |
|
Just leaving a note for myself in case I need to revisit this later. It seems like Thanks for working on this! |
In pallets/click#1468 , string parameter types (the default) added a catchall case in which a value which fails to decode is run though `decode('utf-8', 'replace')`. This succeeds even on things like latin-1 chars which can't decode into utf-8 . As a result, our test case which relied on the value being passed through verbatim doesn't work anymore. However, the case which uses our custom param type (the mkdir test) still fails as expected. We may still be able to and want to test our failure modes around invalid UTF8 bytes like this, but for the purposes of being click 7.1 compatible, just remove the failing test.
Even when the filesystem encoding and sys.argv encoding are the same we
should still fall back to trying UTF-8 for decoding command line
arguments.