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

url: Fix py2 compatibility in trim_url() #1517

Merged
merged 1 commit into from
Mar 24, 2019
Merged

url: Fix py2 compatibility in trim_url() #1517

merged 1 commit into from
Mar 24, 2019

Conversation

dgw
Copy link
Member

@dgw dgw commented Mar 24, 2019

Python 2 doesn't like thing is unicode; it wants thing == unicode. Of course, it just works with regular str in both py2 and py3… Oh well.

This is another find from @Exirel's overhaul of URL handling for 7.x.

As long as I was touching the line anyway, I added parentheses to make the operator precedence explicit. Likely unnecessary, but probably easier to read by some small measure. (@Exirel, if you don't think the added parens are useful, I can drop it from this PR so you don't have to mirror it in #1510. 😸)

Python 2 doesn't like `thing is unicode`; it wants `thing == unicode`.
Of course, it just works with regular str in both py2 and py3… Oh well.

This is another find from @Exirel's overhaul of URL handling for 7.x.

As long as I was touching the line anyway, I added parentheses to make
the operator precedence explicit. Likely unnecessary, but probably
easier to read by some small measure.
@dgw dgw added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Mar 24, 2019
@dgw dgw added this to the 6.6.5 milestone Mar 24, 2019
@dgw dgw requested a review from Exirel March 24, 2019 05:08
@dgw
Copy link
Member Author

dgw commented Mar 24, 2019

Fortunately, find_urls was never actually told to clean what it found (#1515), so this bug was only dormant in Sopel <=6.6.4 and shouldn't have affected anything.

Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

LGTM

(also 💢 @ Python 2)

@dgw dgw merged commit 5cdc607 into 6.6.x Mar 24, 2019
@dgw dgw deleted the dgw/py2-trim_url-fix branch March 24, 2019 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants