Skip to content
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

Add Nextcloud Notes to exporter plugin #12301

Merged
merged 5 commits into from
Nov 8, 2024
Merged

Conversation

kotnik
Copy link
Contributor

@kotnik kotnik commented Aug 5, 2024

As requested, here is Nextcloud Notes support for exporting notes and highlights.

Fixes #11522

TODO:

  • Create wiki page mentioned in help

This change is Reviewable

@kotnik kotnik marked this pull request as draft August 5, 2024 21:17
@kotnik kotnik marked this pull request as ready for review August 5, 2024 21:36
@Frenzie Frenzie added the Plugin label Aug 6, 2024
Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinging @pazos, nothing jumps out to me besides the nit.

plugins/exporter.koplugin/main.lua Outdated Show resolved Hide resolved
@pazos
Copy link
Member

pazos commented Aug 12, 2024

@kotnik: thank you very much for your time :)

I would like to have a few days to test the plugin myself. I'm short of time right now.

What I would like is feature parity w/ the joplin exporter but I'm not sure if it's already implemented or even doable (Didn't look at your code or at nextcloud api docs yet). I mean, among others:

  • ability to update a note if it was changed.
  • ability to ignore a note for a book if the same note exists on the server

This way implies the exporter works as a sync server and any past highlight or annotation is deleted from the server when the client exports/syncs it after deletion.

Please let me know if that's ok with you. If you prefeer some kind of archive without updating/deleting I think we can make it configurable :)

@kotnik
Copy link
Contributor Author

kotnik commented Aug 16, 2024

@kotnik: thank you very much for your time :)

I would like to have a few days to test the plugin myself. I'm short of time right now.

Thank you for the review. This is, obviously, not urgent at all and I am here to make it through. It's quite fun to code Lua!

What I would like is feature parity w/ the joplin exporter but I'm not sure if it's already implemented or even doable (Didn't look at your code or at nextcloud api docs yet). I mean, among others:

* ability to update a note if it was changed.

* ability to ignore a note for a book if the same note exists on the server

This way implies the exporter works as a sync server and any past highlight or annotation is deleted from the server when the client exports/syncs it after deletion.

Please let me know if that's ok with you. If you prefeer some kind of archive without updating/deleting I think we can make it configurable :)

Yeah, I looked into Joplin, and this (the first) iteration works by uploading every note/annotation to Nextcloud. If the note from the same title already exists it will be updated (version changes can be configured on the Nextcloud side). Notes are tagged with KOReader tag, which in Nextcloud means they will be put in separate directory, so it is easy to work with them.

I can add an option to update or skip if existing note already exists in Nextcloud. I'd appreciate testing as well, it works fine with my local and remote Nextcloud instances, but I am not super sure how it would work with big number of notes.

@kotnik kotnik force-pushed the nc_notes_export branch 2 times, most recently from b359b05 to 216eeb3 Compare August 16, 2024 10:43
@pazos
Copy link
Member

pazos commented Oct 7, 2024

I'm very sorry, @kotnik. I forgot completely about this PR 😭

Yeah, I looked into Joplin, and this (the first) iteration works by uploading every note/annotation to Nextcloud. If the note from the same title already exists it will be updated (version changes can be configured on the Nextcloud side). Notes are tagged with KOReader tag, which in Nextcloud means they will be put in separate directory, so it is easy to work with them.

That's completely fine!

I can add an option to update or skip if existing note already exists in Nextcloud

Not required, current behaviour works fine.

. I'd appreciate testing as well, it works fine with my local and remote Nextcloud instances, but I am not super sure how it would work with big number of notes.

I've tested briefly and works fine, but I had to generate a bunch of notes to test. The best way for getting tests for free is to merge this and let hardcore hightlighters to use it.

TODO:

Create wiki page mentioned in help

If you want to add documentation before merging this be my guest ;). Otherwise I'll add some notes after merging.

Again, apologies for the late response.

@@ -40,7 +40,7 @@ local _ = require("gettext")

-- migrate settings from old "evernote.koplugin" or from previous (monolithic) "exporter.koplugin"
local function migrateSettings()
local formats = { "flomo", "html", "joplin", "json", "memos", "my_clippings", "readwise", "text", "xmnote" }
local formats = { "flomo", "html", "joplin", "json", "memos", "my_clippings", "nextcloud_notes", "readwise", "text", "xmnote" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT this plugin cannot export to any other Nextcloud facilities other than "Nextcloud Notes". If so, please use the keyword "nextcloud". References to it on the UI are ok as "Nextcloud notes"

Copy link
Member

@pazos pazos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is totally fine as is:

I'm trying to figure out if my requested changes are some kind of OCD or not. Feedback welcome:

Requested changes:

use "nextcloud" instead of "nextcloud_notes" to refer to code and files, since all the integration with nextcloud, for this particular plugin, will have to happen thought the Nextcloud Notes APIs

@pazos pazos merged commit 7c166a2 into koreader:master Nov 8, 2024
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: Exporting your notes and highlights on Nextcloud Note
3 participants