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

Add link_attr option to Autolink filter #89

Merged
merged 1 commit into from
Oct 22, 2013
Merged

Conversation

excid3
Copy link
Contributor

@excid3 excid3 commented Oct 17, 2013

Let's you add things like target="_blank" to links if you want to through the link_attr option just like Rinku allows.

@@ -15,6 +15,11 @@ def test_autolink_option
AutolinkFilter.to_html('<p>"http://www.github.com"</p>', :autolink => false)
end

def test_autolink_link_attr
assert_equal '<p>"<a href="http://www.github.com" target="_blank">http://www.github.com</a>"</p>',
AutolinkFilter.to_html('<p>"http://www.github.com"</p>', :link_attr => 'target="_blank"')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels strange to pass in the actual HTML string here. How would one pass in multiple link attributes?

Is it possible to have a hash syntax like:

AutolinkFilter.to_html '<p>"http://www.github.com"</p>',
  :link_attr => { :target => "_blank", :data_foo => "bar"}

@jch
Copy link
Contributor

jch commented Oct 22, 2013

👍 good idea

@excid3
Copy link
Contributor Author

excid3 commented Oct 22, 2013

I felt the same way but this is actually how Rinku works according to the
docs. Figured it made more sense to leave it compatible rather than adding
things on top. What do you think?
On Oct 21, 2013 7:03 PM, "Jerry Cheung" [email protected] wrote:

[image: 👍] good idea


Reply to this email directly or view it on GitHubhttps://github.com//pull/89#issuecomment-26767597
.

@jch
Copy link
Contributor

jch commented Oct 22, 2013

Works for me. If you're using Rinku, then you're already used to the syntax.

jch added a commit that referenced this pull request Oct 22, 2013
Add link_attr option to Autolink filter
@jch jch merged commit 8ef9c5e into gjtorikian:master Oct 22, 2013
@excid3
Copy link
Contributor Author

excid3 commented Oct 22, 2013

👍

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