-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
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.
Pinging @pazos, nothing jumps out to me besides the nit.
@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:
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 :) |
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!
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. |
b359b05
to
216eeb3
Compare
I'm very sorry, @kotnik. I forgot completely about this PR 😭
That's completely fine!
Not required, current behaviour works fine.
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.
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. |
plugins/exporter.koplugin/main.lua
Outdated
@@ -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" } |
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.
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"
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.
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
216eeb3
to
5bc640b
Compare
As requested, here is Nextcloud Notes support for exporting notes and highlights.
Fixes #11522
TODO:
This change is