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

[Ihor Radchenko]

Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:

>> We need to do something about the logic determining whether any previews
>> are displayed or not. The current check that preview overlays are
>> present in requested region is no longer accurate as they may still be
>> queued and not yet visible to the user, creating impression that there
>> are no previews yet.
>
> Please try it now, it should be fixed.  I now set the
> `org-image-overlay' when the preview is done/successful, instead of when
> it is queued.

I tried and things seems to be working fine now.

>>> 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.
>>
>> Just run make in Org git repo.
>
> Thanks.

And the warnings are still present :)

>>> 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?
>>
>> I like org-link-preview-- more, but the most important part is to make
>> things consistent across the function/variable names.
>
> I will check this in the final version, along with the manual/NEWS
> updates.

I have a few minor comments.

> +  (org-with-point-at (or beg (point-min))
> +    (let ((case-fold-search t)
> +          (preview-queue))

Nitpick: Just preview-queue without parenthesis will be the same.

> +      ;; Collect links to preview
> +      (while (re-search-forward org-link-any-re end t)
> +        (when-let* ((link (org-element-lineage
> +                           (save-match-data (org-element-context))
> +                           'link t))

Do we need `save-match-data' here? Nothing inside seems to be making use
of it.

> +     (setq org-link-preview--timer
> +           (and org-link-preview--queue
> +                (run-with-idle-timer
> +                 (time-add (or (current-idle-time) 0)
> +                           org-link-preview-delay)
> +                 nil #'org-link-preview--process-queue org-buffer))))))

What if `org-buffer' gets killed when the preview timer triggers?

-- 
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>