Re: Link preview generation with new link preview property

Re: Link preview generation with new link preview property

[Karthik Chikmagalur]

> The PR do port the package to this new handler property:
> https://github.com/gaoDean/org-remoteimg/pull/4
>
> However it left me with a few questions:
> - Who is supposed to take care of the scaling/width of the preview
>   image, the handler or the caller of the handler?

The handler is responsible for sizing the image.  The reason for this is
that the preview does not have to be an image -- it can be any kind of
overlay decoration.

If you decide to use an image file as the preview, you can call
`org-link-preview-file' inside your handler to handle the geometry for
you.  This includes the size and alignment specified by #+attr_*
keywords, `org-image-max-width' and `org-image-align'.

If you are using a preview image from image data, you'll have to copy
some of the code in `org-link-preview-file' to your handler if you want
to respect these user options.

> - When calling the link preview function for a description with prefix
>   argument 1 I noticed that it doesn't get the contents of the
>   description.
>   Why is that? In this instance the link looked like this:
>   [[https://stable.melpa.org/#/rpm-spec-mode][file:https://stable.melpa.org/packages/rpm-spec-mode-badge.svg]]

I don't follow.  What do you mean by "it doesn't get the contents"?  Did
the preview work as expected when you used a prefix arg of 1?

Karthik

Re: Link preview generation with new link preview property

[Björn Bidar]

Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:

>> The PR do port the package to this new handler property:
>> https://github.com/gaoDean/org-remoteimg/pull/4
>>
>> However it left me with a few questions:
>> - Who is supposed to take care of the scaling/width of the preview
>>   image, the handler or the caller of the handler?
>
> The handler is responsible for sizing the image.  The reason for this is
> that the preview does not have to be an image -- it can be any kind of
> overlay decoration.
>
> If you decide to use an image file as the preview, you can call
> `org-link-preview-file' inside your handler to handle the geometry for
> you.  This includes the size and alignment specified by #+attr_*
> keywords, `org-image-max-width' and `org-image-align'.
>
> If you are using a preview image from image data, you'll have to copy
> some of the code in `org-link-preview-file' to your handler if you want
> to respect these user options.

Would it be possible to also handle image data in the function or
refactor the org-link-preview-file function in a way that the geometry
handling is done in a helper function which can be reused by other
handlers.

In the example I mentioned it should be possible to use the cached file
from the url cache but that might be not so easy in other cases.

>> - When calling the link preview function for a description with prefix
>>   argument 1 I noticed that it doesn't get the contents of the
>>   description.
>>   Why is that? In this instance the link looked like this:
>>   [[https://stable.melpa.org/#/rpm-spec-mode][file:https://stable.melpa.org/packages/rpm-spec-mode-badge.svg]]
>
> I don't follow.  What do you mean by "it doesn't get the contents"?  Did
> the preview work as expected when you used a prefix arg of 1?

The link element passed towards the handler didn't contain the
description e.g. in this case file:https://stable.melpa.org/packages/rpm-spec-mode-badge.svg.

Re: Link preview generation with new link preview property

[Ihor Radchenko]

Björn Bidar <bjorn.bidar@thaodan.de> writes:

> Would it be possible to also handle image data in the function or
> refactor the org-link-preview-file function in a way that the geometry
> handling is done in a helper function which can be reused by other
> handlers.

There is such function: `org-display-inline-image--width'.
We may consider exposing it as public function.

> The link element passed towards the handler didn't contain the
> description e.g. in this case file:https://stable.melpa.org/packages/rpm-spec-mode-badge.svg.

This is expected. See https://orgmode.org/worg/dev/org-element-api.html#org22c967a
You can access the contents by looking at buffer text between
:contents-begin and :contents-end. Either directly, or by invoking
sub-parser on the text.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>

Re: Link preview generation with new link preview property

[Björn Bidar]

Ihor Radchenko <yantar92@posteo.net> writes:

> Björn Bidar <bjorn.bidar@thaodan.de> writes:
>
>> Would it be possible to also handle image data in the function or
>> refactor the org-link-preview-file function in a way that the geometry
>> handling is done in a helper function which can be reused by other
>> handlers.
>
> There is such function: `org-display-inline-image--width'.
> We may consider exposing it as public function.

That would be awesome, the easier it is for other modes to add their own
handlers and them to survive org-mode updates.
Although for this http link handler it could be better a function inside org-mode.

>> The link element passed towards the handler didn't contain the
>> description e.g. in this case file:https://stable.melpa.org/packages/rpm-spec-mode-badge.svg.
>
> This is expected. See https://orgmode.org/worg/dev/org-element-api.html#org22c967a
> You can access the contents by looking at buffer text between
> :contents-begin and :contents-end. Either directly, or by invoking
> sub-parser on the text.

Oh thanks, will look into that. Is there a offline version of the manual
like the org info manual? 
 

Re: Link preview generation with new link preview property

[Ihor Radchenko]

Björn Bidar <bjorn.bidar@thaodan.de> writes:

>> This is expected. See https://orgmode.org/worg/dev/org-element-api.html#org22c967a
>> You can access the contents by looking at buffer text between
>> :contents-begin and :contents-end. Either directly, or by invoking
>> sub-parser on the text.
>
> Oh thanks, will look into that. Is there a offline version of the manual
> like the org info manual? 

Nothing shipped with Org distribution.
But you can always clone WORG - it is mostly a collection of .org files.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>

Re: Link preview generation with new link preview property

[Björn Bidar]

Ihor Radchenko <yantar92@posteo.net> writes:

> Björn Bidar <bjorn.bidar@thaodan.de> writes:
>
>>> This is expected. See https://orgmode.org/worg/dev/org-element-api.html#org22c967a
>>> You can access the contents by looking at buffer text between
>>> :contents-begin and :contents-end. Either directly, or by invoking
>>> sub-parser on the text.
>>
>> Oh thanks, will look into that. Is there a offline version of the manual
>> like the org info manual? 
>
> Nothing shipped with Org distribution.
> But you can always clone WORG - it is mostly a collection of .org files.

That's a good suggestion. I never used org for reading only but it
should be better than a website. Does view-mode mode work fine with org?

Anyway it would be a good improvement to include the org-mode api manual
with org-mode similar to how Emacs includes the Elisp manual.

Re: Link preview generation with new link preview property

[Ihor Radchenko]

Björn Bidar <bjorn.bidar@thaodan.de> writes:

> Anyway it would be a good improvement to include the org-mode api manual
> with org-mode similar to how Emacs includes the Elisp manual.

Maybe. But we need to write that org-element API manual first.
The WORG page is not a manual. It's a mixture of API reference (partial)
with some elements of what supposed to be in the manual.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>

Re: Link preview generation with new link preview property

[Karthik Chikmagalur]

>> Would it be possible to also handle image data in the function or
>> refactor the org-link-preview-file function in a way that the geometry
>> handling is done in a helper function which can be reused by other
>> handlers.
>
> There is such function: `org-display-inline-image--width'.
> We may consider exposing it as public function.

This will not be enough, we will also have to expose `org-image--align'
and the alignment code:

(when align
  (overlay-put
   ov 'before-string
   (propertize
    " " 'face 'default
    'display
    (pcase align
      ("center" `(space :align-to (- center (0.5 . ,image))))
      ("right"  `(space :align-to (- right ,image)))))))

I don't think we can expect third party packages to figure out and
reimplement Org's image alignment logic.

Considering this, it might be better to just split
`org-link-preview-file' into two public functions, where the "inside"
function accepts an image instead of a file.

Karthik

Re: Link preview generation with new link preview property

[Ihor Radchenko]

Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:

> ...
> Considering this, it might be better to just split
> `org-link-preview-file' into two public functions, where the "inside"
> function accepts an image instead of a file.

Agree.
Would you be interested to create a patch?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>

Re: Link preview generation with new link preview property

[Björn Bidar]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

Ihor Radchenko <yantar92@posteo.net> writes:

> Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:
>
>> ...
>> Considering this, it might be better to just split
>> `org-link-preview-file' into two public functions, where the "inside"
>> function accepts an image instead of a file.
>
> Agree.
> Would you be interested to create a patch?

Did you think of something like this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-link-Split-link-preview-into-two-functions.patch --]
[-- Type: text/x-patch, Size: 2817 bytes --]

From a3c682a7d730b0ae9492407d080ba8549a19e02d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <bjorn.bidar@thaodan.de>
Date: Fri, 20 Dec 2024 01:00:46 +0200
Subject: [PATCH] org-link: Split link preview into two functions

* lisp/ol.el (org-link-preview-file,
org-link-preview-image-data): Split up the actual preview part into
a separate function.  The new preview function can be called by
:preview handler to display the image according to the correct alignment
for the link to be previewed.
---
 lisp/ol.el | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index 032610bad..dd6b07152 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -2177,27 +2177,34 @@ (defun org-link-preview-file (ov path link)
                 ((string-match-p (image-file-name-regexp) file))
                 ((file-exists-p file)))
       (let* ((width (org-display-inline-image--width link))
-	     (align (org-image--align link))
-             (image (org--create-inline-image file width)))
-        (when image            ; Add image to overlay
-	  ;; See bug#59902.  We cannot rely
-          ;; on Emacs to update image if the file
-          ;; has changed.
-          (image-flush image)
-	  (overlay-put ov 'display image)
-	  (overlay-put ov 'face 'default)
-	  (overlay-put ov 'keymap image-map)
-          (when align
-            (overlay-put
-             ov 'before-string
-             (propertize
-              " " 'face 'default
-              'display
-              (pcase align
-                ("center" `(space :align-to (- center (0.5 . ,image))))
-                ("right"  `(space :align-to (- right ,image)))))))
+             (image-data (org--create-inline-image file width)))
+        (when image-data
+	  (org-link-preview-image-data ov image-data link)
           t)))))
 
+(defun org-link-preview-image-data (ov image-data link)
+  "Display image data IMAGE-DATA in overlay OV for link.
+
+This intended to be used by functions which provide the :preview link property
+of links such as in `org-link-preview-file'"
+  (let ((align (org-image--align link)))
+    ;; See bug#59902.  We cannot rely
+    ;; on Emacs to update image if the file
+    ;; has changed.
+    (image-flush image-data)
+    (overlay-put ov 'display image-data)
+    (overlay-put ov 'face 'default)
+    (overlay-put ov 'keymap image-map)
+    (when align
+      (overlay-put
+       ov 'before-string
+       (propertize
+        " " 'face 'default
+        'display
+        (pcase align
+          ("center" `(space :align-to (- center (0.5 . ,image-data))))
+          ("right"  `(space :align-to (- right ,image-data)))))))))
+
 ;;;; "help" link type
 (defun org-link--open-help (path _)
   "Open a \"help\" type link.
-- 
2.45.2

Re: Link preview generation with new link preview property

[Karthik Chikmagalur]

>> ...
>> Considering this, it might be better to just split
>> `org-link-preview-file' into two public functions, where the "inside"
>> function accepts an image instead of a file.
>
> Agree.
> Would you be interested to create a patch?

Ihor, did you write a patch for this already?  I thought I saw something
from you but can't locate it now.

If not, I can submit a patch but it will be a week or so before I can
get to it.

Karthik

Re: Link preview generation with new link preview property

[Ihor Radchenko]

Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:

>>> ...
>>> Considering this, it might be better to just split
>>> `org-link-preview-file' into two public functions, where the "inside"
>>> function accepts an image instead of a file.
>>
>> Agree.
>> Would you be interested to create a patch?
>
> Ihor, did you write a patch for this already?  I thought I saw something
> from you but can't locate it now.

No, I did not. (I hope I did not forget 🤦)

> If not, I can submit a patch but it will be a week or so before I can
> get to it.

Maybe you can work with Bjorn on his patch instead?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>

Re: Link preview generation with new link preview property

[Björn Bidar]

Ihor Radchenko <yantar92@posteo.net> writes:

> Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:
>
>>>> ...
>>>> Considering this, it might be better to just split
>>>> `org-link-preview-file' into two public functions, where the "inside"
>>>> function accepts an image instead of a file.
>>>
>>> Agree.
>>> Would you be interested to create a patch?
>>
>> Ihor, did you write a patch for this already?  I thought I saw something
>> from you but can't locate it now.
>
> No, I did not. (I hope I did not forget 🤦)
>
>> If not, I can submit a patch but it will be a week or so before I can
>> get to it.
>
> Maybe you can work with Bjorn on his patch instead?

Anything is fine for me. Feedback on my last patch would be great.

Re: Link preview generation with new link preview property

[stardiviner]

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

I don't think so, my package org-link-beautify ported to the new
:preview mechanism. The file preview not always image. Could be text, or
icon etc. So pass in image object is not a good idea. It will limit
preview functionality extensibility.

[stardiviner]           <Hack this world!>      GPG key ID: 47C32433
IRC(freeenode): stardiviner                     Twitter:  @numbchild
Key fingerprint = 9BAA 92BC CDDD B9EF 3B36  CB99 B8C4 B8E5 47C3 2433
Blog: http://stardiviner.github.io/


On Tue, Dec 17, 2024 at 11:43 AM Karthik Chikmagalur <
karthikchikmagalur@gmail.com> wrote:

> >> Would it be possible to also handle image data in the function or
> >> refactor the org-link-preview-file function in a way that the geometry
> >> handling is done in a helper function which can be reused by other
> >> handlers.
> >
> > There is such function: `org-display-inline-image--width'.
> > We may consider exposing it as public function.
>
> This will not be enough, we will also have to expose `org-image--align'
> and the alignment code:
>
> (when align
>   (overlay-put
>    ov 'before-string
>    (propertize
>     " " 'face 'default
>     'display
>     (pcase align
>       ("center" `(space :align-to (- center (0.5 . ,image))))
>       ("right"  `(space :align-to (- right ,image)))))))
>
> I don't think we can expect third party packages to figure out and
> reimplement Org's image alignment logic.
>
> Considering this, it might be better to just split
> `org-link-preview-file' into two public functions, where the "inside"
> function accepts an image instead of a file.
>
> Karthik
>
>

[-- Attachment #2: Type: text/html, Size: 2425 bytes --]

Re: Link preview generation with new link preview property

[Karthik Chikmagalur]

> I don't think so, my package org-link-beautify ported to the new
> :preview mechanism. The file preview not always image. Could be text, or
> icon etc. So pass in image object is not a good idea. It will limit
> preview functionality extensibility.

The API will not be changed.  Org will provide a new
`org-link-preview-image' function that you can use to preview links
using image data.  You'll have to write the adapter to register this as
the :preview handler for a link type yourself.

Karthik

Re: Link preview generation with new link preview property

[Björn Bidar]

Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:

>> I don't think so, my package org-link-beautify ported to the new
>> :preview mechanism. The file preview not always image. Could be text, or
>> icon etc. So pass in image object is not a good idea. It will limit
>> preview functionality extensibility.
>
> The API will not be changed.  Org will provide a new
> `org-link-preview-image' function that you can use to preview links
> using image data.  You'll have to write the adapter to register this as
> the :preview handler for a link type yourself.
>
> Karthik

I'm also wondering how much could there be shared if the thing to be
displayed on the overlay isn't an image.

The org-image--align function talks about images but ultimately
doesn't do anything specific to image.
I think the alignment the only part which is generic.

Re: Link preview generation with new link preview property

[Ihor Radchenko]

Björn Bidar <bjorn.bidar@thaodan.de> writes:

> I'm also wondering how much could there be shared if the thing to be
> displayed on the overlay isn't an image.
>
> The org-image--align function talks about images but ultimately
> doesn't do anything specific to image.
> I think the alignment the only part which is generic.

If we provide a generic alignment, yes. We may or may not. Or we may
choose some other way to control non-image alignment.

I see no reason to generalize things needlessly. We can always do it is
we wish to. We cannot go back as easily if we commit prematurely.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>

Link preview generation with new link preview property

[Björn Bidar]

Hello,

I have a few question regarding the new link preview handler property
added recently. I ported the org-remoteimg package to it which was quite
easy, much cleaner with the new property.

The PR do port the package to this new handler property:
https://github.com/gaoDean/org-remoteimg/pull/4

However it left me with a few questions:
- Who is supposed to take care of the scaling/width of the preview
  image, the handler or the caller of the handler?
- When calling the link preview function for a description with prefix
  argument 1 I noticed that it doesn't get the contents of the
  description.
  Why is that? In this instance the link looked like this:
  [[https://stable.melpa.org/#/rpm-spec-mode][file:https://stable.melpa.org/packages/rpm-spec-mode-badge.svg]]

Thanks have a great day,

Björn