Hey Ihor,

Thanks for the review! There’s no rush, I’m still trying to find time
to work on the HTTP side.

Before sending v2 I wanted to clarify how to handle one of your
comments.

On 2025-10-12 07:28, Ihor Radchenko wrote:
>>-       (const :tag "Use dvipng to make images" dvipng)
>>-       (const :tag "Use imagemagick to make images" imagemagick)
>>-       (const :tag "Use dvisvgm to make images" dvisvgm)
>>+       (symbol :tag "Convert fragments to images" :value dvipng)
>>        (const :tag "Leave math verbatim" verbatim)))
>
> After this change, type specification will not longer be accurate.

That’s true; org-html-with-latex has the same problem (which I
modelled it after). A symbol type is too permissive, and removing
consts like dvipng makes the customization menu less friendly and
discoverable. I can think of a few ways to handle this, any thoughts?

1. Unroll org-preview-latex-process-alist into consts when the
   defcustom is evaluated:

   #+begin_src emacs-lisp
   (let ((preview-choices
          (seq-map (lambda (pv) (list 'const (car pv)))
                   org-preview-latex-process-alist)))
     `(choice :tag "Convert fragments to images"
              :value ,@(cdar preview-choices)
              ,@preview-choices))
   #+end_src

   This is accurate so long as the preview list doesn’t change.

2. Use a restricted-sexp to check membership:

   #+begin_src emacs-lisp
   (restricted-sexp :tag "Convert fragments to images"
                    :value dvipng
                    :match-alternatives
                    ((lambda (pv) (assq pv org-preview-latex-process-alist))))
   #+end_src

   This is always accurate, but isn’t as friendly to customize.

3. Some hybrid of consts and restricted-sexp.

> This clause looks overengineered.
> Why not simply leaving the guard check and then putting the rest into
> the body?

No good reason, I was just taking advantage of the let*-like bindings
in pcase. I agree that it’s too much for the top conditional, which
should only be switching on the processing type. Moved inside the body
in v2.

> Also, we need to mark the change in the variable meaning via :package-version.
> --8<--
> :package-version, not :version. :version should only be used for
> internal Emacs libraries.

Done in v2, both are marked with :package-version '(Org . "9.8") only.

> Also, please add a record announcing the new custom option in etc/ORG-NEWS.

Added this, and the change in org-odt-with-latex in v2.

Thanks,

--
Jacob S. Gordon
jacob.as.gor...@gmail.com
Please avoid sending me HTML emails and MS Office documents.
https://useplaintext.email/#etiquette

Reply via email to