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 upAdd China/Asia On Demand translator #1737
Conversation
adam3smith
requested changes
Sep 8, 2018
Thanks for taking this! Two small things (the first one is really more cosmetic). |
"translatorID": "d2a9e388-5b79-403a-b4ec-e7099ca1bb7f", | ||
"label": "CAOD", | ||
"creator": "Guy Aglionby", | ||
"target": "^https?://caod\\.oriprobe\\.com/articles/[^#]+", |
This comment has been minimized.
This comment has been minimized.
adam3smith
Sep 8, 2018
Collaborator
remember you don't need the target to capture the whole URL, it simply runs target.test(url)
so the [^#]+
doesn't do anything
*/ | ||
|
||
function detectWeb(doc, url) { | ||
if (url.includes('articles/found.htm?')) { |
This comment has been minimized.
This comment has been minimized.
adam3smith
Sep 8, 2018
Collaborator
this needs to be more restrictive, e.g. gives a false positive on empty searches such as http://caod.oriprobe.com/articles/found.htm?keyword=rtryiy&package=&key_author=&key_qkname=&key_year=&key_volumn=&key_issue=
that's why we have getSearchResults
in the templates -- it's pretty much always a good idea to match doWeb
and detectWeb
to avoid false positive in the latter.
This comment has been minimized.
This comment has been minimized.
Thanks for the feedback and the heads up re. |
adam3smith
merged commit 5d260e6
into
zotero:master
Sep 9, 2018
1 check passed
This comment has been minimized.
This comment has been minimized.
Thanks! |
GuyAglionby commentedSep 8, 2018
Addresses #1716.
The second test case has the university names as authors, as this is what is indicated on the website. Unfortunately I can't think of a good way of detecting these and removing them, given the range of things (especially wrt. spacing) seen in the author field (e.g. one, two).
Thanks!