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

Make rewrite_urls more precise with regard to backslash removal and support "new" subdomain. #487

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mikupls
Copy link
Contributor

@mikupls mikupls commented Apr 16, 2022

Adjust function description and add another test-case.


Addresses #475 and relates to #281.

So we have tests for both observed breakages now. However, now that I understand a bit more what is going on here, this solution is also far from perfect. It will start breaking again once someone intentionally puts \_ into a comment AFAICT.

The intention of the function is probably to limit the scope of the regex replacement to <a></a> elements. However operating with regexes on HTML is known to be tricky (or "impossible"). I propose we live with this intermediary solution until someone finds another relevant breakage.

@mikupls
Copy link
Contributor Author

mikupls commented Apr 17, 2022

Added another commit to support "new.reddit.com"; discovered on r/japan/comments/skbmu8/the_rjapan_basic_questions_thread_february_2022/.

@mikupls mikupls changed the title Make rewrite_urls more precise with regard to backslash removal. Make rewrite_urls more precise with regard to backslash removal and support "new" subdomain. Apr 17, 2022
@mikupls
Copy link
Contributor Author

mikupls commented May 24, 2022

Anything else that is needed here?

@Daniel-Valentine
Copy link
Member

Sorry this has been on the backburner. I'm going through outstanding issues and PRs and intend to get to this one soon. Bleiben Sie dran.

@mikupls
Copy link
Contributor Author

mikupls commented Mar 2, 2023

Thanks. Looks like this patch still applies cleanly. Don't expect me to be able to explain the regexes after a year though :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants