diegodlh / zotero-cita

Cita: a Wikidata addon for Zotero with citations metadata support
GNU General Public License v3.0
234 stars 12 forks source link

#4 BibTeX import/export #118

Closed Dominic-DallOsto closed 3 years ago

Dominic-DallOsto commented 3 years ago

Work on enabling importing/exporting citations from BibTeX (#4).

Uses the Zotero Import Translator to convert from BibTeX to a Zotero item. We also get .ris support and more(?) for free - could change the option to be "import from file" or "import from clipboard"? Uses the Zotero_File_Exporter to export to a file, handling everything automatically.

Currently just reads from the clipboard, like Zotero's File > Import from Clipboard option. Would it be better to have a dialog box to paste the BibTeX into? To read from a file? Should we try to reconcile QIDs for all items on import? Otherwise it's quite tedious to do this item-by-item later.

I tested with the .ris citation list from here - works OK apart from the problem that the raw data isn't complete.

Extensions:

diegodlh commented 3 years ago

Work on enabling importing/exporting citations from BibTeX (#4).

This is so cool! This not only fixes #4, but also provides a workaround for #39 and #61, hence greatly simplifying adding citations and improving Cita's value. Thank you for all your work!

I'm sorry I couldn't reply to you earlier. I also regret not having had the time to implement automatic software tests yet (#30). I know it discourages making changes to the code and makes them so much more difficult :(

We also get .ris support and more(?) for free - could change the option to be "import from file" or "import from clipboard"?

What about "Import citations from clipboard"?

Would it be better to have a dialog box to paste the BibTeX into? To read from a file?

I think having a dialog box to paste into, with an option to read from a file and edit, would be ideal. But it may be more complex to code, and perfect is the enemy of good. I think "from clipboard" is OK for now, and we could open an issue to change it in the future. If you do implement the dialog box, though, what about having the option read "Import citations from file/clipboard", or just "Import citations"?

Should we try to reconcile QIDs for all items on import? Otherwise it's quite tedious to do this item-by-item later.

That would be useful, but you may also consider these alternatives:

Either way, I would suggest doing only exact matches if batch fetching QIDs for two or more citations; i.e., not partial matches. This is the default behavior of the Wikidata.reconcile method for two or more items anyway.

Uses the Zotero_File_Exporter to export to a file, handling everything automatically.

It's great to have other options than just BibTeX. However, I think that some translator options ("Export Notes", "Export Files", "Export Tags" and "Export Collections") that show for some formats may be confusing, as citations can't have notes, files or tags, or belong to specific collections (I'm not sure what the TEI translator option "Full TEI Document" means and whether it applies to citations, or what other options may be available for other export translators). What do you think?

I think the only way to remove these options would be to reimplement Zotero's Zotero_File_Export.save method, unfortunately. Maybe by changing translators' displayOptions array property here, or by using a custom exportOptions dialog here.

Alternatively, we may just skip format choice (maybe choose BibTeX by default), and copy to clipboard (instead of saving) for simplicity. But it would be so sad :'(

Depending on what choice we make, we should rename the option to either "Export citations to file" or "Export citations to clipboard as BibTeX".

I tested with the .ris citation list from here - works OK apart from the problem that the raw data isn't complete.

Tested it myself. I can only say I'm so happy this is working so well!

At first I was hesitant about having methods that added citations without checking for duplicates. In fact, the first and simplest method to add citations, that is via the Citation Editor, does check for duplicates before adding a new citation. On the other hand, the "sync citations with Wikidata" method compares QIDs of local cited items and remote "cites work" statement objects; but it doesn't compare other fields (title, DOI, etc).

Hence, at some point, I realized that proactively making sure that no duplicate citations were created might be too difficult, and so I shifted to having a method (not yet implemented) to retroactively check for and merge duplicate citations (#37).

Although I wouldn't wait until having #37 closed to merge this pull request, I think it underscores the need to fix it.

I will add some in-line comments to your code now. Once again, thank you so much for this contribution! I can't highlight enough how important I think it is!!

Dominic-DallOsto commented 3 years ago

Work on enabling importing/exporting citations from BibTeX (#4).

This is so cool! This not only fixes #4, but also provides a workaround for #39 and #61, hence greatly simplifying adding citations and improving Cita's value. Thank you for all your work!

I'm sorry I couldn't reply to you earlier. I also regret not having had the time to implement automatic software tests yet (#30). I know it discourages making changes to the code and makes them so much more difficult :(

No worries - thanks for developing Cita in the first place!

Would it be better to have a dialog box to paste the BibTeX into? To read from a file?

I think having a dialog box to paste into, with an option to read from a file and edit, would be ideal. But it may be more complex to code, and perfect is the enemy of good. I think "from clipboard" is OK for now, and we could open an issue to change it in the future. If you do implement the dialog box, though, what about having the option read "Import citations from file/clipboard", or just "Import citations"?

Ok. For the moment I just changed it to import from clipboard. It should be easy to add another option for importing from a file. But yeah, a nicer dialog box can maybe be a later improvement...

Should we try to reconcile QIDs for all items on import? Otherwise it's quite tedious to do this item-by-item later.

That would be useful, but you may also consider these alternatives:

* Add a "Fetch QIDs for citations" option to the item menu on the Citations Pane. This was proposed in [Batch fetch QID actions #38](https://github.com/diegodlh/zotero-cita/issues/38), but probably closed when the option to batch "Fetch QID(s)" for citing items was added to the item tree's Cita submenu.

* Auto fetch (citing and cited) QIDs before syncing citations with Wikidata ([Offer to fetch QIDs before syncing citations #36](https://github.com/diegodlh/zotero-cita/issues/36)).

Either way, I would suggest doing only exact matches if batch fetching QIDs for two or more citations; i.e., not partial matches. This is the default behavior of the Wikidata.reconcile method for two or more items anyway.

Ok, I'll have to think more about this - either of those solutions sounds good. Maybe it would be better not to reconcile automatically if a user wants use the extension offline.

Uses the Zotero_File_Exporter to export to a file, handling everything automatically.

It's great to have other options than just BibTeX. However, I think that some translator options ("Export Notes", "Export Files", "Export Tags" and "Export Collections") that show for some formats may be confusing, as citations can't have notes, files or tags, or belong to specific collections (I'm not sure what the TEI translator option "Full TEI Document" means and whether it applies to citations, or what other options may be available for other export translators). What do you think?

I think the only way to remove these options would be to reimplement Zotero's Zotero_File_Export.save method, unfortunately. Maybe by changing translators' displayOptions array property here, or by using a custom exportOptions dialog here.

Alternatively, we may just skip format choice (maybe choose BibTeX by default), and copy to clipboard (instead of saving) for simplicity. But it would be so sad :'(

Depending on what choice we make, we should rename the option to either "Export citations to file" or "Export citations to clipboard as BibTeX".

Yeah, I guess the extra options should be harmless, but definitely a bit confusing. I'll look at how the menu can be modified. An extra option to export BibTeX to clipboard might be convenient too, at the risk of having too many.

Thanks for the comments! I went through and hopefully addressed them all, except for creating a new Citation from a jsonItem. Feel free to make your own changes, but otherwise I'll work through the remaining changes.

diegodlh commented 3 years ago

For the moment I just changed it to import from clipboard (...) a nicer dialog box can maybe be a later improvement...

Opened #124 to keep track of this.

either of those solutions sounds good. Maybe it would be better not to reconcile automatically if a user wants use the extension offline.

I think I prefer having the button to batch fetch QIDs for citing items (#38). It would offer a workaround for both #36 and this (i.e., auto reconcile on import), and may be used in other cases as well. I've reopened #38.

Yeah, I guess the extra options should be harmless, but definitely a bit confusing. I'll look at how the menu can be modified.

Again here I would support a good is better than perfect solution. Opened #125 to keep track of this.

An extra option to export BibTeX to clipboard might be convenient too, at the risk of having too many.

I agree it may be too many options in the Citations Pane menu. I meant having export to clipboard as an alternative to export to file if we decided to reimplement Zotero_File_Export.save and wanted to avoid the complexity of using the file picker, etc.

If we redesign the export dialog to hide the confusing translator options mentioned above, we could have two buttons: one to save to a file and another to copy to the clipboard. Included this in #125.

Thanks for the comments! I went through and hopefully addressed them all, except for creating a new Citation from a jsonItem. Feel free to make your own changes, but otherwise I'll work through the remaining changes.

Cool! I've replied to your comment about creating a new Citation from a jsonItem. Hope it helps. Thank you so much once again!!

Dominic-DallOsto commented 3 years ago

I just added the option to batch fetch QIDs for citations, and also for a single citation (not sure if needed?). The save handler for the citation ItemWrappers didn't work, so I had to manually disable saving and them save the source item at the end. Not sure if this is correct?

I tested again with importing the .ris reference list from here (Q108045069), the batch fetching the QIDs. This was a bit slow (5-10 s for 67 items) but worked.

Would it be nice to have the progress message be "XX QIDs fetched" if more than one item is provided for reconciliation?

diegodlh commented 3 years ago

I just added the option to batch fetch QIDs for citations, and also for a single citation (not sure if needed?). The save handler for the citation ItemWrappers didn't work, so I had to manually disable saving and them save the source item at the end. Not sure if this is correct?

Looks awesome! Thank you!! I've fixed the problem with the save handler for the citation target in acb1211aa6f0312ca083be4366e0e3b3b87b064e. Anyway, I've changed your code to use batch mode so the source item doesn't have to be saved each time a target QID is set.

Would it be nice to have the progress message be "XX QIDs fetched" if more than one item is provided for reconciliation?

I like the idea. This would imply changing how Wikidata.reconcile handles the progress window. As you said, it would be for when more than one item is provided for reconciliation, and when approximate matches are disabled (default behavior for two or more items). I would suggest that you open a separate issue, though, so we can merge this without further changes. What do you think?

One question about importing citations. In citation-importer/index.js you wrote "todo: update with zotero/filePicker - discussion about this here: https://groups.google.com/g/zotero-dev/c/a1IPUJ2m_3s". Could you elaborate further on this point? Do you think we would have to address that before merging this PR? Or is this something we could fix later?

Finally, I'm going to push one or two more commits now, with Spanish and "documentation" (qqq) translations, and I think we should be ready to merge. What do you think? I would try and release a new version later this week, although I may wait a few days to roll out automatic updates, just in case.

I don't have an official way to communicate news about Cita. I could show an update message, but I would have to look into how to do it neatly, and I don't want to keep postponing this. I will just twit something. Do you use Twitter? Can I mention you in my twit?

Dominic-DallOsto commented 3 years ago

I would suggest that you open a separate issue, though, so we can merge this without further changes. What do you think?

Yep, I added #128.

One question about importing citations. In citation-importer/index.js you wrote "todo: update with zotero/filePicker - discussion about this here: https://groups.google.com/g/zotero-dev/c/a1IPUJ2m_3s". Could you elaborate further on this point? Do you think we would have to address that before merging this PR? Or is this something we could fix later?

In 2019 Zotero updated from using firefox 52 to firefox 60 as its base, which resulted in a number of changes as mentioned in that forum post. Specifically, the nsIFilePicker.show() method was replaced by nsIFilePicker.open(), but it was advised to use a newly added FilePicker zotero module instead. This is shown here, with a backwards compatible version here.

According to the comment from Emiliano Heyns in that google groups thread, it's not possible to use the new FilePicker when using webpack, because the module needs to be available at build time and this seemingly isn't the case for 'zotero/filePicker'. I experienced the same - I couldn't get any code to compile that used the new FilePicker. Therefore, I used the same solution as Emiliano and stuck with the old nsIFilePicker.

According to Dan Stillman in that same thread, at some point this will likely need to be changed to use the new FilePicker. But I imagine only when Zotero updates to a newer version of firefox than 60, and this doesn't seem like it will happen super soon.

So, I think it's fine for now, but might need to be changed at some point in the future. We could make an issue with this information though?

Finally, I'm going to push one or two more commits now, with Spanish and "documentation" (qqq) translations, and I think we should be ready to merge. What do you think? I would try and release a new version later this week, although I may wait a few days to roll out automatic updates, just in case.

Looks good! Adding citations by identifiers should be good to go too (might need a lot of the same small changes you made here though). But could also wait until after the dust has settled from this update - whatever you think.

I don't have an official way to communicate news about Cita. I could show an update message, but I would have to look into how to do it neatly, and I don't want to keep postponing this. I will just twit something. Do you use Twitter? Can I mention you in my twit?

It seems possible to add release notes to the add-ons section in Zotero. I just updated my Better BibTeX extension and it had a section for release notes, but then they failed to load. So not sure if this works / would be easy to implement. I don't use Twitter sorry, but feel free to mention me if that works. But no worries if not.

diegodlh commented 3 years ago

the module needs to be available at build time and this seemingly isn't the case for 'zotero/filePicker'.

Thanks for the thorough explanation. I think I came across similar situations trying to import some Zotero's JSX components. I managed to import zotero/filePicker doing import FilePicker from 'zotero@zotero/filePicker'; and adding 'zotero@zotero/filePicker': 'commonjs zotero/modules/filePicker' to the externals array in webpack.config.js. Could you give it a try, so we don't leave an open issue?

Adding citations by identifiers should be good to go too (might need a lot of the same small changes you made here though). But could also wait until after the dust has settled from this update - whatever you think.

Yes! I'll try and check that now, if life allows. Thanks!

It seems possible to add release notes to the add-ons section in Zotero. I just updated my Better BibTeX extension and it had a section for release notes, but then they failed to load. So not sure if this works / would be easy to implement.

I'm going to check that. Thanks!

Dominic-DallOsto commented 3 years ago

Thanks, that seems to work now. Do we want to also keep backwards compatibility with the FF52 version of Zotero like Zotfile does, or not worry about it given this version is ~2 years old?

diegodlh commented 3 years ago

Thank you very much for all your work on this, Dominic! And my apologies once again for taking so long to review and approve your changes.

I had to make one final tweak because of a change introduced in Zotero's beta channel.

Do we want to also keep backwards compatibility with the FF52 version of Zotero like Zotfile does, or not worry about it given this version is ~2 years old?

I haven't checked compatibility with older versions of Zotero while developing Cita, and I'm sure there must be other things which are not compatible already. So let's not worry about this.

As I said before, I think your changes greatly improve Cita's usability and I will try and release a new version today or tomorrow including this and other changes. I think I'm gonna mark it as pre-release and wait to change the update.rdf until we have tested it a little bit.

Next big step should be addressing #30 so future contributions can be more straight-forward. Not an easy one, though :/

Thank you so much!!