Ihor Radchenko <yantar92@posteo.net> writes:

> "Christopher M. Miles" <numbchild@gmail.com> writes:
>
>>> Unlike `org-latex-preview', here we need to (1) respect active region;
>>> (2) keep the backward compatibility for INCLUDE-LINKED.
>>>
>>> For (1), for example, it would make sense to respect region when prefix
>>> argument is C-u and clear images only there. For (2), we need to at
>>> least allow toggling images with description using some prefix argument
>>> (previously, any prefix argument would do).
>>
>> I indeed implemented all same behavior like `org-latex-preview' in new
>> `org-toggle-inline-images'.
>>
>> ...
>> And I also implement the toggle inline images in region logic in
>> `org-toggle-inline-images' as bellowing:
>>
>> ;; Display region selected inline images.
>>    ((use-region-p)
>>     (message "Displaying inline images in region...")
>>     (org-display-inline-images include-linked t (region-beginning) (region-end))
>>     (message "Displaying inline images in region... done."))
>
> This is not a toggle. This is unconditional refresh.
>
Yes, indeed it's a unconditional refresh. It does not match the meaning of word "toggle".

Seems need to detect whether has image overlays in region. I added this
detection in new patch. Like bellowing:

#+begin_src emacs-lisp
...
((use-region-p)
    (if (seq-contains-p
         (mapcar
          (lambda (ov)
            (plist-get (overlay-properties ov) 'org-image-overlay))
          (overlays-in beg end))
         t)
        (progn
          (org-remove-inline-images beg end)
          (message "Inline images in region removed."))
      (message "Displaying inline images in region...")
      (org-display-inline-images include-linked nil (region-beginning) (region-end))
      (message "Displaying inline images in region... done.")))
....
#+end_src

>> For (2), It's working. The code passed the parameter `include-linked' to
>> `org-display-inline-images'. So they works in any case of [C-u] prefix.
>
> My concern is that previously C-u M-x org-toggle-inline-images would
> "display links with a text description part". With your patch, it is no
> longer the case because INCLUDE-LINKED is not affected by the prefix
> argument.
>
> And there is no clean way to allow INCLUDE-LINKED while keeping
> consistency with latex preview commands.

About the INCLUDE-LINKED argument, I don't know how to process it. In
theory, it should be handled by function org-display-inline-images
instead of org-toggle-inline-images. If you have improvements on it, can
you add code on my patch?

>
> What we might do here is making a new defcustom that will control
> whether linked images should be displayed. Then, something like C-1
> org-toggle-inline-images could toggle that defcustom and refresh all the
> image previews in buffer (if any).
>
> WDYT?

Refreshing all image previews in buffer is same as old behavior. My
patch's purpose is to improve function org-toggle-inline-images behavior
which refresh only in current level scope instead of whole buffer with
lot of inline images especially heavy and suspend Emacs. Because
function org-toggle-inline-images is used in of lot Emacs ob-* related
packages on after babel execution hook. Old behavior caused execute one
source block need to refresh whole buffer all images. Instead of only
current headline images, or just results image.