-
Notifications
You must be signed in to change notification settings - Fork 84
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
Fix broken getCiteKey() resulting from recent BBT update #225
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. Thank you!
Before this PR gets approved, you can simply clone the repo, manually add the changes that @RoBaaaT made (just two lines. Thank you for this important fix!) and execute the following commands: |
@retorquere Thanks for your suggestion! However that would not work for users who still have older BBT versions with
That way we do not even have to check for the existence of |
I'm trying to think of a situation where it wouldn't, as In any case I would prefer it very much if people would not go around encouraging others to run outdated versions of BBT. Zotero auto-updates plugins by default for a reason.
I don't think But come to think of it -- Zotero already has citationKey support for itemtypes
because
That already works, and the try-catch method doesn't even have to know or care about BBT being present, now and in the future. |
'citekey' has been renamed to 'citationKey' in BBT. See retorquere/zotero-better-bibtex@63028dc
Thanks for the detailed feedback! Using try-catch indeed seems like the best way to do it. I've updated the pull request accordingly. |
This will not work as src/markdown-utils.js cannot be found when zipping the folder directly after applying the change. Thanks for the fix :) |
you can take the xpi, unzip that (xpi's are just zipfiles), make the change, and re-zip the xpi. |
I tried this and it works perfectly now. What's the timeline for the PR getting accepted? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. It would be better if the xpi file is uploaded in Releases directly.
Thank you, RoBaaaT, for your excellent work. Could author please update this fix in the Releases? It's not easy to access this fix at the moment. |
Tried implementing
Hi, I tried implementing this. (downloaded .xpi, renamed .zip, changed getCitekey to try-catch, and reconverted back to .xpi), but this did not work. Did i miss/skip a step? Thanks! |
Can't say, sorry. As far as I can tell, that should work. You'd need @argenos to get involved. |
A recent change in the BetterBibTex plugin (retorquere/zotero-better-bibtex@63028dc, included in version 6.7.132, released on 2023-10-23) renamed 'citekey' to 'citationKey' in the BetterBibTex source code. This broke the
getCiteKey()
function in zotero-mdnotes, resulting in exports failing when the 'Use the item's citekey as title' option was enabled by the user (see #224).This change fixes the problem by updating
getCiteKey()
to access 'citationKey' instead of 'citekey' if 'citekey' is undefined.PS: Sorry about the whitespace changes, those were done by my editor. I can resubmit the pull request without them if you prefer.