Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd Google AI translator #1741
Conversation
adam3smith
reviewed
Sep 14, 2018
|
Just a couple of questions/nits -- this looks good as is, but see if any of these make sense/are helpful. |
| if (url.includes('/pubs/pub')) { | ||
| let bibtex = extractBibtex(doc); | ||
| let doctype = bibtex.split('{')[0].replace('@', ''); | ||
| return bibtex2zoteroTypeMap[doctype]; |
This comment has been minimized.
This comment has been minimized.
adam3smith
Sep 14, 2018
Collaborator
I wonder if it's worth putting in a fallback here? Just in case they add an unforeseen item type -- we'd detect the wrong type, but at least we'd show a save icon and the actual import would likely be correct.
This comment has been minimized.
This comment has been minimized.
GuyAglionby
Sep 15, 2018
Author
Contributor
Could it be worth copying in the whole list from BibTeX.js? Or is there a cleaner way of doing the mapping? I only added types that I saw, but it seems like they put a variety of content on there so adding this and a fallback seems a good idea.
This comment has been minimized.
This comment has been minimized.
adam3smith
Sep 28, 2018
Collaborator
Good point. I don't see any real cost of adding the whole list. It's pretty short. I'd still do a fallback, likely to journal article, just in case because bibtex is so poorly standardized. (and sorry for the delay, I could have written those 2 sentences faster ;)
This comment has been minimized.
This comment has been minimized.
GuyAglionby
Sep 28, 2018
Author
Contributor
Didn't know it was so poorly standardised -- good to know. Amended as suggested, and no problem about the delay
| } | ||
| ZU.processDocuments(Object.keys(selected), scrape); | ||
| }); | ||
| } else if (detectType) { |
This comment has been minimized.
This comment has been minimized.
adam3smith
Sep 14, 2018
Collaborator
You can just do else here. It's impossibel to call doWeb if detectWeb is false.
|
|
||
| function extractBibtex(doc) { | ||
| let bibtexElement = ZU.xpath(doc, '//a[span[contains(@class, "icon--copy")]]')[0]; | ||
| let escapedBibtex = bibtexElement.getAttribute('ng-click') |
This comment has been minimized.
This comment has been minimized.
adam3smith
Sep 14, 2018
Collaborator
any reason you're not just using ZU.xpathText(doc, '//a[span[contains(@class, "icon--copy")]]/@ng-click')? or the attr function with a CSS selector? Same above for getting the PDF
adam3smith
added
waiting
New Translator
labels
Sep 15, 2018
adam3smith
referenced this pull request
Sep 15, 2018
Closed
Add new Google Research translator #1503
This comment has been minimized.
This comment has been minimized.
|
Thanks for the hints, made the changes as suggested :) |
adam3smith
merged commit 56fa65b
into
zotero:master
Sep 28, 2018
1 check passed
This comment has been minimized.
This comment has been minimized.
|
Great, thanks! |
GuyAglionby commentedSep 9, 2018
Add translator for https://ai.google/research . Supersedes #1503 as they now redirect https://research.google.com to this URL. Put URLs in the code for testing the search page and for personal pages, as they use Angular to populate data and thus
defer: trueis needed for automated testing, which doesn't work (zotero-dev).Thanks :)