Maintenance/pdf modularization

[petermao]

I'm trying to move all pdf-tools-specific functions out of -core.el and into modules/-pdf.el. The first 3 commits are just renames so everything in the modules start with org-noter-<module>--.... 46e639a moves the -show-arrow function, makes a hook and adds the moved function to the hook in the usual way.

In 7bd1145, I take the (require 'pdf-tools) out of -core.el and it breaks the packaged version of org-noter, but not my local copy. Specifically, the arrow stops showing up and emacs claims it knows nothing about the pdf-tools function pdf-view-current-overlay.

In be25512, the explicit call outside of condition-case fixes the "bug," but that subverts the intent of the condition-case that was brought in at PR #13 to fix issue #9.

1aaea12 shows that the calls to declare-function have no effect on the behavior of the code.

[petermao]

If I go to the first "buggy" commit, 7bd1145, and byte-compile modules/org-noter-pdf.el, then the arrow doesnt work, with message:
Error running timer ‘org-noter--show-arrow’: (invalid-function pdf-view-current-overlay)
But show-arrow has no problems with the interpreted lisp file!

Compilation only produces warnings:

Compiling file /<path-to>/org-noter/modules/org-noter-pdf.el at Sun Apr 30 11:08:48 2023
Entering directory ‘/<path-to>/org-noter/modules/’
org-noter-pdf.el:35:1: Warning: variable ‘_’ not left unused

In org-noter-pdf--get-buffer-file-name:
org-noter-pdf.el:68:55: Warning: Unused lexical argument `mode'
org-noter-pdf.el:398:1: Warning: variable ‘_’ not left unused

no complaints against pdf-tools functions during compilation!

[petermao]

So my current understanding is that the (require 'pdf-tools) in the condition-case at

org-noter/modules/org-noter-pdf.el

Lines 29 to 32 in 3856c1b

	(condition-case nil
	(require 'pdf-tools)
	(require 'pdf-annot)
	(error (message "ATTENTION: org-noter-pdf has many featues that depend on the package `pdf-tools'")))

is not being executed in the byte-compiled code. What to do?

[petermao]

@dmitrym0 @terlar @pepetr @uliw @rdiaz02 please pull and test this branch, as you are known stake-holders in the changes being implemented.

[pepetr]

Works fine here, thanks a lot !

[dmitrym0]

@petermao I can't seem to reproduce this. Can you tell me what you need to do to get this error?

I manually invoke org-noter--show-arrow from your branch and it seems to work without errors.

My org-noter-pdf.el is byte compiled too:

[petermao]

@petermao I can't seem to reproduce this. Can you tell me what you need to do to get this error?

which commit were you on? if you're on the last one, it should work fine. If you check out 7bd1145, then what I find is that the byte-compiled file won't show arrows.

[rdiaz02]

Works fine here too (I could only do limited testing, though).

[dmitrym0]

which commit were you on? if you're on the last one, it should work fine. If you check out 7bd1145, then what I find is that the byte-compiled file won't show arrows.

I tried 7bd1145. Both org-noter-pdf--get-curent-view and org-noter--show-arrow show up for me (at least in describe function).

[petermao]

With 7bd1145, when I run the compiled modules/org-noter-pdf.elc, no arrow shows up. When I run the uncompiled modules/org-noter-pdf.el, it works fine. The last commit should work in either case.

I think it's stable enough to merge. We can revisit my ugly code if you (@dmitrym0) find a cleaner way to implement this:

org-noter/modules/org-noter-pdf.el

Lines 29 to 35 in d541042

	(eval-when-compile ; ensure that the compiled code knows about PDF-TOOLS, if installed
	(condition-case nil
	(require 'pdf-tools)
	(error (message "`pdf-tools' package not found"))))
	(condition-case nil ; inform user at run time if pdf-tools is missing
	(require 'pdf-tools)
	(error (message "ATTENTION: org-noter-pdf has many featues that depend on the package `pdf-tools'")))