Skip to content

Support urls with dots - #176

Merged
codelucas merged 1 commit into
codelucas:masterfrom
alexanderlukanin13:urls_with_dots
Jan 10, 2016
Merged

Support urls with dots#176
codelucas merged 1 commit into
codelucas:masterfrom
alexanderlukanin13:urls_with_dots

Conversation

@alexanderlukanin13

Copy link
Copy Markdown
Contributor

There is a critical bug in newspaper3k==0.1.5 which prevents it from recognizing urls with dots in them.

For example: http://www.enfieldindependent.co.uk/news/

Please upload a new version on PyPI if possible.

@codelucas

Copy link
Copy Markdown
Owner

@alexanderlukanin13 another great find!

@codelucas codelucas closed this Jan 10, 2016
@codelucas codelucas reopened this Jan 10, 2016
codelucas added a commit that referenced this pull request Jan 10, 2016
@codelucas
codelucas merged commit e1118ee into codelucas:master Jan 10, 2016
Comment thread newspaper/urls.py

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Perhaps 5 should be defined in a constants file? Ugh, but it's not like the rest of this codebase set a good example

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@codelucas 5 is a quick-and-dirty heuristic, and it should be kept local. Moving it to constants would only give it an undeserved air of authority.

The proper solution is to use a extension blacklist and HEAD requests for uncertain cases.

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