Added ftp urls to be catched in urlize#587
Added ftp urls to be catched in urlize#587choialdrichdy wants to merge 2 commits intopallets:masterfrom
Conversation
|
|
||
| If target is not None, a target attribute will be added to the link. | ||
| """ | ||
| scheme = ['http://','https://','ftp://'] |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Commit messages should use present tense / command style, e.g,. "Add ftp urls ..." instead of "Added ...". Also, you could squash the second commit into the first one - the fact that you fixed the tests in the first commit is not really something useful in the project history in case the PR gets merged. |
|
Would it make sense to have only http/https by default and add an |
|
We recently added a policy configuration. I'm happy to accept a policy for the url schemes supported. I'm not super happy with the urlize filter as it stands right now because it's not super correct. Instead of piling up on it i rather accept a more generalized PR that improves on it (eg: tld handling etc.). |
using #522 as an inspiration, i have added the 'ftp://' url to be accepted in urlize. Also, i have separated both the domain and schemes in a list.