Karthik Chikmagalur <[email protected]> writes:
> I'm working on org-latex-preview actively this week, so even incremental
> feedback will be helpful as I can get started on it.
Comments on org-latex-preview.el (part 3).
> (defun org-latex-preview (&optional mode)
> ...
> MODE can also be a org-element LaTeX environment or fragment, which
> will be treated as \"point\"."
Unless I miss something, it does not look like MODE=element is ever used
anywhere in the code.
> (defun org-latex-preview--auto-aware-toggle (datum)
I think these "auto" is a leftover from the previous naming of the
mode. It would make sense to rename symbols according to new naming.
> (defun org-latex-preview--preview-region (processing-type &optional beg end)
> "Produce image overlays of LaTeX math fragments between BEG and END.
>
> The LaTeX fragments are processed using PROCESSING-TYPE, a key of
> `org-latex-preview-process-alist'.
>
> If `point' is currently on an LaTeX overlay, then no overlays
> will be generated. Since in practice `org-latex-preview-clear-overlays'
> should have been called immediately prior to this function, this
> situation should not occur in practice and mainly acts as
> protection against placing doubled up overlays."
What do you mean that "this situation should not occur"?
What about
('buffer
(org-latex-preview--preview-region
org-latex-preview-process-default (point-min) (point-max)))
C-u C-u M-x org-latex-preview??
> (when (memq processing-type '(dvipng dvisvgm imagemagick))
> (overlay-recenter (or end (point-max))))
This should be a FIXME. overlay-recenter does nothing since Emacs 29.
> (end (- (org-element-property :end element)
> (or (org-element-property :post-blank element) 0)
> (if (eq (char-before (org-element-property :end element))
> ?\n)
> 1 0)))
As I mentioned in another email, this is not the correct way to find
position before blank in the case of fragments.
> ;;;###autoload
> (defun org-latex-preview-place (processing-type entries &optional
> numbering-offsets latex-preamble)
Does it have to be autoloaded?
> "Creat a hash table from numbered equations to their initial index.
*Create
> (defvar org-latex-preview--numbering-count-buffer nil)
Docstring.
> (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.
> (goto-char (org-element-property :begin element))
> (when (looking-at "\\\\begin{\\([^}]+\\)}")
> (setq environment-name (match-string 1)))
What if there are affiliated keywords?
> (insert-buffer-substring-no-properties
> (org-element-property :buffer element)
> (org-element-property :begin element) (org-element-property :end
> element))
> (goto-char (point-min))
> (org-skip-whitespace)
> ;; Remove the "outer" environment, so we can then
> ;; easily delete all inner environments.
> (delete-region (point-min) (+ (point) 8 (length environment-name))) ; 8
> = (length "\\begin{}")
same question
> ((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).
> (cl-defun org-latex-preview-cache-images
checkdoc warnings.
> (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?
> (cl-defun org-latex-preview--create-image-async
> (processing-type fragments-info &key latex-processor latex-preamble
> appearance-options place-preview-p)
> "Preview PREVIEW-STRINGS asynchronously with method PROCESSING-TYPE.
What is PREVIEW-STRINGS?
> It is worth noting the FRAGMENTS-INFO plists will be modified
> during processing to hold more information on the fragments.
+It is worth noting the+ (it is redundant)
> When provided, LATEX-PREAMBLE overrides the default LaTeX preamble.
> (processing-info
> (nconc (list :latex-processor latex-processor
> :latex-header latex-preamble)
> (alist-get processing-type org-latex-preview-process-alist)))
Maybe I am missing something, if latex-preamble=nil, will it simply
shadow :latex-header from the process-alist? Then it implies that
:latex-header in process-alist is ignored.
> ((eq cache-location 'live)
> org-latex-preview-live--cache-dir)
I think I got confused by this in the past.
'live looks like something users can set for org-latex-preview-cache,
but it is actually internal. Maybe use some more telling symbol name for this.
> (and (eq exit-code 1)
> (with-current-buffer
> (save-excursion
> (goto-char (point-min))
> (search-forward ": No such file or directory"
> nil t))))
with-current-buffer seems to be not intentional here.
> (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?
> (defun org-latex-preview--create-tex-file (processing-info fragments
> appearance-options)
> "Create a LaTeX file based on PROCESSING-INFO and FRAGMENTS.
APPEARANCE-OPTIONS undocumented in the docstring.
--
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
