-
Notifications
You must be signed in to change notification settings - Fork 23
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
support more arxiv urls formats #711
Conversation
Codecov Report
@@ Coverage Diff @@
## master #711 +/- ##
============================================
- Coverage 68.61% 67.05% -1.57%
- Complexity 1609 1622 +13
============================================
Files 12 12
Lines 3202 3217 +15
============================================
- Hits 2197 2157 -40
- Misses 1005 1060 +55
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another instance where it's worth adding a test case: it doesn't look to me like your regexp will match the URL you provide as an example.
Template.php
Outdated
@@ -832,7 +832,7 @@ protected function get_identifiers_from_url($url_sent = NULL) { | |||
} | |||
return $this->add_if_new("citeseerx", urldecode($match[1])); // We cannot parse these at this time | |||
|
|||
} elseif (preg_match("~\barxiv\.org/.*(?:pdf|abs)/(.+)$~", $url, $match)) { | |||
} elseif (preg_match("~\barxiv\.org/.*(?:pdf|abs|tp/arxiv/papers/\d{4})/(.+?)(?:\.pdf)?$~us", $url, $match)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need unicode support for URLs?
and have you deliberately omitted the f from ftp?
Such as https://arxiv.org/ftp/arxiv/papers/1312/1312.7288.pdf