Skip to content

fix weird boto docstrings #656

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.

Sign up for GitHub

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

Merged
sebastinas merged 1 commit into from
Nov 19, 2016
Merged

fix weird boto docstrings #656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions bpython/curtsiesfrontend/replpainter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def matches_lines(rows, columns, matches, current, config, format):
if m != current
else highlight_color(
m.ljust(max_match_width))
for m in matches[i:i+words_wide])
for m in matches[i:i + words_wide])
for i in range(0, len(matches), words_wide)]

logger.debug('match: %r' % current)
Expand Down Expand Up @@ -161,6 +161,12 @@ def formatted_argspec(funcprops, arg_pos, columns, config):


def formatted_docstring(docstring, columns, config):
if isinstance(docstring, bytes):
docstring = docstring.decode('utf8')
elif isinstance(docstring, str if py3 else unicode):
pass
else:
return []
Copy link
Contributor

@sebastinas sebastinas Nov 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Is the elif and else really necessary? Or in other words: does the elif really cover all valid cases?

Copy link
Member Author

@thomasballinger thomasballinger Nov 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

The cases to cover:

Py2 bytes -> decode
Py2 unicode -> nop
Py2 something else (integer etc) -> abort

Py3 bytes -> shouldn't happen, but decode
Py3 bytes -> nop
Py3 something else -> abort

Might be nicer to:

if unicode:
    pass
else:
    try:
        docstring = docstring.decode

Copy link
Member Author

@thomasballinger thomasballinger Nov 18, 2016

Choose a reason for hiding this comment

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

To answer your question, docstrings should always be unicode in python 3, and in Python 2 they should always be bytestrings. (since we're getting them from pydoc.getdoc, which does this normalization) If we got a unicode string somehow in Python 2 that would be ok, but I don't know how that would happen. If we got a bytestring in Python3, which shouldn't happen, we would try to decode. So this does cover all valid cases, but it covers some extra too.

Now that I see where docstring comes from (pydoc.getdoc) I agree that the else isn't necessary.

The correct thing to do here is to find out the encoding of the source file the docstring comes from, since it doesn't have to be utf8, or at least catch errors here so a bad docstring doesn't crash bpython.

color = func_for_letter(config.color_scheme['comment'])
return sum(([color(x) for x in (display_linize(line, columns) if line else
fmtstr(''))]
Expand Down Expand Up @@ -211,7 +217,7 @@ def paint_last_events(rows, columns, names, config):
return fsarray([])
width = min(max(len(name) for name in names), columns - 2)
output_lines = []
output_lines.append(config.left_top_corner+config.top_border * width +
output_lines.append(config.left_top_corner + config.top_border * width +
config.right_top_corner)
for name in reversed(names[max(0, len(names) - (rows - 2)):]):
output_lines.append(config.left_border + name[:width].center(width) +
Expand Down
40 changes: 39 additions & 1 deletion bpython/test/test_curtsies_painting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# coding: utf8
from __future__ import unicode_literals
import itertools
import string
import os
import pydoc
import string
import sys
from contextlib import contextmanager

Expand Down Expand Up @@ -136,6 +137,43 @@ def test_formatted_docstring(self):
'Also has side effects'])
self.assertFSArraysEqualIgnoringFormatting(actual, expected)

def test_unicode_docstrings(self):
"A bit of a special case in Python 2"
# issue 653

def foo():
u"åß∂ƒ"

actual = replpainter.formatted_docstring(
foo.__doc__, 40, config=setup_config())
expected = fsarray([u'åß∂ƒ'])
self.assertFSArraysEqualIgnoringFormatting(actual, expected)

def test_nonsense_docstrings(self):
for docstring in [123, {}, [], ]:
try:
replpainter.formatted_docstring(
docstring, 40, config=setup_config())
except Exception:
self.fail('bad docstring caused crash: {!r}'.format(docstring))

def test_weird_boto_docstrings(self):
# Boto does something like this.
# botocore: botocore/docs/docstring.py
class WeirdDocstring(str):
# a mighty hack. See botocore/docs/docstring.py
def expandtabs(self, tabsize=8):
return u'asdfåß∂ƒ'.expandtabs(tabsize)

def foo():
pass

foo.__doc__ = WeirdDocstring()
wd = pydoc.getdoc(foo)
actual = replpainter.formatted_docstring(wd, 40, config=setup_config())
expected = fsarray([u'asdfåß∂ƒ'])
self.assertFSArraysEqualIgnoringFormatting(actual, expected)

def test_paint_lasts_events(self):
actual = replpainter.paint_last_events(4, 100, ['a', 'b', 'c'],
config=setup_config())
Expand Down