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 linkify example code in README #722

Merged
merged 1 commit into from
Oct 28, 2020

Conversation

pocke
Copy link
Contributor

@pocke pocke commented Oct 20, 2020

This pull request updates an example code of Linkify in README. Because it does not work correctly.

Problem

The example said "disables .py as top level domain" by md.linkify.tlds('.py', false), but it is not correct.

It actually disables all top-level domains, except ..py. So, with the configuration, linkify doesn't convert foo.py to a link, but it also doesn't convert github.com and so on. But it converts foo..py to a link accidentally. 😂

Why it happens

The example misunderstood the behavior of linkify-it's tlds function.
http://markdown-it.github.io/linkify-it/doc/#LinkifyIt.prototype.tlds

The second argument is "keepOld".

merge with current list if true (false by default)

So, if it is false, the function replaces the whole of the current list with ['.py']. It means it only allows ..py domain.

Solutoon

I replaced the wrong example with another example that disables converting emails.
I think disabling email conversion is not a rare setting, so it is appropriate.

By the way, I couldn't find an easy way to disable .py TLD. I guess we need the following code for this, but it isn't smart for me.

md.linkify.tlds(allTopLevelDomainList.filter((tld) => tld !== 'py'), false)

So I changed the example from disabling .py.

@puzrin puzrin merged commit 1b204ef into markdown-it:master Oct 28, 2020
@puzrin
Copy link
Member

puzrin commented Oct 28, 2020

Thanks for clarification.

Anyway, it was not good idea to cut basic domains. But if you really need this feature - create issue with your reasons in linkify-it tracker, and i will try to help.

@pocke pocke deleted the fix-linkify-example-code branch October 28, 2020 07:03
@pocke
Copy link
Contributor Author

pocke commented Oct 28, 2020

Anyway, it was not good idea to cut basic domains. But if you really need this feature - create issue with your reasons in linkify-it tracker, and i will try to help.

Thank you, but now I do not need this feature because I noticed my application doesn't need to convert URL without a scheme to a link. So I will not open an issue.

Thanks for merging and the offer to help!

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