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

Add Google AI translator #1741

Merged
merged 3 commits into from Sep 28, 2018

Conversation

Projects
None yet
2 participants
@GuyAglionby
Copy link
Contributor

GuyAglionby commented Sep 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: true is needed for automated testing, which doesn't work (zotero-dev).

Thanks :)

@adam3smith
Copy link
Collaborator

adam3smith left a comment

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.

@adam3smith

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.

@GuyAglionby

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.

@adam3smith

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.

@GuyAglionby

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.

@adam3smith

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.

@adam3smith

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

@GuyAglionby

This comment has been minimized.

Copy link
Contributor Author

GuyAglionby commented Sep 15, 2018

Thanks for the hints, made the changes as suggested :)

@adam3smith adam3smith merged commit 56fa65b into zotero:master Sep 28, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@adam3smith

This comment has been minimized.

Copy link
Collaborator

adam3smith commented Sep 28, 2018

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.