> More comments on org-latex-preview.el (part 2)
>
>> (defun org-latex-preview--ensure-overlay (beg end)
>> ...
>> (overlay-put ov 'preview-state nil) ;is fragment modified?
>
> You are using all kinds of special overlay properties.
> Please,
> 1. Use org- prefix for the property names
Done.
> 2. Move these functions after the comment describing the purpose of all
> these special overlay properties.
Done.
>> (setq org-latex-preview-mode--from-overlay
>> (eq (get-char-property (point) 'org-overlay-type)
>> 'org-latex-overlay))
>
>> (into-overlay-p (eq (get-char-property (point) 'org-overlay-type)
>> 'org-latex-overlay))
>
> What if we have multiple overlays at point? For example latex preview
> overlay inside table with shrunk columns.
> I think it will glitch with display-live=t
I couldn't get it to glitch, but if you have a reproducer let me know.
I understand the issue in principle: some other org-overlay-type
covering a LaTeX preview overlay, but in three years of using this
feature I haven't had it happen.
I can change this check from
(eq (get-char-property (point) 'org-overlay-type) 'org-latex-overlay)
to
(get-char-property (point) 'org-latex-overlay)
where a non-nil value of this property implies that it's a LaTeX
overlay. This is how image overlays are handled IIRC, with an overlay
property org-image-overlay instead of setting over-overlay-type.
But if there are multiple overlays that are setting the display property
I think glitches will happen no matter what. Let me know what you
think.
>> (get-char-property (point) 'invisible)) ;Overlay in invisible region
>
> non-nil 'invisible property does not mean that the overlay is
> invisible. Depending on buffer-invisibility-spec, it may be visible.
> You can use `org-invisible-p'
I think the distinction between invisible because of folding vs some
other reason doesn't matter here, so replaced it with invisible-p vs
org-invisible-p.
>> (get-char-property (point) 'view-text)
>
> So, 'view-text is boolean. I think it is worth discussing the values of
> 'view-text and other special properties in the comment describing how
> preview-mode works.
Added for all four special overlay properties. But in the process I
realized org-latex-preview-mode--insert-front-handler and
org-latex-preview-mode--insert-behind-handler might not be needed at
all. I'm leaving it in for now and it should be fine to include them in
the patch, but I will need to do some testing to make sure I'm not
misunderstanding their purpose.
>> (while (and (< (point) end)
>> (search-forward "\n" end t))
>> (skip-chars-forward " \t")
>> (when (> (point-max) (+ 4 (point)))
>> (push (point) line-start-positions)))
>
> Why not simply forward-line?
Hmm, I'm not sure. I changed it to
(while (and (< (point) end) (= 0 (forward-line 1)))
(skip-chars-forward " \t")
(when (> (point-max) (+ 4 (point)))
(push (point) line-start-positions)))
Hopefully that's cleaner and equivalent. I added a test to the return
value of forward-line because I'm afraid of an infinite loop where
forward-line can't move point (eob in a narrowed buffer, maybe) and the
while loop keeps going.
>> (elem-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)))
>
> This will be broken if a latex environment has blank lines that are not
> mere newlines but also contain whitespace.
Why? I tried it with
--8<---------------cut here---------------start------------->8---
\begin{align}
x + 3
\end{align}
And some more text
--8<---------------cut here---------------end--------------->8---
with the point in the align environment, which has a blank line with
some whitespace. It appears to work as expected.
>> ;; delay is reduced. Setting an 0.05s timer isn't
>> ;; necesarily the optimal duration, but from a little
>> ;; testing it appears to be fairly reasonable.
>> (run-at-time 0.01 nil #'org-latex-preview-mode--regenerate-overlay
>> ov)
>
> So, 0.05 or 0.01?
0.01 after further testing on different machines. Fixed.
>> When LaTeX preview auto mode is on, LaTeX fragments in Org
>
> Leftover from previous naming.
Fixed.
>> (if org-latex-preview-mode
>> (progn
>> (setq org-latex-preview-mode--marker (make-marker))
>> (add-hook 'pre-command-hook
>> #'org-latex-preview-mode--handle-pre-cursor nil 'local)
>
> What if there is indirect buffer?
It works fine in indirect buffers when the buffer-local variables and
hooks get copied over. Do you have an example of a problem?
> What if we do M-x org-tree-to-indirect-buffer with
> org-latex-preview-mode active?
Then org-latex-preview-mode is active in the indirect buffer too.
> What if it is not active?
> What if we disable
> org-latex-preview-mode in one buffer but not another?
It works as expected in these cases.
> Note that after-change-functions in one buffer are not called when edits
> are done in another indirect buffer.
The indirect buffer gets its own local after-change-functions hook which
contains org-latex-preview-mode--detect-fragments-in-change.
I haven't encountered a problem with org-latex-preview-mode and indirect
buffers in the past couple of years. If you have any specific concerns
or an experiment that causes issues let me know.
>> (defvar-local org-latex-preview-live--element-type nil)
>>
>> (defvar-local org-latex-preview-live--generator nil)
>
> Docstrings.
Added.
>> (defcustom org-latex-preview-mode-display-live '(block edit-special)
>
> :package-version is missing.
Added
>> (defvar-local org-latex-preview-live--preview-times
>> (make-vector 3 1.0)
>> "Vector containing the last three preview run times in this buffer")
>> (defvar-local org-latex-preview-live--last-hash nil
>> "Last hashed fragment when live-previewing")
>
> Missing "." at the end
Added (also in two other locations).
>> (defvar-local org-latex-preview-live--preview-times-index 0)
>
> docstring.
Done.
>> (defconst org-latex-preview-mode-display-type 'buffer
>
> This is a duplicate of earlier defcustom.
Deleted.
>> (defun org-latex-preview-live--ensure-overlay (&optional ov)
>
> This name is confusing. Unlike `org-latex-preview--ensure-overlay' it
> does not always create an overlay.
I've renamed things as follows:
org-latex-preview-live--ensure-overlay -> org-latex-preview-live--setup-overlay
org-latex-preview-live--update-overlay -> org-latex-preview-live--update-overlay
(no change)
org-latex-preview-live--clearout -> org-latex-preview-live--clearout-overlay
The three functions set up the live preview image for an overlay, update
it when an image is regenerated because of changes, and clear out the
live preview image for the overlay, respectively.
>> (equal major-mode (org-src-get-lang-mode "latex"))
>
> derived-mode-p maybe?
Changed to (derived-mode-p (org-src-get-lang-mode "latex")), not yet
pushed.
All other changes have been pushed.
I will address the feedback about the fragility of
org-latex-preview-live--src-buffer-setup in another email, as it's
a more involved discussion.
Karthik
