Skip to content

Conversation

@osdiab
Copy link
Contributor

@osdiab osdiab commented Oct 16, 2019

Fixes #48
Fixes #24

Not sure how this ought to work with the extractFromQueryString param since I never use that - should it just forward the flag there?

@sindresorhus
Copy link
Owner

The option name makes sense for the regex, but not here, as including www and protocol-less URLs is actually looser. I think it should be called requireScheme and default to true.

@sindresorhus
Copy link
Owner

Not sure how this ought to work with the extractFromQueryString param since I never use that - should it just forward the flag there?

I don't think it should apply there as it will be too ambiguous.

@osdiab
Copy link
Contributor Author

osdiab commented Oct 16, 2019

@sindresorhus edited with comments

@osdiab osdiab changed the title Add strict url parsing flag Add requireScheme flag Oct 16, 2019
@sindresorhus
Copy link
Owner

Hmm, sorry, I missed that it also affects www, so the option should be requireSchemeOrWww to be explicit. Also use the scheme word instead of protocol.

@sindresorhus
Copy link
Owner

Maybe also add a note that when it’s off it will match things like unicorn.education.

@osdiab osdiab changed the title Add requireScheme flag Add requireSchemeOrWww flag Oct 16, 2019
@osdiab
Copy link
Contributor Author

osdiab commented Oct 16, 2019

back to you @sindresorhus

@nestarz
Copy link

nestarz commented Oct 17, 2019

Can we give the user the hand to the list of the TLD ? For example, .onions are not captured using this.

@osdiab
Copy link
Contributor Author

osdiab commented Oct 17, 2019

I actually added an issue to url-regex earlier for that exact request, although for a different reason: kevva/url-regex#65

Would be helpful to advocate your reason for wanting this there!

With that in mind maybe it is better to just do #24 instead for maximum flexibility with less API surface, what do you think @sindresorhus ?

@osdiab
Copy link
Contributor Author

osdiab commented Oct 18, 2019

@nestarz made a PR for that feature, which if passed in some form maybe can end up here too. kevva/url-regex#66

@sindresorhus sindresorhus changed the title Add requireSchemeOrWww flag Add requireSchemeOrWww option Oct 18, 2019
@sindresorhus
Copy link
Owner

With that in mind maybe it is better to just do #24 instead for maximum flexibility with less API surface, what do you think @sindresorhus ?

No, I want to explicitly only expose options that make sense.

@sindresorhus sindresorhus merged commit 5daceb8 into sindresorhus:master Oct 18, 2019
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.

Add option to get URLs without a protocol Support url-regex options

3 participants