-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Rule: Fix URI encoding of strings #7009
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.
Can you add a test in Test_tableLinkForExpression
please? but to be honest: i dont understand it; the URL seems valid on its own ( not in a html context ); is this maybe an issue of whatever renders the notification in the end?
Ah i understand now; spaces are encoded as |
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, can you squash commits please?
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. Can we update changelog?
Signed-off-by: Kartikay <[email protected]>
202711c
to
31850ab
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.
LGTM
Changes
Should fix #7002
Added how the URI should treat
+
as it is ignored byurl.QueryEscape
Reference