Re: [PATCH 2-v1] New: auto display inline images under subtree when `org-cycle'.

[Ihor Radchenko <yantar92@gmail.com>]

"Christopher M. Miles" <numbchild@gmail.com> writes:

> Ok, I created the patch for subtree cycling display inline images.
> Tested by edebug when org cycling, it should works fine. I don't know
> how to write test for this.

Thanks for the update!

For tests, you will need to test the existence of image overlays.
You can refer to test-org-fold/org-fold-reveal-broken-structure but
need to test overlays-at instead of just org-invisible-p.

> But I got a little problem:
>
> When ~org-fold-core-style~ is ~'overlays~, the ~delete-overlay~ will
> cause subtree unfolded. More detailed check out the "org.el" function
> ~org-remove-inline-images~ in my patch.

I will comment in the code.

>  (defcustom org-cycle-hook '(org-cycle-hide-archived-subtrees
> -		   org-cycle-show-empty-lines
> -		   org-cycle-optimize-window-after-visibility-change)
> +                            org-cycle-show-empty-lines
> +                            org-cycle-optimize-window-after-visibility-change
> +                            org-cycle-display-inline-images)
>    "Hook that is run after `org-cycle' has changed the buffer visibility.
>  The function(s) in this hook must accept a single argument which indicates
>  the new state that was set by the most recent `org-cycle' command.  The
> @@ -229,6 +230,12 @@ normal outline commands like `show-all', but not with the cycling commands."
>    :group 'org-cycle
>    :type 'boolean)
>  
> +(defcustom org-cycle-inline-images-display nil
> +  "Non-nil means auto display inline images under subtree when cycling."
> +  :group 'org-startup
> +  :group 'org-cycle
> +  :type 'boolean)

You are providing both hook and a customization. It is redundant. Users
can already remove the hook when desired.

In this area, we generally do not use custom variables (see M-x
customize-group org-cycle). So, it is better to use hook.

> -(defun org-remove-inline-images ()
> +(defun org-remove-inline-images (&optional beg end)
>    "Remove inline display of images."
>    (interactive)
> -  (mapc #'delete-overlay org-inline-image-overlays)
> -  (setq org-inline-image-overlays nil))
> +  (let* ((beg (or beg (point-min)))
> +         (end (or end (point-max)))
> +         (overlays (overlays-in beg end)))
> +    (dolist (ov overlays)
> +      (delete ov org-inline-image-overlays))
> +    (mapc #'delete-overlay overlays)
> +    ;; FIXME: `org-cycle-display-inline-images' can't fold because `delete-overlay' will unfold subtree.
> +    (when (eq org-fold-core-style 'overlays)
> +      ;; FIXME: don't know how to get the correct `spec'.
> +      (let ((spec (alist-get 'org-fold-hidden org-fold-core--specs)))
> +        (org-fold-core-region beg end t spec)))))

You can just

(dolist (ov overlays)
  (when (memq ov org-inline-image-overlays)
    (setq org-inline-image-overlays (delq ov org-inline-image-overlays))
    (delete-overlay ov)))

Note that `delete' and `delq' are not safe to use on their own. You
should always use (setq foo (delq 'member foo)). It is detailed in the
docstring.

-- 
Ihor Radchenko,
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