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 upAdding a translator for idref.fr #1545
Conversation
symac
added some commits
Jan 12, 2018
adam3smith
requested changes
Feb 12, 2018
|
This is elegantly done -- I don't mind the tests failing. A couple of comments, mostly stylistic to keep this in line with our evolving style recommendations, one substantive comment. |
| "translatorID": "271ee1a5-da86-465b-b3a5-eafe7bd3c156", | ||
| "label": "Idref", | ||
| "creator": "Sylvain Machefert", | ||
| "target": "^https?://www\\.idref\\.fr/.*", |
This comment has been minimized.
This comment has been minimized.
adam3smith
Feb 12, 2018
Collaborator
no need for the .* -- the target needs to match against the URL, not the other way around.
| "priority": 100, | ||
| "inRepository": true, | ||
| "translatorType": 4, | ||
| "browserSupport": "g", |
This comment has been minimized.
This comment has been minimized.
| "browserSupport": "g", | ||
| "lastUpdated": "2018-02-03 09:39:41" | ||
| } | ||
|
|
This comment has been minimized.
This comment has been minimized.
| var resultsHref = ZU.xpath(doc, '//div[@id="perenne-references-docs"]/span[contains(@class, "detail_label")]/a/@href'); | ||
|
|
||
| items = {}; | ||
| for (var i in resultsTitle) { |
This comment has been minimized.
This comment has been minimized.
adam3smith
Feb 12, 2018
Collaborator
iterate through arrays using either for (let i=0; i<resultsTitle.lenght; i++) or for (result of resultsTitle)
| Z.debug("scraping : " + url); | ||
| var translator = Zotero.loadTranslator('web'); | ||
|
|
||
| if (url.indexOf("archives-ouvertes")!=-1){ |
This comment has been minimized.
This comment has been minimized.
adam3smith
Feb 12, 2018
Collaborator
we've switched to using includes throughout for literal matches for readability (about same efficiency), so if (url.includes("archives-ouvertes")) etc.
| href = resultsHref[i].textContent; | ||
| // We need to replace the http://www.sudoc.fr/XXXXXX links are they are redirects and aren't handled correctly from subtranslator | ||
| href = href.replace(/http:\/\/www\.sudoc\.fr\/(.*)$/, "http://www.sudoc.abes.fr/xslt/DB=2.1//SRCH?IKT=12&TRM=$1"); | ||
| items[href] = resultsTitle[i].textContent; |
This comment has been minimized.
This comment has been minimized.
adam3smith
Feb 12, 2018
Collaborator
Why not filter out the URLs that you know we won't be able to import here? Better to not offer those results than to fail on them.
This comment has been minimized.
This comment has been minimized.
|
Thanks @adam3smith . I have updated the file following your comments, should be better now. Tell me if you see anything else that could be improved. |
zuphilip
reviewed
Feb 12, 2018
| Copyright (C) 2018 Sylvain Machefert - web@geobib.fr | ||
| This program is free software: you can redistribute it and/or modify | ||
| it under the terms of the GNU General Public License as published by |
This comment has been minimized.
This comment has been minimized.
zuphilip
Feb 12, 2018
Collaborator
All Zotero translators should be licensed under AGPL (Affero), see https://github.com/zuphilip/translators/wiki/Common-code-blocks-for-translators for the relevant block.
This comment has been minimized.
This comment has been minimized.
symac
Feb 12, 2018
Author
Contributor
now fixed. I had copied another translator from the repo without having in mind this information regarding the license to be used. AGPL is also fine for me, thanks for the pointer.
zuphilip
added
the
New Translator
label
Feb 16, 2018
This comment has been minimized.
This comment has been minimized.
|
@adam3smith @zuphilip what is the next step regarding this translator? I believe I have fixed the requests both of you did previously, or do I just need to wait (which is fine, no hurry, just want to be sure I am not missing something that I could do). |
adam3smith
merged commit aad2aa4
into
zotero:master
Mar 12, 2018
1 check passed
This comment has been minimized.
This comment has been minimized.
|
Thanks! |
This comment has been minimized.
This comment has been minimized.
|
@adam3smith can you confirm that it is working as expected for you? I have been told there were some issues and when testing, the translator is not showing in Firefox. If I try to visit this page in FF, nothing happens, if I visit the same page with Scaffold inside Zotero it is ok. I have just resinstalled everything (Zotero, Scaffold, Firefox connector). Any idea ? don't know if you have access to debug reports but in case, I have just created one under reference D1823926315. |
This comment has been minimized.
This comment has been minimized.
|
Yes, there's something weird going on with detect. Reloading the page sometimes works for me but not always. |
symac
referenced this pull request
Mar 28, 2018
Open
Idref translator not working in Firefox, ok in Scaffold #1610
symac commentedFeb 3, 2018
Hello,
idref.fr is the website allowing to get information about authorities (identifiers) available in various French catalogs (mainly SUDOC and HAL for the moment). These linked catalogs already have existing translators so this translator is mainly calling the correct one based on the list of items existing on a page.
Currently the translator is working as expected (https://www.idref.fr/199676100 for example), but I have not been able to run the populated test, I imagine that this is related to the way I call selectItems but I have not been able to understand what the issue is), I hope the Zotero team will be able to fix this slight issue before merging.
Happy to update my code if any advice is given regarding things that would not be done according to Zotero's expectations.