Make a good faith effort to display a bytestring when one is provided… #1438
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey @Bachmann1234 thanks for the PR! 😄 |
| @@ -1115,7 +1115,10 @@ def _idval(val, argname, idx, idfn): | |||
| elif isinstance(val, (float, int, str, bool, NoneType)): | |||
| return str(val) | |||
| elif isinstance(val, REGEX_TYPE): | |||
| return val.pattern | |||
| if isinstance(val.pattern, str): | |||
There was a problem hiding this comment.
While this works, I think a better approach here would be to reuse the effort made by _escape_bytes to handle bytes as ascii-escaped values instead of trying to decode from utf-8:
elif isinstance(val, REGEX_TYPE):
return _escape_bytes(val.pattern) if isinstance(val.pattern, bytes) else val.pattern
|
Aside from my comments, please also add a test in Thanks again for putting work into this! 😄 |
|
Will do! Thanks for the quick feedback! |
…code to turn it into a string before further processing. Thanks @nicoddemus for the review and tips!
|
Should be fixed up and squashed down. Let me know if there is anything else needed. |
RonnyPfannschmidt
added a commit
that referenced
this pull request
Mar 6, 2016
Make a good faith effort to display a bytestring when one is provided…
|
Well done thanks for putting in the work 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a PR, your contribution is really appreciated!
Here's a quick checklist that should be present in PRs:
[ X] Target: for bug or doc fixes, target
master; for new features, targetfeatures[ X] Make sure to include one or more tests for your change
[ X] Add yourself to
AUTHORS[ X] Add a new entry to the
CHANGELOG(choose any open position to avoid merge conflicts with other PRs)… as a regex patterm
Attempting to fix #1437
This will display odd behavior when the text in the bytestring is not encoded in ascii or utf-8. It wont crash but it will be wrong.
However, as I dont see a way to know the encoding for sure this seemed reasonable
Open to other ideas. Saw this on twitter and thought I would take a stab at it