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

Fix/remove-webrick-dependency #108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kreintjes
Copy link

No description provided.

Comment on lines +346 to +347
# Note: this RegEx is obtained by executing WEBrick::HTTPUtils::UNESCAPED,
# splitting it over multiple lines and using the x modifier to make this possible.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would be better to either copy over the smaller pieces of this Regex, as WEBrick does it, or at least make that a perma-link to the version from which it was taken (i.e., https://github.com/ruby/webrick/blob/9350944141a3f15acda9c79edd5393289c098e04/lib/webrick/httputils.rb#L498). Or… both!?!

Copy link
Author

Choose a reason for hiding this comment

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

I did not copy the smaller pieces of the regex but used the complete result instead on purpose. We do not need the smaller piecers. We do not use them by ourselves. It may improve understandability of where this is coming from, but to use the smaller piecer, we would also need to copy the custom _make_regex method and logic of what the UNESCAPED constant is (UNESCAPED = _make_regex(control+space+delims+unwise+nonascii)). This would result in duplicating more code.

Instead I think it is better to just execute WEBrick::HTTPUtils::UNESCAPED and copying the result as I did here.

Good suggestion about the permalink though. Fixed that :)

Was only used to URL escape a string (also see interagent@585de87).
Seems a bit overkill to include a complete webserver only for this. We therefore only include this method ourselves.

Note: also tried CGI.escape and URI.encode_www_form_component but those don't work since they escape too much (e.g. colons).
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.

2 participants