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

Fixes T #1351

Merged
merged 34 commits into from Jul 15, 2017

Conversation

@zuphilip
Copy link
Collaborator

commented Jul 5, 2017

Finally, this is the last letter in my tour from A to Z 🎉 .

Please have a look at these fixes, update and rewrites.

zuphilip added some commits Jun 27, 2017

Delete Tatar zamanı.js
The website for this newspaper does not exists anymore
and the domain www.tatartime.com/ just show some information
about hard drive destruction.
Rewrite The Daily Beast.js
I used newspaperArticle for the type rather than
generic website. Moreover, it seems that the blogs
are not anymore hosted under this domain. The test
case for this for example moved to this new url
http://pulitzercenter.org/reporting/egyptian-theater-troupe-brings-domestic-violence-light
The cheatsheet seems to be a page with multiple
smaller news articles and is currently skipped
completely.
Update The Economist.js
The translator should work now when the page is fully loaded.
However, the website uses quite extensively some delayed loading
of contents, such that test sometimes fail.
Rename and rewrite The Times and Sunday Times.js
* The name corresponds now to the label
* There seems to exist no search on the website
--> no multiples
Update tests in Theory of Computing.js
There is a 404 on the Indian mirror,
switched test url to Swedish mirror.

zuphilip added some commits Jul 5, 2017

Update Twitter.js
Note that multiples work when site is loaded but not as
autoamtic test cases.
@adam3smith

This comment has been minimized.

Copy link
Collaborator

commented Jul 6, 2017

@zuphilip -- any views on the test here? It shouldn't fail, but can't handle the rename of the translator. Since that test only goes for individual PRs (i.e. won't keep failing), we could just ignore it, or do you want to try to fix the test?

@zuphilip

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 6, 2017

Here it is a rename and completely rewrite which will not work for Travis. I tried some improvements in the linked PR, but also with this improved script, Travis will still complain. Thus, I suggest to ignore Travis on this here.

@adam3smith
Copy link
Collaborator

left a comment

whew. That was a marathon. Given that, quite few comments and all should be pretty quick.

"notes": [
{
"note": "n.1. a. Physical or mental exertion, especially when difficult or exhausting; work. See Synonyms at work.b. Something produced by work.2. A specific task.3. A particular form of work or method of working: manual labor.4. Work for wages.5. a. Workers considered as a group.b. The trade union movement, especially its officials.6. Labor A political party representing workers' interests, especially in Great Britain.7. The process by which childbirth occurs, beginning with contractions of the uterus and ending with the expulsion of the fetus or infant and the placenta."
"note": "n.1. Physical or mental exertion, especially when difficult or exhausting; work. See Synonyms at work.2. A specific task or effort, especially a painful or arduous one: \"Eating the bread was a labor I put myself through to quiet my stomach\" (Gail Anderson-Dargatz).3. A particular form of work or method of working: manual labor.4. Work for wages: businesses paying more for labor.5. a. Workers considered as a group.b. The trade union movement, especially its officials.6. Labor A political party representing workers' interests, especially in Great Britain.7. The process by which childbirth occurs, beginning with contractions of the uterus and ending with the expulsion of the fetus or infant and the placenta."

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

Not wild about all these notes -- let's put this into a single one?

item.creators.push(ZU.cleanAuthor(authors[i], "author"));
}
}
//FW.Xpath('//p[@class="byline"]').text().remove(/\s(in|for)\s.+/).split(/\s+and\s+|\s*,\s*/).cleanAuthor("author")

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

delete

//translator.setDocument(doc);

translator.setHandler('itemDone', function (obj, item) {
item.publicationTitle = "The Hamilton Spectator";

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

item.ISSN = "1189-9417";

@@ -2,14 +2,14 @@
"translatorID": "9499c586-d672-42d6-9ec4-ee9594dcc571",
"label": "The Hindu (old)",
"creator": "Prashant Iyengar and Michael Berkowitz",
"target": "^https?://(www\\.)?hindu\\.com/",
"target": "^https?://(www\\.)?thehindu\\.com/lr/",

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

let's also add item.ISSN = "0971-751X"; and remove Zotero.wait() and Zotero.done()

if (authorString) {
var authors = authorString.split(/\.,|&/);
for (var i=0; i<authors.length; i++) {
item.creators.push(ZU.cleanAuthor(authors[i], "author"));

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

I think you need a , true here. note the authors in the tests have lastName & firstName swapped

}
],
"date": "2017-07-02",
"ISSN": "0140-0460",

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

I think the Sunday times should be 0956-1382 see https://en.wikipedia.org/wiki/The_Sunday_Times

***** END LICENSE BLOCK *****
*/


function detectWeb(doc, url) {
if (url.indexOf("search") != -1 && url.indexOf("classifieds") == -1) {

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

&& getSearchResults(doc, true)

Twitter.js Outdated
}
var urlParts = url.split('/');
item.blogTitle = '@' + urlParts[3];
item.websiteType = "microblog";

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

We should maybe reconsider this -- no one seems to actually use it. We could just say "Tweet" which is what APA uses (http://blog.apastyle.org/apastyle/2013/10/how-to-cite-social-media-in-apa-style.html ) and what MLA used to use ( https://www.theatlantic.com/technology/archive/2012/03/how-do-you-cite-a-tweet-in-an-academic-paper/253932/ )

This comment has been minimized.

Copy link
@zuphilip

zuphilip Jul 10, 2017

Author Collaborator

ok

item.creators.splice(i, 1);
}
}

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

item.ISSN = "0931-9085";

taz.de.js Outdated
//translator.setDocument(doc);

translator.setHandler('itemDone', function (obj, item) {
item.publicationTitle = "taz. die tageszeitung";

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 9, 2017

Collaborator

I think this should just be "die tageszeitung", no?

This comment has been minimized.

Copy link
@zuphilip

zuphilip Jul 10, 2017

Author Collaborator

In the ZDB we have Die Tageszeitung : taz which looks nice for me. I will change it to this.

@zuphilip

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 10, 2017

Thank you for the helpful comments. Please have a look at the new version.

taz.de.js Outdated
@@ -87,7 +87,8 @@ function scrape(doc, url) {
//translator.setDocument(doc);

translator.setHandler('itemDone', function (obj, item) {
item.publicationTitle = "taz. die tageszeitung";
item.publicationTitle = "Die Tageszeitung : taz";

This comment has been minimized.

Copy link
@adam3smith

adam3smith Jul 10, 2017

Collaborator

without the space before the colon, though, no?

@zuphilip

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 10, 2017

Yes, deleted space before colon.

@adam3smith adam3smith merged commit 1797efb into zotero:master Jul 15, 2017

1 check failed

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

This comment has been minimized.

Copy link
Collaborator

commented Jul 15, 2017

Woot! You just finished the translator ironman!

@zuphilip zuphilip deleted the zuphilip:fixes-T branch Jul 15, 2017

zuphilip added a commit to zuphilip/translators that referenced this pull request Mar 28, 2018

Fixes T (zotero#1351)
* Update Tagesspiegel.js
* Update Talis Aspire.js
* Delete Tatar zamanı.js
The website for this newspaper does not exists anymore
and the domain www.tatartime.com/ just show some information
about hard drive destruction.
* Rewrite taz.de.js
* Update Taylor and Francis+NEJM.js
* Update tests in The Atlantic.js
* Update The Boston Globe.js
* Update The Chronicle of Higher Education.js
* Rewrite The Daily Beast.js
I used newspaperArticle for the type rather than
generic website. Moreover, it seems that the blogs
are not anymore hosted under this domain. The test
case for this for example moved to this new url
http://pulitzercenter.org/reporting/egyptian-theater-troupe-brings-domestic-violence-light
The cheatsheet seems to be a page with multiple
smaller news articles and is currently skipped
completely.
* Update The Economist.js
The translator should work now when the page is fully loaded.
However, the website uses quite extensively some delayed loading
of contents, such that test sometimes fail.
* Update The Free Dictionary.js
* Update tests in The Globe and Mail.js
* Rewrite The Guardian.js
Closes zotero#1347
* Rewrite The Hamilton Spectator.js
* Rewrite The Hindu.js
* Update The Hindu (old).js
* Rewrite The Met.js
* Rewrite The Microfinance Gateway.js
* Rewrite The Nation.js
* Update The New Republic.js
* Rewrite The New York Review of Books.js
* Rewrite The New Yorker.js
* Update tests in The Telegraph.js
* Rename and rewrite The Times and Sunday Times.js
Name corresponds now to the label
There seems to exist no search on the website
--> no multiples
* Update TheMarker.js
* Update tests in Theory of Computing.js
There is a 404 on the Indian mirror,
switched test url to Swedish mirror.
* Rewrite Toronto Star.js
* Update Treesearch.js
* Rewrite TVNZ.js
* Update Twitter.js
Note that multiples work when site is loaded but not as
autoamtic test cases.
* Fix date in The Hindu.js

zuphilip added a commit to zuphilip/translators that referenced this pull request Mar 28, 2018

Fixes T (zotero#1351)
* Update Tagesspiegel.js
* Update Talis Aspire.js
* Delete Tatar zamanı.js
The website for this newspaper does not exists anymore
and the domain www.tatartime.com/ just show some information
about hard drive destruction.
* Rewrite taz.de.js
* Update Taylor and Francis+NEJM.js
* Update tests in The Atlantic.js
* Update The Boston Globe.js
* Update The Chronicle of Higher Education.js
* Rewrite The Daily Beast.js
I used newspaperArticle for the type rather than
generic website. Moreover, it seems that the blogs
are not anymore hosted under this domain. The test
case for this for example moved to this new url
http://pulitzercenter.org/reporting/egyptian-theater-troupe-brings-domestic-violence-light
The cheatsheet seems to be a page with multiple
smaller news articles and is currently skipped
completely.
* Update The Economist.js
The translator should work now when the page is fully loaded.
However, the website uses quite extensively some delayed loading
of contents, such that test sometimes fail.
* Update The Free Dictionary.js
* Update tests in The Globe and Mail.js
* Rewrite The Guardian.js
Closes zotero#1347
* Rewrite The Hamilton Spectator.js
* Rewrite The Hindu.js
* Update The Hindu (old).js
* Rewrite The Met.js
* Rewrite The Microfinance Gateway.js
* Rewrite The Nation.js
* Update The New Republic.js
* Rewrite The New York Review of Books.js
* Rewrite The New Yorker.js
* Update tests in The Telegraph.js
* Rename and rewrite The Times and Sunday Times.js
Name corresponds now to the label
There seems to exist no search on the website
--> no multiples
* Update TheMarker.js
* Update tests in Theory of Computing.js
There is a 404 on the Indian mirror,
switched test url to Swedish mirror.
* Rewrite Toronto Star.js
* Update Treesearch.js
* Rewrite TVNZ.js
* Update Twitter.js
Note that multiples work when site is loaded but not as
autoamtic test cases.
* Fix date in The Hindu.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.