Improve URI.register_scheme tests and automatically upcase the given scheme

ruby / uri

URI is a module providing classes to handle Uniform Resource Identifiers

https://ruby.github.io/uri/

Other

79 stars 42 forks source link

Improve URI.register_scheme tests and automatically upcase the given scheme #28

Closed eregon closed 2 years ago

eregon commented 3 years ago

Also add docs and mention current limitations.
For reference, https://stackoverflow.com/a/3641782/388803 mentions the valid characters in schemes.

Follow-up of https://github.com/ruby/uri/pull/27#discussion_r677858406 cc @byroot / @casperisfine and @hsbt

Maybe +/-/. could be supported by replacing them with some other characters valid as constant names, but not sure how to avoid conflicts (replacing all 3 with just _ seems bad), and whether someone would actually want a custom URI::Generic subclass for a scheme. Also any extra replacement to .upcase will make URI.for slower for non-initial schemes.

byroot commented 3 years ago

Maybe +/-/. could be supported by replacing them with some other characters valid as constant names, but not sure how to avoid conflicts

Yeah, I almost went with some form of gsub('-', '__DASH__') etc, but I wasn't sure the overhead was worth it.

eregon commented 3 years ago

Indeed, and we can always expand in the future if someone requests it, while we can't remove it if we add it now. The previous "API" using @@schemes wasn't really a proper public API so I think it's OK to have additional restrictions for URI.register_scheme.

byroot commented 3 years ago

using @@schemes wasn't really a proper public API so I think it's OK to have additional restrictions for URI.register_scheme.

Agreed. It was pretty much a monkey patch method before, so if people have really complex need they can extend .for.

nurse commented 2 years ago

@eregon Sorry about the late review but it looks good. Once you solve the conflicts, I'll merge this