Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upFix recognition of articles and getting PDFs in Nature #1766
Conversation
adam3smith
requested a review
from
zuphilip
Oct 12, 2018
This comment has been minimized.
This comment has been minimized.
I guess that it fixes #1763 and maybe this is the report you remembered? |
zuphilip
reviewed
Oct 12, 2018
Okay, looks fine. Some small comments and suggestions. The most fragile thing is IMO the order of the attribute values of |
if (!item.attachments) { | ||
var hasPDF = false; | ||
for (let attach of item.attachments){ | ||
if (attach.title.includes("PDF")) { |
This comment has been minimized.
This comment has been minimized.
} | ||
} | ||
if (!hasPDF) { | ||
item.attachments=[]; |
This comment has been minimized.
This comment has been minimized.
zuphilip
Oct 12, 2018
Collaborator
That line seems negligible, because its value is overwritten by the next line.
var m = url.match(/(^[^#?]+\/)(?:full|abs)(\/[^#?]+?\.)[a-zA-Z]+(?=$|\?|#)/); | ||
if (m && m.length) return m[1] + 'pdf' + m[2] + 'pdf'; | ||
else if (attr(doc, 'a[data-track-action="download pdf"]', 'href')) { | ||
return attr(doc, 'a[data-track-action="download pdf"]', 'href'); |
This comment has been minimized.
This comment has been minimized.
zuphilip
Oct 12, 2018
Collaborator
Is the check before here needed? attr
should return null
if the element is not found, which should be okay here.
@@ -354,7 +361,7 @@ function scrapeRIS(doc, url, next) { | |||
if (!risURL) risURL = doc.evaluate('//li[@class="download-citation"]/a', doc, null, XPathResult.ANY_TYPE, null).iterateNext(); | |||
if (!risURL) risURL = doc.evaluate('//a[normalize-space(text())="Export citation" and not(@href="#")]', doc, null, XPathResult.ANY_TYPE, null).iterateNext(); | |||
if (!risURL) risURL = ZU.xpath(doc, '//ul[@data-component="article-info-list"]//a[@data-track-source="citation-download"]')[0]; | |||
|
|||
if (!risURL) risURL = doc.querySelectorAll('a[data-track-action="download article citation"]')[0]; |
This comment has been minimized.
This comment has been minimized.
zuphilip
Oct 12, 2018
Collaborator
For the first element you can also use .querySelector
cf. https://developer.mozilla.org/de/docs/Web/API/Document/querySelector
This comment has been minimized.
This comment has been minimized.
OK, all done |
zuphilip
merged commit 4381a20
into
zotero:master
Oct 13, 2018
1 check passed
This comment has been minimized.
This comment has been minimized.
Thank you very much! |
adam3smith commentedOct 12, 2018
(this was reported on the Zotero forums, but can't find it (?)