-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: url regex in style_render
does not pass colon and other valid
#46457
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm but for 2 things:
1: needs a "whats new" release rotes (from the docs folder) expplaining the changes.
2: the second test should include all the new charcaters so that future changes to the regex do not remove them
@@ -780,6 +780,8 @@ def test_hiding_index_columns_multiindex_trimming(): | |||
("ftp scheme: ftp://www.web", True, "ftp://www.web"), | |||
("subdirectories: www.web.com/directory", True, "www.web.com/directory"), | |||
("Multiple domains: www.1.2.3.4", True, "www.1.2.3.4"), | |||
("with port: http://web.com:80", True, "http://web.com:80"), | |||
("with anchor: http://web.com/anc#hor", True, "http://web.com/anc#hor"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add all the special characters to this line so that they are tested simultaneously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I made the tests more exhaustive and also tweaked the regex a little bit more.
URLs containing some valid characters such as colon in port numbers get cut off when html-formatting. As a workaround, expanded the regex to match a wider variety of URLs.
d688715
to
ba9cc9a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kianelbo for the PR.
minor change request to give the release note some context.
Co-authored-by: Simon Hawkins <[email protected]>
Thanks @kianelbo |
@meeseeksdev backport 1.4.x |
…ot pass colon and other valid
Something went wrong ... Please have a look at my logs. |
…lon and other valid (#46497) Co-authored-by: Kian Eliasi <[email protected]>
…andas-dev#46457) * BUG: url regex in `style_render` does not pass colon and other valid URLs containing some valid characters such as colon in port numbers get cut off when html-formatting. As a workaround, expanded the regex to match a wider variety of URLs. * Add whatsnew entry for pandas-dev#46389 fix * Update whatsnew entry for fix pandas-dev#46389 Co-authored-by: Simon Hawkins <[email protected]> Co-authored-by: Simon Hawkins <[email protected]>
URLs containing some valid characters such as colon in port numbers get
cut off when html-formatting. As a workaround, expanded the regex to
match a wider variety of URLs.
doc/source/whatsnew/v1.4.2.rst
file if fixing a bug or adding a new feature.