Re: [PATCH v9] Inline image display as part of a new org-link-preview system

[Ihor Radchenko]

Karthik Chikmagalur  writes:

> None of the above have been obsoleted.  Let me know if I should make
> them all obsolete aliases instead.

I think that an alias is fine.
At least, it is the approach I am leaning to recently.

>> Aside: I am wondering if we should also attach preview toggle to 
>> when ~org-cycle-inline-images-display~ is non-nil and there is a link at
>> point.
>
> I haven't added this.  I think it's better not to do this, since cycling
> is expected to only involve hiding/showing buffer contents.  Generating
> a preview might cause new external processes to be started.  I can add
> it if you think we should.

~org-cycle-inline-images-display~ already invokes link previews. What I
had in mind is extending it a tiny bit - I myself have this feature
locally and find it useful.

You do not have to implement it though. It is slightly out of scope of
this particular patchset.

> Subject: [PATCH 2/2] Org: Document preview API for arbitrary link types
> ...
> ...
> +  By default, only image links without a description are displayed.
> +  You can force displaying previews for all supported links using a
> +  numeric argument of ~1~ to toggle all previews in the active
> +  region, the link at point or the current section.  A numeric prefix
> +  argument of ~11~ will toggle previews in the whole buffer.

It is slightly inaccurate I think.  Once new :preview handlers are added
to non-image link types, the same rule about links with/without
description will apply to non-image links. So, we may better explain C-1
and C-11 arguments in more generic terms: "image links without
description" -> "previews for links without description"

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: Org-cite: Replace basic follow-processor with transient menu?

[Ihor Radchenko]

Tor-björn Claesson  writes:

> ...
>>(mapcar (pcase-lambda (`(,key ,desc ,fn ,transform))
>>(list ,key ,desc
>>  (lambda ()
>>(interactive)
>>(apply fn (eval transform)
>>org-cite-basic-follow-actions)))
>
> ...
> I will keep trying, but I must find the spare time to learn more about
> mapping and pattern matching in elisp, so this might take a while. In
> case Ihor wants to just fix it, please go ahead :-)

Check "11.4.4 Destructuring with ‘pcase’ Patterns" section of Elisp
manual. It has examples explaining how backquote pattern works.

> It would be good if we could match against the case of '(key desc
> suffix) as well, so that we could include otherwhere defined suffixes.

To not complicate things, I suggest something simple along the lines of

(lambda (&rest suffix-spec)
 (pcase suffix-spec
  (`(,key ,desc ,fn ,transform) ...)
  (`(,key ,desc ,suffix) ...)))

Remember that you can always play around with things in scratch buffer,
in IELM, or even just using C-x C-e from anywhere:

(pcase '(1 2 3 4)
  (`(,key ,desc ,fn ,transform)
(format "key: %s; desc: %s; fn:%s; transform: %s"
   key  desc fn transform))
  (`(,key ,desc ,suffix)
(format "key: %s; desc: %s; suffix:%s"
   key desc suffix)))
;; => "key: 1; desc: 2; fn:3; transform: 4"

(pcase '(1 2 3)
  (`(,key ,desc ,fn ,transform)
(format "key: %s; desc: %s; fn:%s; transform: %s"
   key  desc fn transform))
  (`(,key ,desc ,suffix)
(format "key: %s; desc: %s; suffix:%s"
   key desc suffix)))
;; => "key: 1; desc: 2; suffix:3"

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH v10] Inline image display as part of a new org-link-preview system

[Karthik Chikmagalur]

New version of Patch 0002 (documentation patch) attached.  Patch 0001
(code patch) is unchanged.

>> None of the above have been obsoleted.  Let me know if I should make
>> them all obsolete aliases instead.
>
> I think that an alias is fine.
> At least, it is the approach I am leaning to recently.

Okay, we can obsolete it later if deemed necessary.

>> I haven't added this.  I think it's better not to do this, since cycling
>> is expected to only involve hiding/showing buffer contents.  Generating
>> a preview might cause new external processes to be started.  I can add
>> it if you think we should.
>
> ~org-cycle-inline-images-display~ already invokes link previews.

While that's true, link previews don't start new OS processes right now.
They can for custom :preview types.

> You do not have to implement it though. It is slightly out of scope of
> this particular patchset.

Okay, let's leave it out. This patchset has become reasonably large
already.

>> Subject: [PATCH 2/2] Org: Document preview API for arbitrary link types
>> ...
>> ...
>> +  By default, only image links without a description are displayed.
>> +  You can force displaying previews for all supported links using a
>> +  numeric argument of ~1~ to toggle all previews in the active
>> +  region, the link at point or the current section.  A numeric prefix
>> +  argument of ~11~ will toggle previews in the whole buffer.
>
> It is slightly inaccurate I think.  Once new :preview handlers are added
> to non-image link types, the same rule about links with/without
> description will apply to non-image links. So, we may better explain C-1
> and C-11 arguments in more generic terms: "image links without
> description" -> "previews for links without description"

Fixed in the the documentation patch.

I have one more question for you:

The overlay property that is used to check if it corresponds to a link
preview is inconsistent with the rest of Org right now.  We are checking
for the non-nil overlay property `org-image-overlay'.

1.  Should we rename this property to `org-link-preview-overlay', or
`org-link-preview'?

2.  Org's LaTeX previews (both the existing and WIP versions) use a
different system.  They set two overlay properties: `category' set to
`org', and `org-overlay-type', set to `org-latex-preview'.  Should we
use a consistent set of properties to identify Org-related overlays?

This is an implementation detail and none of it really matters, but I
can unify them if required.

If this is not an issue and there's nothing else, I think we are good to
merge.

Karthik
>From 7d4656c3718e303330c02e0025817851d8761b5b Mon Sep 17 00:00:00 2001
From: Karthik Chikmagalur 
Date: Sun, 27 Oct 2024 20:22:59 -0700
Subject: [PATCH 2/2] Org: Document preview API for arbitrary link types

* etc/ORG-NEWS: Mention new commands and options for link preview.

* doc/org-manual.org: Add sections on
- previewing external links (Images and link previews), and
- adding the link preview feature to other link types (Adding
Hyperlink preview).
---
 doc/org-manual.org | 291 ++---
 etc/ORG-NEWS   |  93 +--
 2 files changed, 279 insertions(+), 105 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 94fabde3b..c94358779 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -11778,7 +11778,75 @@ ** Literal Examples
 the end of the current line.  Then the label is stored as a link
 =(label)=, for retrieval with {{{kbd(C-c C-l)}}}.
 
-** Images
+** Images and link previews
+:PROPERTIES:
+:DESCRIPTION: Preview links in the buffer.
+:END:
+
+Org mode can display previews of [[*Hyperlinks][hyperlinks]] inside Org buffers.  By
+default, only image links[fn::Image links are =file:= and
+=attachment:= links to existing image files; Emacs should be able to
+display the linked images (see ~image-types~ variable)] can be
+previewed inline, where the images are displayed in place of the link
+path.
+
+You can customize the previews as described in the [[*Adding Hyperlink
+preview]] section.  Link previews do not have to display images -- any
+kind of [[info:elisp#Overlay Properties][display decoration]] can be used.
+
+You can preview the supported link types in the whole buffer, in the
+current section, active region or at point with the following commands: 
+
+- {{{kbd(C-c C-x C-v)}}} (~org-link-preview~) ::
+
+  #+kindex: C-c C-x C-v
+  #+findex: org-link-preview
+  Create inline previews for external links in the active region, the
+  link at point or in the current section.  With a prefix argument,
+  clear link previews at point or in the current entry.  With a double
+  prefix argument, preview all links in the buffer.  With triple
+  prefix argument, hide previews for all links in the buffer.
+
+  By default, only links without descriptions are previewed.  You
+  can force displaying previews for all supported links (including
+  links with descriptions) using a numeric argument of ~1~.  This
+

Re: Noisy and unreliable (de-)tangling

[Ihor Radchenko]

Rudolf Adamkovič  writes:

> Ihor Radchenko  writes:
>
>> `org-id-link-to-org-use-id' should be respected once you use :comments
>> link
>
> As of now?  It is not.
>
> With `emacs -Q',
> ...

Confirmed.
This is likely a regression after we moved id: link handling into
`org-store-link-functions'. Because the code let-binds it to nil, id:
links are never created.

Probably, that cl-letf should be removed now. Or id: store function
should be kept there. I will need to look closer.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: Org-cite: Replace basic follow-processor with transient menu?

[Tor-björn Claesson]

This is a great learning experience, thank you Ihor and Jonas for the
friendly feedback!

(defcustom org-cite-basic-follow-actions
  '[["Open"
 ("b" "bibliography entry" org-cite-basic-goto (list (transient-scope) 0))] 
   
["Copy"
 ("d" "DOI" org-cite-basic-follow.copy-doi)]
["Browse"
 ("u" "url" org-cite-basic-follow.browse-url)]]
  "Hepp"
  :group 'org-cite
  :type 'sexp)

The bibliography entry demonstrates using any function (a bit ugly since
org-cite-basic-goto insists on the prefix argument).

(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
  [:class transient-columns
   :setup-children org-cite-basic-follow--setup
   :pad-keys t]
  (interactive)
  (if org-cite-basic-follow-ask
  (transient-setup 'org-cite-basic-follow nil nil
   :scope (list citation))
(org-cite-basic-goto citation prefix)))

Here I removed activating the transient menu if a prefix argument is
present. If org-cite-basic-follow-ask is nil (the default) and I want
to call org-cite-basic-goto with a prefix argument, this would cause the
transient menu to activate, which seems wrong.

(defun org-cite-basic-follow--setup (_)
  (transient-parse-suffixes
   'org-cite-basic-follow
   (cl-map 'vector (lambda (group)
  (cl-map 'vector (lambda (entry)
(pcase entry
  ((and (pred stringp) label)
   label)
  (`(,key ,desc ,fn ,transform)
   (list key desc `(lambda ()
 (interactive)
 (apply ',fn ,transform
  (`(,key ,desc ,suffix)
   (list key desc suffix
  group))
org-cite-basic-follow-actions)))

Prefixes of :class transient-columns expect a vector of vectors of a string and
lists, so we need to use a map that produces vectors.

This works. Do the code look OK to you? I'll prepare a patch if that is the
case.

Cheers,
Tor-björn