>>> Unless I miss something, it does not look like MODE=element is ever used
>>> anywhere in the code.
>>
>> It's not used in the code, but is implemented in this function and
>> provided for elisp scripting purposes. Should it be removed?
>
> I think it is simple, so it is ok to keep it.
> I was concerned that some part of the changes might be missing from the
> latest branch.
Okay.
>>> Does it have to be autoloaded?
>>
>> Yes. This is the API entry point for Emacs-wide LaTeX previews (in any
>> major-mode). Anywhere in Emacs, you can run
>>
>> [...]
>>
>> should be provided too as the fourth argument to this function for
>> Org-independent previews, but it should work without this argument as well)
>>
>> You can try it now in your mail client:
>>
>> Select this fragment: \( 2\pi + 3 \)
>>
>> and eval-expression:
>>
>> (org-latex-preview-place 'dvisvgm `((,(region-beginning) ,(region-end))))
>>
>> In the future I want to add an API for full LaTeX preview support for
>> any major-mode, including live previews.
>
> Then, it is a good idea to add similar high-level description to the top
> comment in the org-latex-preview.el.
Added package commentary to org-latex-preview, explaining the major
commands and API functions made available.
>>>> (with-current-buffer (org-element-property :buffer element)
>>>> (save-excursion
>>>> (goto-char (org-element-property :begin element))
>>>
>>> (org-with-point-at element ...) will do almost the same.
>>
>> I'm not familiar with (org-with-point-at element) and hesitant to change
>> this now. Is it okay to leave it in its current form? You can make the
>> change before merging. I can also give you push access to my working
>> repo.
>
> Sure. This is not a big deal.
Okay.
>>>> ((org-element--cache-active-p)
>>>> (org-element-cache-map
>>>
>>> org-element-cache-map on main works even when cache is not active (by
>>> temporarily enabling it).
>>
>> I take it to mean the org-element-map of this cond can be discarded.
>> I've made a tentative change, could you take a look at whether it makes
>> sense?
>
> Yes, it does.
Okay.
>>>> (elements
>>>> (org-element-map parse-tree
>>>> '(latex-fragment latex-environment)
>>>> #'identity export-info))
>>>> (entries-and-numbering
>>>> (org-latex-preview--construct-entries
>>>> elements t parse-tree))
>>>
>>> This implies that current buffer is the origin of the PARSE-TREE. Is it
>>> always the case?
>>
>> org-latex-preview-cache-images is meant to be used by exporters for
>> generating LaTeX preview images. In that context, I think this is the case?
>>
>> If you think a more robust alternative is required, please let me know
>> what change to make.
>
> I think it will suffice to mention this in the docstring.
> Maybe similar problems in other public functions.
Added note to the docstring.
>> I modified it to
>>
>> (processing-info
>> (nconc (list :latex-processor latex-processor)
>> (and latex-preamble (list :latex-header latex-preamble))
>> (alist-get processing-type org-latex-preview-process-alist)))
>>
>> Should org-latex-preview-process-alist have a :latex-header field? I'm
>> not sure what its purpose is.
>
> It is in the docstring:
>
> :latex-header list of strings, the LaTeX header of the snippet file.
> When nil, the fallback value is used instead, which is
> controlled by option `org-latex-preview-preamble',
> `org-latex-default-packages-alist' and
> `org-latex-packages-alist', which see.
Yes. I was wondering if it made sense to keep it.
org-latex-preview-process-alist is no longer an alias of the old variable, and
we can decide if it makes sense to include the :latex-header specification here
as an option.
Personally, I don't see a need for it.
>>>> (if (pcase (plist-get extended-info :processor)
>>>> ('dvisvgm (eq exit-code 252)) ; Input file does not exist.
>>>> ('dvipng ; Same check, just a bit more involved.
>>>> (and (eq exit-code 1)
>>>> (with-current-buffer
>>>> (save-excursion
>>>> (goto-char (point-min))
>>>> (search-forward ": No such file or directory"
>>>> nil t))))))
>>>> (propertize org-latex-preview--latex-log 'face 'warning)
>>>> (propertize org-latex-preview--image-log 'face 'warning))
>>>
>>> More generally, why do you need to guess the buffer here and do not use
>>> _buf instead?
>>
>> I am now using _buf as the process buffer to search in for the dvipng
>> branch. But the problem that this is trying to solve is different: when
>> the image-extract-async task fails, we don't know if it failed because
>> dvipng failed to convert the dvi file to images, or because the dvi file
>> itself was not generated. If the dvi file itself was not generated, the user
>> should look in org-latex-preview--latex-log, else in
>> org-latex-preview--image-log. This is what the if statement above is
>> determining.
>
> But this is in
> (plist-put (cddr img-extract-async) :failure
> (list
> #'org-latex-preview--failure-callback
> #'org-latex-preview--cleanup-callback))
> So, this should only trigger when image conversion fails, which implies
> that .dvi files has been generated.
> What do I miss?
Two things:
1. preview.sty can fail with exit-code 1 even when the run is
successful. This is by design for preview.sty, details are in its
manual. For this reason we have:
(plist-put (cddr tex-compile-async) :success img-extract-async)
(plist-put (cddr tex-compile-async) :failure img-extract-async)
implying we always run the image extractor, even if the tex compilation
"fails". So when the image converter fails, we don't know if it was
because the tex compilation failed and no dvi was produced.
2. dvipng has a --follow option that we use, where it is started right
away, before any dvi files have been produced. This is for speed.
dvipng can read a partially produced dvi file and start producing images
for completed pages. This is why the dvipng route is much faster than
dvisvgm -- dvipng previews can feel instant if you have a small LaTeX
preamble.
But in this context it means image-extract-async runs whether or not the
LaTeX process fails.
Karthik
