[BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

Hello,

Link abbreviations do not work inside property drawers and are instead
confused with internal links. The following org file illustrates this
behavior.

#+LINK: org-manual https://orgmode.org/manual/

* Heading
  :PROPERTIES:
  :myprop:  [[org-manual:Hyperlinks.html]]
  :END:
  - Opening this link will move point to heading below

* org-manual:Hyperlinks.html

* Another Heading
  :PROPERTIES:
  :myprop:  [[org-manual:Link-Abbreviations.html]]
  :END:
  - Opening this link will result in the following prompt:
    - No match: create this as a new heading? (yes or no)

* Yet Another Heading
  [[org-manual:Link-Abbreviations.html]]
  - Opening this link will browse
  https://orgmode.org/manual/Link-Abbreviations.html, as it should


I know that there is discussion of whether timestamps and links should
work or not inside property drawers, but since they currently work, I
think link abbreviations should be supported too. Let me know if you
agree and I'll debug the issue and send a patch with a fix proposal.

Regards,

Ignacio


Emacs  : GNU Emacs 29.0.50 (build 44, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
 of 2022-04-23
Package: Org mode version 9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> Link abbreviations do not work inside property drawers and are instead
> confused with internal links. The following org file illustrates this
> behavior.

This is to be expected. org-open-at-point does the following:

       ;; No valid link at point.  For convenience, look if something
       ;; looks like a link under point in some specific places.
       ((memq type '(comment comment-block node-property keyword))
	(call-interactively #'org-open-at-point-global))

That is, links are only partially recognised inside comments, node
properties, and keywords.

> I know that there is discussion of whether timestamps and links should
> work or not inside property drawers, but since they currently work, I
> think link abbreviations should be supported too. Let me know if you
> agree and I'll debug the issue and send a patch with a fix proposal.

I do not think that it is an easy problem to solve properly.

In the discussion you referenced [1] Nicolas raised the problem that we
cannot modify org-element parser to support links in more contexts as it
may create various issues (e.g. in exporter).

Without modifying the parser, supporting abbreviations will require code
duplication or referring to internal org-element functions - a fragile
approach.

I am currently tinkering with an idea to implement several parser
contexts, similar to optional argument in org-at-timestamp-p.
Basically, org-element parser could allow using multiple
named org-element-object-restrictions: 'agenda, 'export, 'lax, etc.
This is not ideal, as Org syntax will be more complex. However, multiple
interpretations of Org syntax are already implemented de facto (e.g.
org-at-timestamp-p). So, centralising the parsing into org-element could
be beneficial.

Probably, the contexts might be defined by let-bound
org-element-parser-context value.

Best,
Ihor

[1] https://orgmode.org/list/877d8llha9.fsf@nicolasgoaziou.fr 

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

Ihor Radchenko <yantar92@gmail.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> Link abbreviations do not work inside property drawers and are instead
>> confused with internal links. The following org file illustrates this
>> behavior.
>
> This is to be expected. org-open-at-point does the following:
>
>        ;; No valid link at point.  For convenience, look if something
>        ;; looks like a link under point in some specific places.
>        ((memq type '(comment comment-block node-property keyword))
> 	(call-interactively #'org-open-at-point-global))
>
> That is, links are only partially recognised inside comments, node
> properties, and keywords.
>
>> I know that there is discussion of whether timestamps and links should
>> work or not inside property drawers, but since they currently work, I
>> think link abbreviations should be supported too. Let me know if you
>> agree and I'll debug the issue and send a patch with a fix proposal.
>
> I do not think that it is an easy problem to solve properly.
>
> In the discussion you referenced [1] Nicolas raised the problem that we
> cannot modify org-element parser to support links in more contexts as it
> may create various issues (e.g. in exporter).
>
> Without modifying the parser, supporting abbreviations will require code
> duplication or referring to internal org-element functions - a fragile
> approach.
>
> I am currently tinkering with an idea to implement several parser
> contexts, similar to optional argument in org-at-timestamp-p.
> Basically, org-element parser could allow using multiple
> named org-element-object-restrictions: 'agenda, 'export, 'lax, etc.
> This is not ideal, as Org syntax will be more complex. However, multiple
> interpretations of Org syntax are already implemented de facto (e.g.
> org-at-timestamp-p). So, centralising the parsing into org-element could
> be beneficial.
>
> Probably, the contexts might be defined by let-bound
> org-element-parser-context value.
>
> Best,
> Ihor
>
> [1] https://orgmode.org/list/877d8llha9.fsf@nicolasgoaziou.fr 

Actually, I have investigated a little bit and I think the issue is more simple
than that:

- Link abbreviations rely in the variables `org-link-abbrev-alist' for
  global abbreviations and `org-link-abbrev-alist-local' for
  abbreviations defined with #+LINK for a single org file.

- `org-open-at-point-global' opens links with
  `org-link-open-from-string' as opposed to `org-open-at-point', which
  uses `org-link-open'.

- `org-link-open-from-string' ends up using `org-link-open' too, but
  inside a `with-temp-buffer' form:

    (defun org-link-open-from-string (s &optional arg)
      ...
      (pcase (with-temp-buffer
    	   (let ((org-inhibit-startup nil))
    	     (insert s)
    	     (org-mode)
    	     (goto-char (point-min))
    	     (org-element-link-parser)))
        (`nil (user-error "No valid link in %S" s))
        (link (org-link-open link arg))))

- But in a temporal buffer, `org-link-abbrev-alist-local' is nil, and
  thus all abbreviations defined with #+LINK are lost.


So a simple solution to this would be preserving the value of
`org-link-abbrev-alist-local' when switching to the temporal buffer. I
think this is orthogonal to the issue of the parser, and it's a bug on
its own, since as a user I would expect that evaluating
`org-link-open-from-string' would use my current buffer's local values
of variables.

What do you think? If you agree, I can send a patch fixing it in two
lines with something like this:

  (let ((org-link-abbrev-alist
         (append org-link-abbrev-alist org-link-abbrev-alist-local)))
    (pcase (with-temp-buffer ...) ...))

or this:

  (let ((tmp-var (org-link-abbrev-alist-local)))
    (pcase (with-temp-buffer
             (setq org-link-abbrev-alist-local tmp-var)
             ...) ...))

P.S. There is another variable defined with `defvar-local' in ol.el:
`org-target-link-regexp'. I don't know what it is used for but it could
potentially suffer from the same problem.

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> Actually, I have investigated a little bit and I think the issue is more simple
> than that:
>
> - Link abbreviations rely in the variables `org-link-abbrev-alist' for
>   global abbreviations and `org-link-abbrev-alist-local' for
>   abbreviations defined with #+LINK for a single org file.
>
> - `org-open-at-point-global' opens links with
>   `org-link-open-from-string' as opposed to `org-open-at-point', which
>   uses `org-link-open'.
>
> - `org-link-open-from-string' ends up using `org-link-open' too, but
>   inside a `with-temp-buffer' form:
> ...
> So a simple solution to this would be preserving the value of
> `org-link-abbrev-alist-local' when switching to the temporal buffer. I
> think this is orthogonal to the issue of the parser, and it's a bug on
> its own, since as a user I would expect that evaluating
> `org-link-open-from-string' would use my current buffer's local values
> of variables.

I am not sure if using current buffer in `org-link-open-from-string' is
to be expected. This function is not tied to buffer, just to string.
Imagine a situation when user function retrieves the link string from
another buffer and then calls `org-link-org-from-string' in the context
of current buffer. What you are proposing is going to change the current
behaviour of `org-link-open-from-string' beyond the discussed problem.

Instead of changing the default behaviour of
`org-link-open-from-string', I would introduce an optional parameter
holding the desired context. The parameter can be set to an Org buffer.
That buffer's context will be used.

> P.S. There is another variable defined with `defvar-local' in ol.el:
> `org-target-link-regexp'. I don't know what it is used for but it could
> potentially suffer from the same problem.

A clean solution would do something similar to
org-export--generate-copy-script.

Best,
Ihor

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

Ihor Radchenko <yantar92@gmail.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> So a simple solution to this would be preserving the value of
>> `org-link-abbrev-alist-local' when switching to the temporal buffer. I
>> think this is orthogonal to the issue of the parser, and it's a bug on
>> its own, since as a user I would expect that evaluating
>> `org-link-open-from-string' would use my current buffer's local values
>> of variables.
>
> I am not sure if using current buffer in `org-link-open-from-string' is
> to be expected. This function is not tied to buffer, just to string.
> Imagine a situation when user function retrieves the link string from
> another buffer and then calls `org-link-org-from-string' in the context
> of current buffer. What you are proposing is going to change the current
> behaviour of `org-link-open-from-string' beyond the discussed problem.
>
> Instead of changing the default behaviour of
> `org-link-open-from-string', I would introduce an optional parameter
> holding the desired context. The parameter can be set to an Org buffer.
> That buffer's context will be used.

I agree that changing the current behavior of
`org-link-open-from-string' may be problematic, however I don't think
that it's worth to introduce the optional argument just for this "bug".
I would just use the let from I suggested:

  (let ((org-link-abbrev-alist
         (append org-link-abbrev-alist org-link-abbrev-alist-local)))
    ...)

but instead of doing it in the definition of
`org-link-open-from-string', do it in `org-open-at-point-global',
like this:

(cond ((org-in-regexp org-link-any-re)
       (let ((org-link-abbrev-alist
              (append org-link-abbrev-alist org-link-abbrev-alist-local)))
	 (org-link-open-from-string (match-string-no-properties 0))))
      ...)

or in `org-open-at-point', like this:

  (cond
   ...
   ((memq type '(comment comment-block node-property keyword))
    (let ((org-link-abbrev-alist
           (append org-link-abbrev-alist org-link-abbrev-alist-local)))
      (call-interactively #'org-open-at-point-global)))
   ...)

What do you think?

--Ignacio

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> I agree that changing the current behavior of
> `org-link-open-from-string' may be problematic, however I don't think
> that it's worth to introduce the optional argument just for this
> "bug".

Makes sense. I am going ahead of what is currently on main. 

> (cond ((org-in-regexp org-link-any-re)
>        (let ((org-link-abbrev-alist
>               (append org-link-abbrev-alist org-link-abbrev-alist-local)))
> 	 (org-link-open-from-string (match-string-no-properties 0))))
>       ...)
> ...
> What do you think?

I do not like this idea. There is also org-link-abbrev-alist-local and
potentially other variables to be introduced in future. We should not
rely too much on internal workings of ol.el.

A better approach could be using org-link-expand-abbrev. It is an API
function and should be forward-compatible.

Best,
Ihor

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

>> (cond ((org-in-regexp org-link-any-re)
>>        (let ((org-link-abbrev-alist
>>               (append org-link-abbrev-alist org-link-abbrev-alist-local)))
>> 	 (org-link-open-from-string (match-string-no-properties 0))))
>>       ...)
>> ...
>> What do you think?
>
> I do not like this idea. There is also org-link-abbrev-alist-local and
> potentially other variables to be introduced in future. We should not
> rely too much on internal workings of ol.el.
>
> A better approach could be using org-link-expand-abbrev. It is an API
> function and should be forward-compatible.

Do you mean something like this?

(defun org-open-at-point-global ()
    ...
    (cond ((org-in-regexp org-link-any-re)
	   (org-link-open-from-string
            (org-link-expand-abbrev (match-string-no-properties 0))))
          ...))

Right now that is not enough because `org-link-expand-abbrev' only works
for links without square brackets, like "abbrev:suffix", and
`org-link-any-re' matches links with square brackets, like
"[[abbrev:suffix]]". That could be easily worked around in
`org-open-at-point-global' but maybe it would be better to change
`org-link-expand-abbrev' to work with both forms.

Let me know if you want me to send a patch implementing any of those
options or any other that you see fit. Otherwise, I know enough now to
fix the issue in my own config, so we can leave at that.

Regards,

--Ignacio

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

>> A better approach could be using org-link-expand-abbrev. It is an API
>> function and should be forward-compatible.
>
> Do you mean something like this?
>
> (defun org-open-at-point-global ()
>     ...
>     (cond ((org-in-regexp org-link-any-re)
> 	   (org-link-open-from-string
>             (org-link-expand-abbrev (match-string-no-properties 0))))
>           ...))
>
> Right now that is not enough because `org-link-expand-abbrev' only works
> for links without square brackets, like "abbrev:suffix", and
> `org-link-any-re' matches links with square brackets, like
> "[[abbrev:suffix]]". That could be easily worked around in
> `org-open-at-point-global' but maybe it would be better to change
> `org-link-expand-abbrev' to work with both forms.

Fair point. Then, the most future-proof way would be calling
org-element-link-parser. It should take care about abbrev expansion and
other edge cases. Then, you just need to use :raw-link property of the
parsed link element.

Best,
Ihor

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

Ihor Radchenko <yantar92@gmail.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>>> A better approach could be using org-link-expand-abbrev. It is an API
>>> function and should be forward-compatible.
>>
>> Do you mean something like this?
>>
>> (defun org-open-at-point-global ()
>>     ...
>>     (cond ((org-in-regexp org-link-any-re)
>> 	   (org-link-open-from-string
>>             (org-link-expand-abbrev (match-string-no-properties 0))))
>>           ...))
>>
>> Right now that is not enough because `org-link-expand-abbrev' only works
>> for links without square brackets, like "abbrev:suffix", and
>> `org-link-any-re' matches links with square brackets, like
>> "[[abbrev:suffix]]". That could be easily worked around in
>> `org-open-at-point-global' but maybe it would be better to change
>> `org-link-expand-abbrev' to work with both forms.
>
> Fair point. Then, the most future-proof way would be calling
> org-element-link-parser. It should take care about abbrev expansion and
> other edge cases. Then, you just need to use :raw-link property of the
> parsed link element.
>
> Best,
> Ihor

And then we come full circle, since that is what is being done already
but in a temporal buffer (so without access to
`org-link-abbrev-alist-local'), and your original concerns in your first
reply apply: doing it inside `org-open-at-point' would duplicate a lot
of code.

So I guess the issue is not as orthogonal as I though with the one of
the parser and it would be complicated to fix it properly, as you said
in your first email. If no one else has reported this problem or replied
to this thread, I guess that probably the best thing to do is fixing
this in my own config and move on for now:

I'll copy here the advice that fixes it, in case anyone needs to add it
to their config too:

(defun my-advice (orig-fun &rest args)
  (let ((org-link-abbrev-alist
         (append org-link-abbrev-alist org-link-abbrev-alist-local)))
    (apply orig-fun args)))

(advice-add 'org-open-at-point-global :around 'my-advice)

Best regards, and thanks for taking a look at this,

--Ignacio

Re: [BUG] link abbreviations do not work inside property drawers [9.5.2 (release_9.5.2-38-g682ccd @ /home/ignacio/repos/emacs/lisp/org/)]

[tony aldon]

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

I quietly followed the conversation.  Thank you for the advice.

On Wed, Apr 27, 2022 at 9:06 AM Ignacio Casso <ignaciocasso@hotmail.com>
wrote:

>
> Ihor Radchenko <yantar92@gmail.com> writes:
>
> > Ignacio Casso <ignaciocasso@hotmail.com> writes:
> >
> >>> A better approach could be using org-link-expand-abbrev. It is an API
> >>> function and should be forward-compatible.
> >>
> >> Do you mean something like this?
> >>
> >> (defun org-open-at-point-global ()
> >>     ...
> >>     (cond ((org-in-regexp org-link-any-re)
> >>         (org-link-open-from-string
> >>             (org-link-expand-abbrev (match-string-no-properties 0))))
> >>           ...))
> >>
> >> Right now that is not enough because `org-link-expand-abbrev' only works
> >> for links without square brackets, like "abbrev:suffix", and
> >> `org-link-any-re' matches links with square brackets, like
> >> "[[abbrev:suffix]]". That could be easily worked around in
> >> `org-open-at-point-global' but maybe it would be better to change
> >> `org-link-expand-abbrev' to work with both forms.
> >
> > Fair point. Then, the most future-proof way would be calling
> > org-element-link-parser. It should take care about abbrev expansion and
> > other edge cases. Then, you just need to use :raw-link property of the
> > parsed link element.
> >
> > Best,
> > Ihor
>
> And then we come full circle, since that is what is being done already
> but in a temporal buffer (so without access to
> `org-link-abbrev-alist-local'), and your original concerns in your first
> reply apply: doing it inside `org-open-at-point' would duplicate a lot
> of code.
>
> So I guess the issue is not as orthogonal as I though with the one of
> the parser and it would be complicated to fix it properly, as you said
> in your first email. If no one else has reported this problem or replied
> to this thread, I guess that probably the best thing to do is fixing
> this in my own config and move on for now:
>
> I'll copy here the advice that fixes it, in case anyone needs to add it
> to their config too:
>
> (defun my-advice (orig-fun &rest args)
>   (let ((org-link-abbrev-alist
>          (append org-link-abbrev-alist org-link-abbrev-alist-local)))
>     (apply orig-fun args)))
>
> (advice-add 'org-open-at-point-global :around 'my-advice)
>
> Best regards, and thanks for taking a look at this,
>
> --Ignacio
>
>

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