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

Fix Markup object repr in debug console #1393

Merged
merged 2 commits into from Nov 16, 2018

Conversation

Projects
None yet
3 participants
@ChrisDryden
Copy link
Contributor

commented Nov 12, 2018

The console was assuming that all objects of string type would only return a string in repr(). This meant that objects that gave response of its object type along with a string in repr were breaking parts of the code that removed the parenthesis. By verifying that an object is a string type instead of a string instance this bug can be fixed.

Fixes #1129

@ChrisDryden

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2018

This fixes issue #1129

@davidism davidism requested a review from ThiefMaster Nov 12, 2018

@ThiefMaster
Copy link
Member

left a comment

LGTM once you remove that trailing whitespace

Show resolved Hide resolved werkzeug/debug/repr.py Outdated
@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Nov 12, 2018

Also, please amend the commit message to be more concise - first line shouldn't be more than 51 chars, and certainly so long that github truncates it. My suggestion would be something like "Fix Markup object repr in debugger"

@davidism davidism changed the title Added check to verify that the string_type Fix Markup object repr in debug console Nov 15, 2018

@davidism davidism force-pushed the ChrisDryden:1129-markupsafe branch from 174b0d1 to f06309c Nov 15, 2018

@davidism davidism force-pushed the ChrisDryden:1129-markupsafe branch from f06309c to c2d6bfa Nov 15, 2018

@davidism

This comment has been minimized.

Copy link
Member

commented Nov 15, 2018

I'd like to experiment a little more before merging this. This fix works, but now it ignores subclasses of str that still have the standard repr.

@davidism

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

Demo of some alternatives in #1397

addressed

@davidism davidism force-pushed the ChrisDryden:1129-markupsafe branch from c3b7f17 to baffc49 Nov 16, 2018

@davidism

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

I ended up committing a different fix that still allows subclasses. Thanks for working on this during the sprints, just digging in and figuring out where things were happening and why was really helpful.

New version collapses the repr rather than slicing the original object, then only adds subclass info if the repr looks like the standard string repr.

@davidism davidism added this to the 0.15 milestone Nov 16, 2018

@davidism davidism force-pushed the ChrisDryden:1129-markupsafe branch from baffc49 to bb57713 Nov 16, 2018

@davidism davidism merged commit 96d8b23 into pallets:master Nov 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@davidism davidism added the debugger label Jan 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.