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

Improve localhost link filtering in markdown during pr-check.js #9207

Merged
merged 2 commits into from
May 8, 2017
Merged

Improve localhost link filtering in markdown during pr-check.js #9207

merged 2 commits into from
May 8, 2017

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented May 8, 2017

On further testing, some inline links in markdown were slipping through, causing false negatives in "gulp check-links". This PR goes through the two kinds of markdown elements that contain links (within parentheses and brackets), and then filters out raw localhost links found in text.

For reference, see https://guides.github.com/features/mastering-markdown/#syntax (scroll down to Links)

Issue #7946
Follow up to PR #9099

@rsimha rsimha self-assigned this May 8, 2017
@rsimha rsimha added this to the Sprint H1 May milestone May 8, 2017
* @return {string} Markdown after filtering out localhost links.
*/
function filterLocalhostLinks(markdown) {
var localhostPattern = 'http:\/\/localhost:8000';
Copy link
Contributor

Choose a reason for hiding this comment

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

can be merged into a single regex, feel free to add TODO + issue to tackle soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@aghassemi aghassemi merged commit 6b68abe into ampproject:master May 8, 2017
@rsimha rsimha deleted the 2017-05-08-FixLocalhostDetection branch May 8, 2017 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants