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 El Pais Translator #1514

Merged
merged 4 commits into from Jan 7, 2018

Conversation

Projects
None yet
2 participants
@adam3smith
Copy link
Collaborator

adam3smith commented Jan 7, 2018

supersedes #1146

@adam3smith adam3smith requested a review from zuphilip Jan 7, 2018

@zuphilip
Copy link
Collaborator

zuphilip left a comment

This looks good in general. Just some nits. Thank you

El Pais.js Outdated
}
}
item.publicationTitle = "El País";
item.ISSN = "0213-4608";

This comment has been minimized.

@zuphilip

zuphilip Jan 7, 2018

Collaborator

That seems to be the ISSN of an weekly published international version between 1983 and 1999, see http://zdb-katalog.de/title.xhtml?idn=012116335 . Better numbers are 1134-6582 from http://zdb-katalog.de/title.xhtml?idn=011132388&tab=4 or 1576-3757 from https://www.wikidata.org/wiki/Q50009 .

El Pais.js Outdated
function detectWeb(doc, url) {
if (url.search(/\d+_\d+\.html/) !== -1) {
return "newspaperArticle";
} else if ((url.includes("/buscador")||url.includes("/tag/")) && getSearchResults(doc, true)) {

This comment has been minimized.

@zuphilip

zuphilip Jan 7, 2018

Collaborator

Add spaces around || for better readability.

El Pais.js Outdated
}
item.publicationTitle = "El País";
item.ISSN = "0213-4608";
item.place = "Madrid";

This comment has been minimized.

@zuphilip

zuphilip Jan 7, 2018

Collaborator

I think they have offices in other cities as well. Is this really meaningful when extracting the online version?

This comment has been minimized.

@adam3smith

adam3smith Jan 7, 2018

Author Collaborator

To the extent that citation styles want that information, El Pais (Madrid) is going to distinguish it from other El Pais papers which may exists e.g. in Latin Americal. ZDB also cites it as Madrid, so I'll leave this in.

El Pais.js Outdated
"libraryCatalog": "elpais.com",
"place": "Madrid",
"publicationTitle": "El País",
"section": "Mundo_global",

This comment has been minimized.

@zuphilip

zuphilip Jan 7, 2018

Collaborator

Replace the underscore with a space.

This comment has been minimized.

@adam3smith

adam3smith Jan 7, 2018

Author Collaborator

done -- I applied the title casing after replacing to get "Mundo Global" etc. in line with what the sections are called in the paper.

El Pais.js Outdated
//Z.debug(authors)
for (let author of authors) {
if (author !== "Agencias") {
item.creators.push(ZU.cleanAuthor(author, "author"))

This comment has been minimized.

@zuphilip

zuphilip Jan 7, 2018

Collaborator

Add ; at the end.

"type": "web",
"url": "https://elpais.com/tag/estados_unidos/a/",
"items": "multiple"
},

This comment has been minimized.

@zuphilip

zuphilip Jan 7, 2018

Collaborator

Add another test case for search page, e.g. https://elpais.com/buscador/?qt=zotero

@adam3smith

This comment has been minimized.

Copy link
Collaborator Author

adam3smith commented Jan 7, 2018

Thanks -- please have a look at updated version

@zuphilip zuphilip merged commit a6b2412 into zotero:master Jan 7, 2018

1 check passed

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

This comment has been minimized.

Copy link
Collaborator

zuphilip commented Jan 7, 2018

Thank you!

@zuphilip zuphilip referenced this pull request Jan 7, 2018

Closed

Add ElPais.com translator #1146

GuyAglionby added a commit to GuyAglionby/translators that referenced this pull request Jan 28, 2018

Add El Pais Translator (zotero#1514)
* Add El Pais Translator
* Update following review
* Update tests after zotero#1515

psisquared2 added a commit to psisquared2/translators that referenced this pull request Feb 8, 2018

Add El Pais Translator (zotero#1514)
* Add El Pais Translator
* Update following review
* Update tests after zotero#1515
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.