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 upAdd new translator for jurion #1241
Conversation
adam3smith
requested changes
Jan 30, 2017
|
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. |
| 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.
This comment has been minimized.
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.
This comment has been minimized.
|
|
||
| var author = getValueToLabel(doc, "Autor") || getValueToLabel(doc, "Verfasst von"); | ||
| if (author) { | ||
| author = author.replace(/Dr\.?|Prof\.?|RA\.?/g, ''); |
This comment has been minimized.
This comment has been minimized.
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.
This comment has been minimized.
|
|
||
| var ref = getValueToLabel(doc, "Referenz"); | ||
| if (ref) { | ||
| var m = ref.match(/(\d\d\d\d),\s+([\d\s\-IVX]*)\((.*)\)/); |
This comment has been minimized.
This comment has been minimized.
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.
This comment has been minimized.
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.
| item.date = ZU.strToISO(date); | ||
| } | ||
| var court = getValueToLabel(doc, "Gericht"); | ||
| if (court) { |
This comment has been minimized.
This comment has been minimized.
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.
This comment has been minimized.
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.
This comment has been minimized.
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.
This comment has been minimized.
This comment has been minimized.
|
Please have a look at the new version. |
adam3smith
merged commit f1d135f
into
zotero:master
Feb 7, 2017
1 check passed
This comment has been minimized.
This comment has been minimized.
|
thanks! |
zuphilip
deleted the
zuphilip:jurion
branch
Feb 7, 2017
zuphilip
added a commit
to zuphilip/translators
that referenced
this pull request
Mar 28, 2018
zuphilip
added a commit
to zuphilip/translators
that referenced
this pull request
Mar 28, 2018
zuphilip commentedJan 28, 2017
This is a German law portal with journal articles,
cases, commentaries, ebooks, statutes. The access
is restricted to subscribers.