Skip to content

[2.7] bpo-24307: Fix unicode error in optparse with unicode value for %default#9911

Closed
tirkarthi wants to merge 3 commits intopython:2.7from
tirkarthi:bpo24307
Closed

[2.7] bpo-24307: Fix unicode error in optparse with unicode value for %default#9911
tirkarthi wants to merge 3 commits intopython:2.7from
tirkarthi:bpo24307

Conversation

@tirkarthi
Copy link
Copy Markdown
Member

@tirkarthi tirkarthi commented Oct 16, 2018

optparse allows %default in the help string where a default value can be supplied. '%default' is replaced with the given value. If the given value is a unicode value than it fails with UnicodeEncodeError in Python 2.7. This is not a problem in Python 3 since all strings are unicode by default. This patch encodes the value before replacement and adds tests.

It's based on the patch by tanbro-liu in the bug tracker. Since there was no response from tanbro-liu I have converted the patch as a PR with test and attribution to tanbro-liu.

https://bugs.python.org/issue24307

@tirkarthi
Copy link
Copy Markdown
Member Author

Seems, there is some encoding difference while testing this on Windows. Unfortunately, I don't have a Windows machine to debug this :(

0:00:33 [ 56/404/1] test_optparse failed
test test_optparse failed -- Traceback (most recent call last):
  File "C:\projects\cpython\lib\test\test_optparse.py", line 623, in test_unicode_default
    self.assertHelp(self.parser, expected_help)
  File "C:\projects\cpython\lib\test\test_optparse.py", line 193, in assertHelp
    actual_help + '"\n')
AssertionError: help text failure; expected:
"Usage: test [options]
Options:
  -h, --help            show this help message and exit
  -p PROB, --prob=PROB  blow up with probability PROB [default: olé!]
"; got:
"Usage: test [options]
Options:
  -h, --help            show this help message and exit
  -p PROB, --prob=PROB  blow up with probability PROB [default: ol�!]
"

@tirkarthi
Copy link
Copy Markdown
Member Author

I found one more error where if the conflicting option is a unicode string then it throws UnicodeDecodeError.

from optparse import OptionParser

parser = OptionParser()
parser.add_option("-f", u"--file", dest="filename",
                  help="write to FILE. Default value %default",
                  metavar="FILE",
                  default=u"   ")

parser.add_option("-f", u"--   ", dest="filename",
                  help="write to FILE. Default value %default",
                  metavar="FILE",
                  default=u"   ")


(options, args) = parser.parse_args()
./python.exe ../backups/bpo24307.py -f
Traceback (most recent call last):
  File "../backups/bpo24307.py", line 14, in <module>
    default=u"早上好")
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/optparse.py", line 1021, in add_option
    self._check_conflict(option)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/optparse.py", line 996, in _check_conflict
    option)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/optparse.py", line 113, in __init__
    self.option_id = str(option)
UnicodeEncodeError: 'ascii' codec can't encode characters in position 5-7: ordinal not in range(128)

@tirkarthi tirkarthi closed this Oct 16, 2018
@tirkarthi tirkarthi reopened this Oct 16, 2018
@tirkarthi tirkarthi closed this Oct 16, 2018
@tirkarthi tirkarthi reopened this Oct 16, 2018
@tirkarthi
Copy link
Copy Markdown
Member Author

Sorry, I had to close and reopen twice. First Appveyor hanged and Travis failed so I closed and reopened while Appveyor was hanging. So Appveyor marked it as cancelled and didn't run while Travis ran. When Travis completed I closed and reopened PR but it seems Appveyor marked it as cancelled forever and closing and opening doesn't help. Thus it shows build failure. If someone has privilege to re-run the build then it will be helpful since I don't want to close and reopen again to waste CI resources.

Thanks

Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a high risk of breaking applications which currently work as expected. If the default value is a non-ASCII string, unicode() will raise a UnicodeDecodeError.

@bedevere-bot
Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@vstinner vstinner dismissed their stale review October 25, 2018 00:03

I'm not sure that I understood the change, so I remove my vote.

@tirkarthi
Copy link
Copy Markdown
Member Author

Closing the PR as per https://bugs.python.org/issue24307#msg332337

@tirkarthi tirkarthi closed this Dec 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants