Re: [PATCH v3] Inline image display as part of a new org-link-preview system

[Ihor Radchenko <yantar92@posteo.net>]

Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:

>> Image cache is cleared _only_ with REFRESH argument.
>> I think that makes sense, right?
>
> Yes.  But `org-link-preview-region' is always called with the REFRESH
> argument set to `t' though.

Sure. What's a problem with that?

In theory, we might not need to clear image cache as long as we call
`image-flush' in `org-link-preview-clear'.

>>> Just unmodified old code.  I've require'd image in ol now, let me know
>>> if I should do it differently.
>>
>> Let's not require it on top level. Maybe better do it within the preview
>> function.
>
> Moved (require 'image) to inside the `org-link-preview-file'.  I know it
> doesn't affect performance in any reasonable way, but I'm usually
> hesitant to do this in functions that run inside loops.

There are pros and cons.
Ideally, we should have these previews in a separate library, so that
loading Org mode does not need to require so much staff. Loading time is
not great already.

My hesitance about top-level require is that ol.el is loaded by default
and that the preview functionality may or may not be used by the
users. So, loading image.el ought to be optional.

> ...
> +      ;; Run preview asynchronously in batches:
> +      ;; preview-queue is a list of preview-batch, which is a list of preview-spec
> +      (when (car preview-queue)
> +        (dolist (preview-batch (nreverse preview-queue))
> +          (run-with-idle-timer
> +           org-link-preview-delay nil

It means that you are scheduling every single preview batch to fire at
the same time. I think that the timers here should run sequentially -
(1) fire the first batch without delay; (2) wait org-link-preview-delay
and fire the next batch; (3) ...

> +           (lambda (previews)
> +             ;; (message "queue: %d" preview-queue-size)
> +             (cl-decf preview-queue-size)
> +             (dolist (preview-spec (nreverse previews)) ;spec is (preview-func overlay path link)
> +               (when-let* ((ov (cadr preview-spec))
> +                           (buf (overlay-buffer ov)))

ov or its buffer may not exit at the time the timer is fired.
Because, for example, the buffer is killed, or because user edited the
underlying link before it got displayed.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>