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

Set rel="external" on external links #1643

Open
wants to merge 1 commit into
base: master
from
Open

Conversation

@ppKrauss
Copy link

@ppKrauss ppKrauss commented Sep 28, 2018

Hi, please check if it is correct... and perhaps opportunity to some reuse. The focus here was only to suggest a simple solution to the issue #1639, adding the attribute rel="external" in an <a href> tag that seems not local.

Copy link
Member

@waylan waylan left a comment

Rather than checking for external v. internal twice, why not have path_to_url return a tuple of the url and a Boolean indicating whether the URL is internal or not?

Also, the tests need to be updated.

@ppKrauss
Copy link
Author

@ppKrauss ppKrauss commented Sep 28, 2018

why not have path_to_url return a tuple of the url and a Boolean

Yes, ideal, but the code need the original schema, not the returned (local can be a HTTP?), so the best is to use components as parameter.

indicating whether the URL is internal or not?
It is all in components to generic test, but, yes, if no reuse can be reduced to specific, so, a boolean.

Also, the tests need to be updated.

You can delete this pull request and do yours, was only a suggestion, I have no way to test here.

@waylan waylan changed the title suggestion for #1639 Set rel="external" on external links Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.