> (define-obsolete-variable-alias 'doc-view-unoconv-program
> - 'doc-view-odf->pdf-converter-program
> - "24.4")
> + 'doc-view-odf->pdf-converter-program
> + "24.4")
Thanks. At this point, the last two args can be placed on the same
line ;-)
This is also due to the auto-indent, but I will correct it manually.
> (defun doc-view-new-window-function (winprops)
> + ;; TODO: write documentation!
> ;; (message "New window %s for buf %s" (car winprops) (current-buffer))
> (cl-assert (or (eq t (car winprops))
> (eq (window-buffer (car winprops)) (current-buffer))))
I plead guilty, but if you give me a more specific question, I'll find
it easier to answer it (even in the form of a docstring or comment).
This I copied from pdf-tools. However, I am planning to enter this documentation
myself, while I am implementing real continuous scroll. I have got that working in
pdf-tools already, but I had to disable all kinds of minor modes there. So I thought
let's start with a simpler task of implementing it in doc-view, however, the same
difficulty, that I encountered also in pdf-tools is that somehow
`bookroll-mode-winprops-alist` (which is exactly the equivalent/copy of
`bookroll-mode-winprops-alist`) obtains a value `t`. I think I've spent some hours
already investigating where it obtains it (somehow I fixed it after debugging for
pdf-tools for few hours, but clearly I do not understand how, and have to repeat the
same things again, well I guess it is a good exercise. Anyway, in the end I hope to
add some useful documentation to prevent other hackers from 'wasting' their time :)
Anyway, if I can keep it like this for now, that would be great, as the TODO really
jumps out font lock wise, and also I can search for it.
> @@ -738,7 +740,7 @@ doc-view-kill-proc
> (interactive)
> (while (consp doc-view--current-converter-processes)
> (ignore-errors ;; Some entries might not be processes, and maybe
> - ;; some are dead already?
> + ;; some are dead already?
> (kill-process (pop doc-view--current-converter-processes))))
> (when doc-view--current-timer
> (cancel-timer doc-view--current-timer)
Here the auto-indentation gets it wrong, sadly.
You mean because it aligns with the line below so that it looks like it belongs
to that one? Because otherwise, I think it does not look bad.
> + ((memq type '(postscript ps eps pdf epub))
> + (if-let (command (and (memq type '(pdf epub)) (executable-find "mutool")))
> + command
> + (unless (eq type 'epub)
> + (and doc-view-ghostscript-program
> + (executable-find doc-view-ghostscript-program)))))
> ((eq type 'odf)
> (and doc-view-odf->pdf-converter-program
> (executable-find doc-view-odf->pdf-converter-program)
Hmm... for PDF files, this changes the previous behavior where we
checked for the presence of `doc-view-pdfdraw-program` rather than "mutool".
Was there a strong reason to hardcode "mutool" here
Ah yes, I was aware of that, but indeed I did that consciously. I have very good experiences
with (py)mupdf (I have implemented an alternative pdf-tools server using it, and indeed it is
much faster than the epdfinfo server, well at least the mupdf vs poppler parts.
really take a very quick look at the two histograms in this
link.
So I decided this behavior really wouldn't be a problem, why not prefer mutool when it is
available?
Also, I think the rest of the code will still launch
`doc-view-pdfdraw-program` rather than `mutool`, so there's
something fishy.
That I do not understand immediately, for me the behavior is still somewhat opaque. But I
will be trying to understand things further today (anyway for implementing the continuous scroll)
and try to understand how it all works (and correct it here).
This error message needs to be shorter. It probably shouldn't talk
about `mutool`. Even with this simplification I think we're going to
have to be creative to make it short enough to fit in a single line of
80 columns. We should probably just say something like
doc-view-pdf->png-converter-function invalid for EPub
and then add in the docstring of `doc-view-pdf->png-converter-function`
a note about the fact that only `doc-view-pdf->png-converter-mupdf`
supports EPub.
That sounds good indeed. I will take care of that.
Thanks, for the feedback... I will try another 'round' probably sooner than later :)