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 LinkExtractor class #120

Merged
merged 1 commit into from
Jan 4, 2018
Merged

Add LinkExtractor class #120

merged 1 commit into from
Jan 4, 2018

Conversation

camillavk
Copy link
Contributor

This LinkExtractor class is used in several different publishing applications when checking links on documents. It extracts any links on the document, excluding ones starting with 'mailto:' or '#', and returns an array of the links present.

private

def extract_links
processed_govspeak.css('a:not([href^="mailto"])').css('a:not([href^="#"])').map { |link| link['href'] }
Copy link
Contributor

Choose a reason for hiding this comment

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

I would maybe extract this line out to another method so it's something like:

def extract_links
  document_anchors.map { |link| link['href'] }
end

def document_anchors
  processed_govspeak.css('a:not([href^="mailto"])').css('a:not([href^="#"])')
end

@thomasleese thomasleese mentioned this pull request Jan 4, 2018
Copy link
Contributor

@thomasleese thomasleese left a comment

Choose a reason for hiding this comment

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

Would be good to update the changelog in this PR.

This LinkExtractor class is used in several different publishing applications when checking links on documents
@camillavk
Copy link
Contributor Author

@thomasleese I've updated the CHANGELOG :)

@camillavk camillavk merged commit 8a1e737 into master Jan 4, 2018
@camillavk camillavk deleted the add_link_extractor branch January 4, 2018 11:59
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