Rewrite Encore #1482
Rewrite Encore #1482
Conversation
closes zotero#1481
Library Catalog (Encore).js
Outdated
| "tag": "Nursing" | ||
| }, | ||
| { | ||
| "tag": "Nursing" |
There was a problem hiding this comment.
Why do we have here duplicate tags?
There was a problem hiding this comment.
Look at the MARC here: http://sallypro.sandiego.edu/iii/encore/record/C__Rb1516899__Stesting__P0,2__Orightresult__U__X6?lang=eng&suite=cobalt&marcData=Y
in the 650 fields:
650 0 Nursing|xEducation
650 0 Critical thinking
650 0 Constructivism (Education)
650 0 Nursing|xDecision making
650 0 Nursing|xPhilosophy
MARC imports both parts of the field as tag -- maybe that's not right? Or we should save the category in the 0 field and check against it to avoid duplication? In any case, this is a MARC issue, I'd like to address it separately
There was a problem hiding this comment.
Okay, I see.
Although, I thought that duplicate tags were technically not possible. At least manually I cannot enter twice the same tag.
There was a problem hiding this comment.
Turns out they only show up in the tests, not in Zotero itself.
Library Catalog (Encore).js
Outdated
| } | ||
| ] | ||
| "url": "http://sallypro.sandiego.edu/iii/encore/record/C__Rb1516899__Stesting__P0%2C2__Orightresult__U__X6?lang=eng&suite=cobalt", | ||
| "items": [{ |
There was a problem hiding this comment.
Did you add these tests with Scaffold? The [{ is usually on two lines...
There was a problem hiding this comment.
of course tests are from scaffold, yes. I think this was jsbeautify which I ran on the whole file.
There was a problem hiding this comment.
Ah, okay. For test it makes IMO sense to leave the formatting as Scaffold outputs it. Otherwise this will show differences the next time someone is updating and testing the same translator.
Library Catalog (Encore).js
Outdated
| @@ -9,15 +9,14 @@ | |||
| "inRepository": true, | |||
| "translatorType": 4, | |||
| "browserSupport": "gcsb", | |||
| "lastUpdated": "2014-08-26 03:59:48" | |||
| "lastUpdated": "2017-11-19 20:03:16" | |||
There was a problem hiding this comment.
For a rewrite we should IMO increase the minVersion also to 3.0.
Library Catalog (Encore).js
Outdated
| @@ -9,15 +9,14 @@ | |||
| "inRepository": true, | |||
| "translatorType": 4, | |||
| "browserSupport": "gcsb", | |||
There was a problem hiding this comment.
Add all of them (or do you have any reasons that they should not work?).
Library Catalog (Encore).js
Outdated
| for (var i in items) { | ||
| articles.push(i); | ||
| } | ||
| scrape(articles) |
Library Catalog (Encore).js
Outdated
| scrape(articles) | ||
| }); | ||
| } else { | ||
| var marcURL = createMarcURL(url) |
Library Catalog (Encore).js
Outdated
| var record = new marc.record(); | ||
| var newItem = new Zotero.Item(); | ||
|
|
||
| var linee = text.split("\n"); |
There was a problem hiding this comment.
Fix typo linee (or do I miss the point in the variable name here?).
Library Catalog (Encore).js
Outdated
|
|
||
| var linee = text.split("\n"); | ||
| for (var i = 0; i < linee.length; i++) { | ||
| if (!linee[i]) { |
There was a problem hiding this comment.
Can we add comment here, e.g. // skip empty line?
Library Catalog (Encore).js
Outdated
|
|
||
| if (linee[i].substr(0, 6) == " ") { | ||
| // add this onto previous value | ||
| tagValue += value; |
There was a problem hiding this comment.
This is IMO quite confusing and tagValue is not yet initialized.
I suggest to actually clean the text before splitting into lines with something like:
// delete empty lines, delete confusing newlines, e.g.
// 245 10 Testing a theoretical model of critical thinking and
// cognitive development /|cby Jane Rapps
text = text.replace(/^\n/mg, '').replace(/\s?\n\s+/gm, ' ');
|
Please take a look. I switched out the test because the catalog always detects book and the old test was a thesis so tests failed (probably not worth fixing detect here) |
|
Thank you very much, @adam3smith ! (I simplified the code a little and add some nits.) |
|
Thanks! |
closes #1481