> Further comments.
>
>> (setf (buffer-local-value
>>                               'org-latex-precompile (get-buffer (plist-get 
>> info :input-buffer)))
>>                              nil)
>
> :input-buffer does not always exist. In particular, when exporting using
> `org-export-data' directly.

I've added a check for :input-buffer. (squashed)

>> (defcustom org-latex-precompile nil
>
> It is defined after it is used.

Moved it to before (squashed).

>> (defvar org-latex-precompile-command
>
> Should it be defcustom? In any case, the docstring is not sufficient to
> understand all the %X inside.

I think this should be a defvar, there is no reason for users to mess
with it.

I've improved the documentation (squashed).

>> (preamble-hash
>>          (sha1 (concat preamble
>>                        (plist-get info :latex-compiler) ;
>>                        (alist-get ?l spec)
>>                        (if tempfile-p "-temp" default-directory))))
>
> This code is used twice. It will be the cleanest to factor it out into
> dedicated internal helper function.

It's only 4 lines and both uses are in view at the same time, so I think
it's okay.  Adding more indirection will make it harder to understand too.

>> temporary-file-directory
>
> Let's prefer (temporary-file-directory) over temporary-file-directory

I've made a note along with the other final checks -- there are many
uses of this in org-latex-preview.el, so I'll do it at the end along
with the other replacements.  It will make it hard to rebase otherwise.

>>        (when-let ((dump-file
>
> when-let*

Postponed to before-merge.

>> (default-directory
>>          (if tempfile-p temporary-file-directory default-directory))
>> ...
>> (org-latex--precompile-preamble
>>                      info preamble
>>                      (expand-file-name preamble-hash 
>> temporary-file-directory)
>>                      spec)
>
> This looks sus. Do you really want to pass (expand-file-name
> preamble-hash temporary-file-directory) even when tempfile-p is nil?

I think this is correct.  I remember us being confused about this too
before arriving at this solution.  I can research this after Tuesday.

Karthik

Reply via email to