[PATCH v3.2] Re: [PATCH] add a function to only refresh inline images under current headline instead of global buffer

[Christopher M. Miles]

Ihor Radchenko <yantar92@posteo.net> writes:

> "Christopher M. Miles" <numbchild@gmail.com> writes:
>
>>> I think that instead of changing the existing function, we can convert
>>> your patch into a new function `org-toggle-inline-images-command' that
>>> will be free to alter the argument order. We can then re-bind that
>>> function in org-keys to make it used by default, but only interactively.
>>>
>> I don't think so, the patch main purpose is for improve image refreshing
>> after babel result image displaying. Because the function is hooked by
>> other ob-* packages. Another purpose is headline level images displaying.
>> So modifying this function is the only way.
>
> Fair point.
> Then, for backward compatibility, we may treat any non-nil, non-list
> (like '(4), '(16), '(64)), non-number (like 1, 11) value as
> INCLUDE-LINKED. This way, the existing calls like
> (org-toggle-inline-images t) will not be broken.
>
> Also, the signature should be
>
> (defun org-toggle-inline-images (&optional arg beg end include-linked)
>
> So that the meaning of the second and third argument is unchanged.
>

I prefer this compromise result.
I updated the patch, Please review it whether it's correct.

>>> As for displaying linked images, what about something like
>>>
>>>
>>> C-1 C-c C-x C-v being equivalent of
>>> C-c C-x C-v + INCLUDE-LINKED=t
>>> (display in current section/region, with linked)
>>>
>>> and
>>>
>>> C-11 C-c C-x C-v being equivalent of
>>> C-u C-u C-c C-x C-v + INCLUDE-LINKED=t
>>> (display in the whole buffer, with linked)
>>>
>> Don't know, I have not use it this way. I think INCLUDE-LINKED is just a
>> option argument for function org-display-inline-images. No need to be
>> available for toggle in interactive command. An option like
>> org-inline-images-include-linked is enough.
>
> Sorry, but ignoring backwards compatibility is not an option.
> If you personally haven't used some feature, it does not mean that
> others also didn't.
>
> We can make limited compromises, but should try our best not to break
> things that were working before.
> See https://bzg.fr/en/the-software-maintainers-pledge/
>

I agree with backward compatibility is very important.

I did a source code statistics researching of using the function 
org-toggle-inline-images at two places:

- GitHub: 
https://github.com/search?q=org-toggle-inline-images&ref=opensearch&type=code&p=3
(Only package scimax incoke this function with INCLUDE-LINKED argument t)
- My installed Emacs packages, NO package invoke this function with argument 
INCLUDE-LINKED.

So lucky this change will not affect lot.

>>>> +If cursor is on an inline image link, display the inline image.
>>>> +If there is none, remove it otherwise.
>>>
>>> I do not quite understand what the last line is trying to say.
>>
>> Emmm, I forget the meaning of it. I read the docstring again, seems it
>> can be deleted. I will delete it in my patch.
>
>> After yesterday night studying source code, I still can't figure out
>> solution. So I pass this patch to you. Wait you to finish the unfinished
>> part of path.
>
> I hope that now you have enough information to update the patch.
> Let me know if not.

If this patch is still not ok, really hope you can edit my patch.

0001-org-Improve-inline-images-displaying-like-LaTeX-prev.patch
Description: Text Data

-- 

[ 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

signature.asc
Description: PGP signature