tecosaur / org-latex-preview-todos

A tracker for the Org LaTeX Preview effort on https://git.tecosaur.net/tec/org-mode
4 stars 0 forks source link

Old API in org-compat is non-funcitonal #34

Open 9viz opened 3 weeks ago

9viz commented 3 weeks ago

The function org-create-formula-image is not functional because of a few factors (so far):

  1. org--get-display-dpi is missing from org-compat
  2. org-preview-latex-process-alist is an alias to the new variable which breaks the function down the line due to missing new format specifications (%l, %L)
  3. the function org-latex-color-format cannot handle COLOR-NAME='auto

This function is still in use by pdf-tools to create LaTeX previews for text annotations which contain LaTeX code. The old preview stuff is also used by comint-mime to preview LaTeX code emitted by python packages.

karthink commented 2 weeks ago

2. org-preview-latex-process-alist is an alias to the new variable which breaks the function down the line due to missing new format specifications (%l, %L)

The new variable supports %l and %L though, so I'm not able to spot problem 2.

karthink commented 2 weeks ago

3. the function org-latex-color-format cannot handle COLOR-NAME='auto

Could you point me to where/when this was added?

EDIT: I've been trying to understand this requirement. Do you mean setting the :foreground to auto in org-format-latex-options? This is supported (and the default) in the new, aliased variable org-latex-preview-appearance-options.

9viz commented 2 weeks ago
  1. org-preview-latex-process-alist is an alias to the new variable which breaks the function down the line due to missing new format specifications (%l, %L) The new variable supports %l and %L though, so I'm not able to spot problem 2.

The problem happens because org-create-formula-image does not pass these specs to org-compile-file in the let* block:

(let* ((err-msg (format "Please adjust `%s' part of \
`org-preview-latex-process-alist'."
            processing-type))
       (image-input-file
    (org-compile-file <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
     texfile latex-compiler image-input-type err-msg log-buf))
       (image-output-file
    (org-compile-file
     image-input-file image-converter image-output-type err-msg log-buf
     `((?D . ,(shell-quote-argument (format "%s" dpi)))
       (?S . ,(shell-quote-argument (format "%s" (/ dpi 140.0))))))))

and the default value of the new variable org-latex-preview-process-alist contains %l and %L for dvisvgm. So as long as the variable alias is removed, things should work automatically here.

  1. the function org-latex-color-format cannot handle COLOR-NAME='auto

Could you point me to where/when this was added?

EDIT: I've been trying to understand this requirement. Do you mean setting the :foreground to auto in org-format-latex-options? This is supported (and the default) in the new, aliased variable org-latex-preview-appearance-options.

org-create-formula-image has the block

(if (memq fg 'default)
    (setq fg (org-latex-color :foreground))
  (setq fg (org-latex-color-format fg)))

this assumes that FG=auto can be handled by org-latex-color-format (actually org-latex-preview--format-color now) which is not the case. So the default value of :foreground in org-format-latex-options breaks the old code. The value 'auto is handled specially by the new code. I hope I made it clear. Simply witness

(org-latex-color-format 'auto)

throwing an error.

Sorry for the late reply. I have limited internet connection outside of working hours so I could not reply to your questions in full in the weekend.

9viz commented 2 weeks ago

BTW, here are the changes I made to get org-create-formula-image before I gave up upon realising that many more things that I have no idea about needed to be changed to make it not signal an error. I am posting this in hopes that it will be helpful.


diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index 9f97b17..e08d77f 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -931,6 +931,14 @@ (defun org-place-formula-image (link block-type beg end value overlays movefile
                'org-latex-src-embed-type
                (if block-type 'paragraph 'character))))))

+(defun org--get-display-dpi ()
+  "Get the DPI of the display.
+The function assumes that the display has the same pixel width in
+the horizontal and vertical directions."
+  (if (display-graphic-p)
+      (round (/ (display-pixel-height)
+       (/ (display-mm-height) 25.4)))
+    (error "Attempt to calculate the dpi of a non-graphic display")))

 ;; FIXME: Unused; obsoleted; to be removed.
 (defun org-create-formula-image
@@ -989,7 +997,7 @@ (defun org-create-formula-image
     (resize-mini-windows nil)) ;Fix Emacs flicker when creating image.
     (dolist (program programs)
       (org-check-external-command program error-message))
-    (if (eq fg 'default)
+    (if (memq fg '(default auto))
    (setq fg (org-latex-color :foreground))
       (setq fg (org-latex-color-format fg)))
     (setq bg (cond
@@ -1017,7 +1025,8 @@ (defun org-create-formula-image
                processing-type))
       (image-input-file
        (org-compile-file
-        texfile latex-compiler image-input-type err-msg log-buf))
+        texfile latex-compiler image-input-type err-msg log-buf
+             `((?l . "pdflatex"))))
       (image-output-file
        (org-compile-file
         image-input-file image-converter image-output-type err-msg log-buf
karthink commented 1 week ago

Thank you for the detailed explanations. The first two issues should be fixed in the next set of patches.

The third one still confuses me. auto has always been an allowed value for the :foreground in org-format-latex-options, so it was always possible for org-create-formula-image to error. The behavior of org-latex-color-format when passed auto is unchanged from before this patchset. So how was it working before?

9viz commented 1 week ago

My copy of org-compat.el has the following

(define-obsolete-function-alias
  'org-latex-color-format 'org-latex-preview--format-color "9.7")
(define-obsolete-function-alias
  'org-latex-color 'org-latex-preview--attr-color "9.7")

the function org-latex-preview--attr-color does not accept 'auto' as an argument leading to the error. :foreground and :background being 'auto' is specially handled by org-latex-preview--face-around (in org-latex-preview--colors-around). But you know this already. Special handling is also done for the 'auto' value in org-format-latex in the old code. If you ask how it never errored, org-format-latex actually updates the value of :foreground and :background to not have 'auto' when it passes OPTIONS to org-create-formula-image deep in the code.

(let (...
  ;; Get the colors from the face at point.
  (fg
   (let ((color (plist-get org-format-latex-options
               :foreground)))
     (if forbuffer
     (cond
      ((eq color 'auto)
       (face-attribute face :foreground nil 'default))
      ((eq color 'default)
       (face-attribute 'default :foreground nil))
      (t color))
       color)))
  (bg
   (let ((color (plist-get org-format-latex-options
               :background)))
     (if forbuffer
     (cond
      ((eq color 'auto)
       (face-attribute face :background nil 'default))
      ((eq color 'default)
       (face-attribute 'default :background nil))
      (t color))
       color)))
  ...
  (options
   (org-combine-plists
    org-format-latex-options
    `(:foreground ,fg :background ,bg))))
  ...
  (unless (file-exists-p movefile)
(org-create-formula-image
 value movefile options forbuffer processing-type)))

IOW, org-create-formula-image never was expected to handle 'auto' in the old code.

Edit: but this doesn't explain why pdf-tools never errored before... There's a missing piece somewhere.

Edit 2: Ah! The old default value of org-format-latex-options never had auto so this is why I didn't notice it all? And 'default' is handled by org-create-formula-image specially. Sigh... I wonder if it would hurt too much if we simply make auto behave the same as default like in my patch.

karthink commented 1 week ago

the function org-latex-preview--attr-color does not accept 'auto' as an argument leading to the error.

org-latex-preview--attr-color is identical to the old org-latex-color so this applies to the old code as well.

If you ask how it never errored, org-format-latex actually updates the value of :foreground and :background to not have 'auto' when it passes OPTIONS to org-create-formula-image deep in the code.

I'm aware of this. But pdf-tools is calling org-create-formula-image, not org-format-latex.

IOW, org-create-formula-image never was expected to handle 'auto' in the old code.

Indeed.

Edit 2: Ah! The old default value of org-format-latex-options never had auto so this is why I didn't notice it all? And 'default' is handled by org-create-formula-image specially. Sigh... I wonder if it would hurt too much if we simply make auto behave the same as default like in my patch.

This is what I'll do then. There's already a bug with pdf-tools if org-format-latex-options has auto in it, so we'd be fixing that bug in the process. org-create-formula-image is not meant to be called directly anyway, I think org-format-latex was supposed to be the preview-creation API function.

karthink commented 1 week ago

Okay, all issues should be fixed as soon as Tecosaur merges my patches, which should be within the day.

I'll leave this issue open since the fixes are untested.

karthink commented 1 week ago

All patches have been applied. When you can, please test and let us know.

9viz commented 6 days ago

Was the patch pushed? I don't see any reference to it in https://code.tecosaur.net/tec/org-mode/commits/branch/dev on my end.

EDIT: Nevermind, I didn't scroll down enough. I will test it out and report tomorrow.

karthink commented 6 days ago

Yes, they've been pushed.

On Fri, Jun 28, 2024, 3:47 AM viz @.***> wrote:

Has the patch been pushed? I don't see any reference to it in https://code.tecosaur.net/tec/org-mode/commits/branch/dev on my end.

— Reply to this email directly, view it on GitHub https://github.com/tecosaur/org-latex-preview-todos/issues/34#issuecomment-2196619772, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBVOLBFB5TYDI7QPG6CUQTZJU5K3AVCNFSM6AAAAABJG5X7DKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJWGYYTSNZXGI . You are receiving this because you commented.Message ID: @.***>

9viz commented 20 hours ago

Sorry for the late reply, I was traveling so didn't have the time to test it properly. I get an error in Messages

Error muted by safe_call: (#[771 "\301\300!\207" [((buffer . #<buffer test.pdf>) (page . 1) (edges 0.329422 0.304045 0.368638 0.334348) (type . text) (id . annot-1-0) (flags . 24) (color . "#ff0000") (contents . "\\(\\lambda\\)") (modified 26246 15648) (label . "Visuwesh") (subject) (opacity . 1.0) (popup-edges) (popup-is-open) (created) (icon . "Note") (state . "unknown") (is-open)) pdf-annot-print-annotation] 5 ("/home/viz/lib/emacs/straight/build/pdf-tools/pdf-annot.elc" . 31840)] #<window 2090 on test.pdf> #<buffer test.pdf> 1) signaled (void-variable extended-info) [16 times]

This is because of the use of the non-existent extended-info in the let block of org-create-formula-image.

9viz commented 20 hours ago

If I just use org-latex-compiler for latex-compiler like below

diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index 2c58867dd..1c22f9f6e 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -1012,8 +1012,7 @@ (defun org-create-formula-image
           org-format-latex-header
           'snippet)))
     (latex-compiler
-          (cdr (assoc (plist-get extended-info :latex-processor)
-                      org-latex-preview-compiler-command-map)))
+          org-latex-compiler)
     (tmpdir temporary-file-directory)
     (texfilebase (make-temp-name
               (expand-file-name "orgtex" tmpdir)))

I get the error

Error muted by safe_call: (#[771 "\301\300!\207" [((buffer . #<buffer test.pdf>) (page . 1) (edges 0.329422 0.304045 0.368638 0.334348) (type . text) (id . annot-1-0) (flags . 24) (color . "#ff0000") (contents . "\\(\\lambda\\)") (modified 26246 15648) (label . "Visuwesh") (subject) (opacity . 1.0) (popup-edges) (popup-is-open) (created) (icon . "Note") (state . "unknown") (is-open)) pdf-annot-print-annotation] 5 ("/home/viz/lib/emacs/straight/build/pdf-tools/pdf-annot.elc" . 31840)] #<window 2137 on test.pdf> #<buffer test.pdf> 1) signaled (error "No valid command to process \"/tmp/pdf-tools-ZCteFC/test.pdf-ndAFoz/pdf-annot-print-annotation-latex/orgtex5fRwrU.tex\".  Please adjust `dvisvgm' part of `org-preview-latex-process-alist'.")

where the contents of the latex file is

%& /home/viz/.cache/org-persist/9a/80e6c7-1d27-4387-965e-8281684a8949-e758133dcf7f75fe61c5bd2840eae3a6
% Created 2024-07-04 வி 11:52
% Intended LaTeX compiler: pdflatex
\documentclass{article}
\usepackage{amssymb}
\usepackage{amsmath}
\usepackage[utf8]{inputenc}
\usepackage[T1]{fontenc}
% Package hyperref omitted
\usepackage{xcolor}
\setlength{\textwidth}{12cm}

% Generated preamble omitted for snippets.

% end precompiled preamble
\ifcsname endofdump\endcsname\endofdump\fi

\begin{document}
\definecolor{fg}{rgb}{0.000,0.000,0.000}%

{\color{fg}
\(\lambda\)%
}

\end{document}