fuxialexander / org-pdftools

A custom org link type for pdf-tools
GNU General Public License v3.0
335 stars 36 forks source link

Enabling org-noter-pdftools breaks org-noters compatability with nov.el #38

Closed TimQuelch closed 3 years ago

TimQuelch commented 4 years ago

I want to annotate both PDFs and EPUBs with org-noter, but it looks like when I enable org-noter-pdftools that annotations made in EPUBs when using nov mode no longer contain the location property.

I haven't had too much of a deep dive into the code of org-noter or org-noter-pdftools, but I suspect it is because the functions org-noter-pdftools uses to override the default functionality of org-noter dont' properly handle when the document buffer is in nov-mode rather than pdf-view-mode.

samwhitlock commented 3 years ago

I'm running into the same issue: org-noter-pdftools sets a bunch of hooks for org-noter, but those don't seem to either check if the mode is indeed pdfview or be mode/buffer-specific.

I get warnings for org-noter-pdftools--pretty-print-location whenever I scroll the nov.el buffer, for instance.

bulbousBullfrog commented 3 years ago

I ran into the same problem. I was getting an error when trying to insert a note within an EPUB. I saw that it is happening when org-noter--doc-approx-location-hook is being called, indicating as samwhitlock said, that the hooks don't consider which mode (pdf or epub) is currently in use. These are how the hooks are written right now.

dolist (pair '((org-noter--check-location-property-hook   . org-noter-pdftools--check-link)
                (org-noter--parse-location-property-hook   . org-noter-pdftools--parse-link)
                (org-noter--pretty-print-location-hook     . org-noter-pdftools--pretty-print-location)
                (org-noter--convert-to-location-cons-hook  . org-noter-pdftools--convert-to-location-cons)
                (org-noter--doc-goto-location-hook         . org-noter-pdftools--doc-goto-location)
                (org-noter--note-after-tipping-point-hook  . org-noter-pdftools--note-after-tipping-point)
                (org-noter--relative-position-to-view-hook . org-noter-pdftools--relative-position-to-view)
                (org-noter--get-precise-info-hook          . org-noter-pdftools--get-precise-info)
                (org-noter--doc-approx-location-hook       . org-noter-pdftools--doc-approx-location)
                (org-noter-insert-heading-hook             . org-noter-pdftools--insert-heading)))
  (add-hook (car pair) (cdr pair)))

I'm unfamiliar with elisp, but it doesn't seem like it should be difficult to make the assigning of the hooks conditional on the document mode type