Ihor Radchenko <yantar92@gmail.com> writes:

I got a new problem in patch, the (point-max) in function
~org-cycle-display-inline-images~ SOMETIMES return EOF error instead of
correct max point value when org-cycle `state' is ~'folded~. I can't
find out what caused this even using Edebug. Do you have any clue?

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

I will try to write tests later.

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

I mock this style from a very similar existing hook function
~org-cycle-hide-archived-subtrees~ and custom variable
~org-cycle-open-archived-trees~. I think removing a hook function from
~org-cycle-hook~ is a way, but not as convenient as defcustom option.
(P.S: I think users prefer this defcustom customization style.)

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

Done

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

Thanks for teaching.

-- 

[ stardiviner ]
I try to make every word tell the meaning that I want to express without misunderstanding.

Blog: https://stardiviner.github.io/
IRC(libera.chat, freenode): stardiviner, Matrix: stardiviner
GPG: F09F650D7D674819892591401B5DF1C95AE89AC3