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

[Karthik Chikmagalur]

Latest patch attached.

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

(org-defkey org-mode-map (kbd "C-c C-x C-M-v") #'org-redisplay-inline-images)

Do we need this command any more?  I don't understand what it does
differently from `org-link-preview'.  Calling `org-link-preview' with
the right scope argument (e.g. C-u C-u for buffer-wide) should
automatically refresh/redisplay images.

I also don't know what to do about this warning:

(define-obsolete-variable-alias 'org-plain-link-re
  'org-link-plain-re "9.3")

org-compat.el:1184:4: Warning: Alias for ‘org-link-plain-re’ should be declared 
before its referent

I think this warning is unrelated to this patch?

I fixed the other warnings.

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

I changed it.  I've switched to using (foo) instead of foo in let blocks
because `elisp-flymake-byte-compile' complains sometimes (not always).
I don't know why this happens.

>
>> +      ;; 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.

I removed 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?

Handled via a `buffer-live-p' check.  I don't bother resetting the buffer-local
variables `org-link-preview--queue' and `org-link-preview--timer' since
I'm assuming the local bindings go away when the buffer is killed.

Karthik

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