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 new translator for jurion #1241

Merged
merged 2 commits into from Feb 7, 2017

Conversation

Projects
None yet
2 participants
@zuphilip
Copy link
Collaborator

zuphilip commented Jan 28, 2017

This is a German law portal with journal articles,
cases, commentaries, ebooks, statutes. The access
is restricted to subscribers.

Add new translator for jurion
This is a German law portal with journal articles,
cases, commentaries, ebooks, statutes. The access
is restricted to subscribers.
@adam3smith
Copy link
Collaborator

adam3smith left a comment

Some questions/requests, mostly stylistic in line. The regex one for removing titles I think is a must-fix. See what you think about the others.

jurion.js Outdated
var type = detectWeb(doc, url);
var item = new Zotero.Item(type);

item.title = ZU.xpathText(doc, '//div[@id="middle-column"]//h1[div[contains(@class, "addToList")]]');// [contains(@class, "artikel_titel") or contains(@class, "gesetz_ev_titel")]

This comment has been minimized.

@adam3smith

adam3smith Jan 30, 2017

Collaborator

what does the commented out code mean? Could you put that in a separate line and add a couple of words to explain?

This comment has been minimized.

@zuphilip

zuphilip Jan 30, 2017

Author Collaborator

That was my first attempt, which I forgot to delete.

jurion.js Outdated

var author = getValueToLabel(doc, "Autor") || getValueToLabel(doc, "Verfasst von");
if (author) {
author = author.replace(/Dr\.?|Prof\.?|RA\.?/g, '');

This comment has been minimized.

@adam3smith

adam3smith Jan 30, 2017

Collaborator

Especially for Dr, this seems too broad and would e.g. turn Dregger or Dreßen into egger and eßen. How about adding a space to the regex, i.e. something like /(Dr|Prof|RA)\.?\s/

This comment has been minimized.

@zuphilip

zuphilip Jan 30, 2017

Author Collaborator

Good call. Yes, I will change.

jurion.js Outdated

var ref = getValueToLabel(doc, "Referenz");
if (ref) {
var m = ref.match(/(\d\d\d\d),\s+([\d\s\-IVX]*)\((.*)\)/);

This comment has been minimized.

@adam3smith

adam3smith Jan 30, 2017

Collaborator

Any reason you use \d\d\d\d over \d{4}? I find the latter much easier to read, but maybe that's just personal preference -- I won't insist.

This comment has been minimized.

@zuphilip

zuphilip Jan 30, 2017

Author Collaborator

I think I had sometimes problems with the \d{4} notation, maybe in shell this is supported? However, in JavaScript there should be no problem and we can change to shorter notation.

jurion.js Outdated
item.date = ZU.strToISO(date);
}
var court = getValueToLabel(doc, "Gericht");
if (court) {

This comment has been minimized.

@adam3smith

adam3smith Jan 30, 2017

Collaborator

for all of thes -- why create a new variable and test for them?
item.court = getValueToLabel(doc, "Gericht"); is all you need. Just taking about court, docket Number, and form here. I understand you need the test for the others.

item.title = caseName;
}

//bookSection

This comment has been minimized.

@adam3smith

adam3smith Jan 30, 2017

Collaborator

so right now you're running through all the xpaths for every item type. Wouldn't it make more sense to do a if (type == "book Section" around this section and ditto for the others?

This comment has been minimized.

@zuphilip

zuphilip Jan 30, 2017

Author Collaborator

I would rather not do "exclusivity" for them, if not needed. Maybe, the labels for two types are the same and then also have the same meaning. If any label has to be handled differently for different types, then I would start to make the distinction.

@zuphilip

This comment has been minimized.

Copy link
Collaborator Author

zuphilip commented Jan 30, 2017

Please have a look at the new version.

@adam3smith adam3smith merged commit f1d135f into zotero:master Feb 7, 2017

1 check passed

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

This comment has been minimized.

Copy link
Collaborator

adam3smith commented Feb 7, 2017

thanks!

@zuphilip zuphilip deleted the zuphilip:jurion branch Feb 7, 2017

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

Add new translator for jurion (zotero#1241)
This is a German law portal with journal articles,
cases, commentaries, ebooks, statutes. The access
is restricted to subscribers.

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

Add new translator for jurion (zotero#1241)
This is a German law portal with journal articles,
cases, commentaries, ebooks, statutes. The access
is restricted to subscribers.
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.