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

[Karthik Chikmagalur]

Next version of patch attached.

> 1. I run preview on the whole buffer
> 2. M-x list-timers shows the timer, and it is ugly:
>       * 10.0s - #f(compiled-function () #<bytecode 0x6278e9619bf2d9a>
>
> [...]
>
>                  10+ sec]
>    - So, no closures please. Use normal named functions

Done.  Only one entry per buffer, `org-link-preview--timer', shows up in
`list-timers' now.

>    - It may also be a good idea to manage preview queue globally, so
>      that subsequent preview requests can be merged automatically (see
>      below)
> 3. It will take a long time to preview all those images, so I get
>    impatient and cancel the previews. The timer still remains...
> 4. I request previews in the whole buffer again. Now, there are two
>    timers. This may be repeated, and the user may end up with many
>    useless idle timers - each of them will take a long time to end,
>    because "preview batches" do not care whether we actually try to
>    preview images or skip them because the underlying overlays were
>    "canceled" by another command - I think that the timers should not
>    group images in batches beforehand, but simply consume images from a
>    single queue until a given number of images are actually submitted
>    for preview generation (overlays are still present).

New design:
1. Only one preview queue per buffer, stored in the buffer-local
variable `org-link-preview--queue'.  This queue is FIFO.
2. Only one (possibly periodically refreshed) preview timer per buffer,
stored in the buffer-local variable `org-link-preview--timer'.

The function `org-link-preview--process-queue' runs through this queue
`org-link-preview-batch-size' items at a time, and sets
`org-link-preview--timer'.  No explicit batches are created.

If you think it's a good idea, I can add pending previews to the queue
in a LIFO fashion instead, so that if you call `org-link-preview' in two
different sections before the first one is done previewing, the later
one is processed first.

If `org-link-preview-clear' is run in a region where there are previews
pending, these region-specific previews are removed from the queue.
Note that this new code in `org-link-preview-clear' is not actually
necessary:

   (when (memq ov org-link-preview-overlays)
     ;; Remove pending preview tasks between BEG and END
     (when-let ((spec (cl-find ov org-link-preview--queue
                               :key #'cadr)))
       (setq org-link-preview--queue
             (delq spec org-link-preview--queue)))

This is because the overlay placed over the links for which previews are
pending are removed anyway.  So when the `--process-queue' function gets
to that preview-pending link, it skips it since the overlay is gone.
Also, the complexity of the above code is quadratic, so I can remove it
if you think it's not required.  I included it to be thorough.

Also, I think the variable `org-link-preview-overlays' is actually
unnecessary.  We can just use an overlay property for link-preview
overlays and use `overlays-in' and `org-find-overlays', and reduce the
amount of global state a little bit this way.  Let me know if I should
remove it.

> And please fix the compiler warnings.

As of this patch, I don't see any flymake errors with the
`elisp-flymake-byte-compile' backend.  Is this not sufficient?
Byte-compiling in my active Emacs session doesn't work as the state is
"polluted".  I don't know of a convenient way to byte-compile code in a
sandbox.

Please note: I need some help with code style, for example
`org-link--preview-queue' vs `org-link-preview--queue', etc. Let's
postpone this particular discussion until after the design is final?

Karthik

0001-org-link-Customizable-preview-API-for-arbitrary-link.patch
Description: Text Data