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 upGet citekey from future citationKey or extract from extra field #1814
Conversation
adam3smith
requested changes
Jan 30, 2019
A couple of questions. Also, you need to bump timestamps, please. |
citekey = buildCiteKey(item, citekeys); | ||
} | ||
var citekey = buildCiteKey(item, extraFields, citekeys); | ||
var extraFields = item.extra ? parseExtraFields(item.extra) : null; |
This comment has been minimized.
This comment has been minimized.
adam3smith
Jan 30, 2019
Collaborator
I always get confused in how JS handles variables, but for clarity we should define the variable before we use it, i.e. flip these two lines.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -1270,7 +1275,8 @@ function doExport() { | |||
if (!type) type = "misc"; | |||
|
|||
// create a unique citation key | |||
var citekey = buildCiteKey(item, citekeys); | |||
var extraFields = item.extra ? parseExtraFields(item.extra) : null; |
This comment has been minimized.
This comment has been minimized.
adam3smith
Jan 30, 2019
Collaborator
here you have them in the right order, so I assume that's just a typo above
@@ -386,7 +386,12 @@ function creatorCheck(item, ctype) { | |||
return false; | |||
} | |||
|
|||
function buildCiteKey(item, citekeys) { | |||
function buildCiteKey (item, extraFields, citekeys) { | |||
const citationKey = extraFields.findIndex(field => field.field && field.value && field.field.toLowerCase() === 'citation key') |
This comment has been minimized.
This comment has been minimized.
adam3smith
Jan 30, 2019
Collaborator
why is this citation key
and not citationkey
? (I might be missing something obvious.)
This comment has been minimized.
This comment has been minimized.
retorquere
Jan 30, 2019
Author
Contributor
As far as I can tell, https://github.com/zotero/translators/blob/master/BibTeX.js#L146 or anywhere after doesn't normalize the key, and according to #1810 (comment) the extra
field would be Citation Key
. Right? I did test and it seemed to work this way. I can add code to normalize the parsed field keys, but I don't know what else that would break.
This comment has been minimized.
This comment has been minimized.
adam3smith
Jan 30, 2019
Collaborator
we don't have a convention on this -- currently, all other fields we use Extra for, we just pick up the CSL variable name (for obvious reasons), but that doesn't exist here.
I don't have a strong view on how this should be named, but to keep it consistent with other fields in extra, should we name it what a CSL variable would be named, i.e. citation-key
?
How do you currently suggest/require people enter keys in Extra for BBT?
This comment has been minimized.
This comment has been minimized.
retorquere
Jan 30, 2019
Author
Contributor
I currently support bibtex:
(for backwards compatibility) and Citation Key:
(suggested by convention, because it's what BBT will add currently if you pin a key), but will follow any convention that Zotero proposes. If we settle for citation-key
here, I'll just adjust BBT to pin keys that way. A re-pin would change bibtex:
or Citation Key:
to citation-key
if present under that regime.
This comment has been minimized.
This comment has been minimized.
adam3smith
Jan 30, 2019
Collaborator
No need to multiply these then. Let's go with Citation Key:
i.e. keep the code as is.
This comment has been minimized.
This comment has been minimized.
retorquere
Jan 30, 2019
Author
Contributor
It's an easy change for me though, so if citation-key
is a better philosophical fit, that's a 5-minute change for me with no further consequences.
This comment has been minimized.
This comment has been minimized.
const citationKey = extraFields.findIndex(field => field.field && field.value && field.field.toLowerCase() === 'citation key') | ||
if (citationKey >= 0) return extraFields.splice(citationKey, 1)[0].value | ||
|
||
if (item.citationKey) return item.citationKey |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
retorquere
Jan 30, 2019
Author
Contributor
You mean outside the buildCiteKey function? Or something else? Sorry, I don't quite follow, can you elaborate?
This comment has been minimized.
This comment has been minimized.
retorquere
Jan 30, 2019
Author
Contributor
Oh wait, do you mean first item.citationKey
and then check the extra
field? My idea was that if the user took the effort to put it in the extra
field, that would always take precedence.
This comment has been minimized.
This comment has been minimized.
adam3smith
Jan 30, 2019
Collaborator
yes, that's what I meant. I'm fine with leaving it as you have here. item.citationKey
will never exists currently, though? Or has it already been generated here (sorry, didn't re-read code)? In that case, definitely go with the Extra solution as the first option as you have it.
This comment has been minimized.
This comment has been minimized.
retorquere
Jan 30, 2019
Author
Contributor
It's there in anticipation of citekeys arriving in Zotero as .citationKey
(as discussed in #1810 (comment)) but in the interim, BBT will make its cite key available there.
This comment has been minimized.
This comment has been minimized.
This would have the side benefit that citation keys the user specified (whether generated by BBT or not) would show up in Overleaf. |
retorquere
added some commits
Jan 30, 2019
adam3smith
merged commit a340b9b
into
zotero:master
Jan 31, 2019
1 check passed
This comment has been minimized.
This comment has been minimized.
Great; having the id in the CSL JSON for pandoc and having the citation key available via server API for Overleaf is going to be very useful. Thanks! |
retorquere commentedJan 29, 2019
No description provided.