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 ARTnews.js #1256

Merged
merged 7 commits into from Mar 12, 2017

Conversation

@owcz
Copy link
Contributor

commented Mar 11, 2017

No description provided.

owcz added some commits Mar 11, 2017

ARTnews.js Outdated
"creatorType": "author"
}
],
"date": "01/23/16 10:35 am",

This comment has been minimized.

Copy link
@adam3smith

adam3smith Mar 11, 2017

Collaborator

It'd be good to transform these dates to YYYY-MM-DD to avoid confusion with different locales. I think something like
.replace(/(\d{2})\/(\d{2})\/(\d{2})/, "20$3-$1-$2") should work, but please test.

This comment has been minimized.

Copy link
@zuphilip

zuphilip Mar 11, 2017

Collaborator

Maybe, before that just try out whether ZU.strToISO(...) is giving us already a nice formatted date.

@adam3smith I guess, that one of $2 should be $1.

This comment has been minimized.

Copy link
@adam3smith

adam3smith Mar 11, 2017

Collaborator

yes, thanks fixed, and good point on strToISO

This comment has been minimized.

Copy link
@owcz

owcz Mar 11, 2017

Author Contributor

strToISO can handle the string but drops the time ("03/02/17 10:40 pm" => "2017-03-02"). For what it's worth, Zotero did detect and export the date correctly as it was originally ingested.

@zuphilip What's the best way to call this in FW? Zotero.Utilities.strToISO(FW.Xpath('//span[@class="date-meta"]').text()) doesn't work.

This comment has been minimized.

Copy link
@zuphilip

zuphilip Mar 11, 2017

Collaborator

The date without the time is normally enough for citations when used as a publication date. The time last modified or accessed a resource might be different.

I am not sure whether you can use this function in the FW.

This comment has been minimized.

Copy link
@adam3smith

adam3smith Mar 11, 2017

Collaborator

yeah, you only have a list of specified functions available for FW, so this probably doesn't work.
It should be available in a hook, though:
https://www.zotero.org/support/dev/translators/framework#post-processing_with_hooks

This comment has been minimized.

Copy link
@owcz

owcz Mar 11, 2017

Author Contributor

@zuphilip I can call strToISO (Zotero.Utilities.strToISO("03/02/17 10:40 pm") works fine) but I don't know if there's a trick to calling it with FW.Xpath

ARTnews.js Outdated
"title": "The Whitney Biennial Arrives! Here’s a Round-Up of Coverage of Artists in the Show",
"creators": [
{
"firstName": "The Editors of",

This comment has been minimized.

Copy link
@adam3smith

adam3smith Mar 11, 2017

Collaborator

how common is this as the exact author string? If this comes up a lot, might be worth adding a hook that transform this into a single-field author.

This comment has been minimized.

Copy link
@owcz

owcz Mar 11, 2017

Author Contributor

Not often in my usage, but upon search, looks often enough. Is there a way to do what you mention within FW or does the whole translator need to be written out? @adam3smith

This comment has been minimized.

Copy link
@adam3smith

adam3smith Mar 11, 2017

Collaborator

I know I'm saying this everywhere, but in FW, hooks are the only way to go here:
https://www.zotero.org/support/dev/translators/framework#post-processing_with_hooks

This comment has been minimized.

Copy link
@owcz

owcz Mar 11, 2017

Author Contributor

Aha! Just what I wanted. Thanks

This comment has been minimized.

Copy link
@adam3smith

adam3smith Mar 11, 2017

Collaborator

I got an e-mail with a question about single field but can't find your post. Did you figure that out or do you still need an answer?

This comment has been minimized.

Copy link
@owcz

owcz Mar 11, 2017

Author Contributor

Figured out out, though I don't see mention of fieldMode in the documentation. I've had issues contributing to the Zotero wiki for a while—I'd be happy to contribute what I've learned if the wiki were, for instance, on Github

This comment has been minimized.

Copy link
@adam3smith

adam3smith Mar 11, 2017

Collaborator

as in you can't log in with edit access or something else? There was so much vandalism, that Zotero devs need to give you access individually (I thought they did that for you?). Post to the forums with a link to this post.

This comment has been minimized.

Copy link
@owcz

owcz Mar 12, 2017

Author Contributor

I log in but don't have write access again—I'll work it out with Dan but just wonder if it wouldn't be easier with another (more accessible) platform

owcz added some commits Mar 11, 2017

ARTnews date parsing
Note that the comma is needed in the replace, or else Zotero will only detect the time and not the date
@adam3smith
Copy link
Collaborator

left a comment

last nit

ARTnews.js Outdated
tags : FW.Xpath('//span[@class="cat-links"]/a').text(),
hooks : { "scraperDone": function (item,doc,url) {
if (item.creators[0].lastName == "ARTnews") {
item.creators[0].lastName = "The Editors of ARTnews";

This comment has been minimized.

Copy link
@adam3smith

adam3smith Mar 11, 2017

Collaborator

you need to remove the firstName here still (I think you can just do that with delete but I always get confused about removing items from objects.

This comment has been minimized.

Copy link
@owcz

owcz Mar 12, 2017

Author Contributor

Done

owcz added some commits Mar 12, 2017

@adam3smith adam3smith merged commit 21d4b8c into zotero:master Mar 12, 2017

1 check passed

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

This comment has been minimized.

Copy link
Collaborator

commented Mar 12, 2017

Great, thanks!

@owcz owcz deleted the owcz:patch-5 branch Mar 12, 2017

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

Add ARTnews.js (zotero#1256)
* ARTnews date parsing
Note that the comma is needed in the replace, or else Zotero will only detect the time and not the date
* Handle "The Editors of ARTnews"

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

Add ARTnews.js (zotero#1256)
* ARTnews date parsing
Note that the comma is needed in the replace, or else Zotero will only detect the time and not the date
* Handle "The Editors of ARTnews"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.