> And now comments on org-latex-preview.el itself (part 1)
Finally, the part of the patch I wrote and am actually familiar with!
Hopefully I can do a better job of addressing these issues than the ones
so far.
>> (defvar org-src-mode-hook nil)
>
> This is not necessary.
Removed.
>> (defvar org-src--beg-marker nil)
>
> Just (defvar org-src--beg-marker) - no need to create a new dynamic
> variable. Even better - move this defvar into inside the function.
Moved into the only function where it is used.
>> (defcustom org-latex-preview-process-alist
>
> :latex-precompiler option is not documented in the docstring.
> In fact, it is also never used. Instead, `org-latex-precompile-command'
> is used.
You must have reviewed this before the commit where I adjusted the
precompilation logic. :latex-precompiler is gone from this variable now.
>> (defcustom org-latex-preview-compiler-command-map
>
> This reminds me of `org-latex-precompile-compiler-map' in ox-latex.
> `org-latex-precompile-compiler-map' is unused in the code.
org-latex-precompile-compiler-map was removed a few commits ago. Only
the compiler-command-map is used, and only by org-latex-preview.
>> (defconst org-latex-preview--precompile-log "*Org Preview Preamble
>> Precompilation*"
>> "Buffer name for Preview LaTeX output.")
>
> This is unused.
Removed now.
>> (defconst org-latex-preview--temp-cache-dir
>> (expand-file-name "org-latex-preview" temporary-file-directory)
>> "Folder used to cache temp-stored previews.")
>
> Please use (temporary-file-directory) call, not variable.
Task recorded for final round, just before merging. This is to make it
easy to rebase in the meantime.
>> (defconst org-latex-preview-live--cache-dir
>> (expand-file-name "org-latex-preview-mode-display-live"
>> temporary-file-directory)
>> "Folder used to cache live previews.")
>
> Is it ever used? From the code, this would correspond to
> org-latex-preview-cache = 'live (undocumented).
Yes, it is used _heavily_. I can explain how:
1. When using live previews, we generate lots (hundreds to thousands) of
previews of LaTeX fragments in valid but intermediate states, i.e. where
things have been partially typed.
2. Typically we only care about persisting the final/finished image for
a fragment.
3. Storing images for all of these states in org-persist causes a huge
increase in the number of LaTeX preview images being stored. My
org-persist cache would reach 2-3 GB size on disk in a few weeks of
heavy use of live previews. org-persist-gc would also run very slowly
when shutting down Emacs as a result.
4. So we cache intermediate states of live previews in
org-latex-preview-live--cache-dir, with a counter that resets when
org-latex-preview-live--max-cache-count (default 1024) live preview
images have been generated. The live cache is wiped when the
max-cache-count is reached. My org-persist disk usage and
org-persist-gc time went down by a large factor after this, and cached
images for the "finished" fragments were still available in org-persist.
5. Internally, this is implemented by let-binding
org-latex-preview-cache to 'live. This is not a value the user needs to
know about.
Added a short description of this design to the existing section comment
explaining live previews.
>> (defcustom org-latex-preview-preamble "\\documentclass{article}
>
> The default value has been altered after the rename. This warrants
> adding :package-version to the defcustom.
Added.
>> "The document header used for processing LaTeX fragments.
>> It is imperative that this header make sure that no page number
>> appears on the page. The package defined in the variables
>> `org-latex-default-packages-alist' and `org-latex-packages-alist'
>> will either replace the placeholder \"[PACKAGES]\" in this
>> header, or they will be appended."
>
> This is not "or they will be appended", but more subtle. See the
> docstring of org-latex-classes.
The docstring is an exact copy of that of org-format-latex-header on
main. I don't understand the "appended" part here or the docstring of
org-latex-classes, so please suggest a replacement.
>> (defcustom org-latex-preview-mode-track-inserts t
>
> I am looking that this is enabled by default, and that it is mentioned
> in the manual. But I am wondering why would everyone want to disable
> this option while keeping org-latex-preview-mode. Maybe it is not worth
> mentioning this option in the manual?
You may want the ability to hide/show preview images automatically by
moving into/out of fragments, but not call LaTeX automatically as you
type or paste text. In this case you can set track-inserts to nil.
This is a common enough need -- starting LaTeX from post-command-hook
can cause performance issues when typing or pasting in a large quantity
of text. Setting track-inserts to nil means that you can delay the
tracking until you call org-latex-preview once.
The nil value behavior of track-inserts is also the default behavior of
preview-latex from AucTeX. So I think it is worth mentioning in the
manual.
Karthik
