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

urlencode is applied to Issue/PR title #68

Open
boesing opened this issue Aug 6, 2021 · 4 comments
Open

urlencode is applied to Issue/PR title #68

boesing opened this issue Aug 6, 2021 · 4 comments

Comments

@boesing
Copy link

boesing commented Aug 6, 2021

Hey there,

I've released v2.0.0 of laminas/laminas-httphandlerrunner and realized, that urlencode is applied to the title of a PR and Issue.

Actually, the titles are used as a descriptive link-text and thus, should not be encoded because they're not part of the URL itself.

17: feature: Add additional headers_sent informations to the EmitterException [...]

I've had a quick view over the source code but could not directly find the corresponding lines. Will have a deeper dive somewhen in the future and will create an bugfix if not done by any other consumer already.

@boesing boesing changed the title urlencode is applied on Issue/PR title urlencode is applied to Issue/PR title Aug 6, 2021
@boesing
Copy link
Author

boesing commented Aug 6, 2021

Okay, I've found the corresponding lines:

$title = str_replace(['[', ']', '_'], ['[', ']', '_'], $title);

Is there any specific reason why the title needs to get replaced like this?

@WyriHaximus
Copy link
Collaborator

Seems to be introduced in #29 to resolve #27

@boesing
Copy link
Author

boesing commented Aug 6, 2021

I guess this was made to avoid having markdown conflicts.
Maybe a more complex regular expression should be applied first while replacing the corresponding matches rather than the whole title?
What do you think?

@WyriHaximus
Copy link
Collaborator

@boesing That should do the trick, and probably also use a dataprovider for that changed tests and toss several scenarios at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants