[BUG] C-c C-o on headline evaluates source code blocks with links inside [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[BUG] C-c C-o on headline evaluates source code blocks with links inside [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

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

Hello,

The recent threads about timestamps inside property drawers, which
mentioned the issue of timestamps and links being recognized in contexts
where they should not, had me experimenting a bit, and I found the
following bug (point 3) which was probably introduced by some change
regarding those issues:

1) `org-open-at-point' with point in a source code block calls and used
to call `org-babel-open-src-block-result'. This is not documented on the
docstring, but happens at least since Emacs 27' org built-in version,
9.4.4.

2) `org-open-at-point' with point in a link inside a source code block
also calls `org-babel-open-src-block-result' since at least org version
9.4.4. However I think that back when I was using Emacs 26, with
org-builtin version 9.1.9 or so, it opened the link. I think that when
that was fixed, the bug in 3) was introduced.

3) `org-open-at-point' with point in a headline collects all org links
in the body of the entry and offers to select one and open it, or if
there was only one it opens it directly. Links are collected using a
regular expression, so they match links inside a source code block. For
the selected link, `org-open-at-point' moves the point to the link and
calls itself recursively. In old versions of org, that means that it
opened the link. But in new versions, that means it evaluates the source
code block.

To reproduce this behavior, just copy the following entry into an org
buffer and type C-c C-o with point in the heading. It will evaluate the
source code block, instead of just messaging "No Links" as it would do
with a source code block without links.

* Heading
  #+begin_src emacs-lisp
    (message "https://orgmode.org/manual/")
  #+end_src

The following patch should fix it:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 807 bytes --]

From bc5092fdef512280b7d7d3aa04f1ba887360a759 Mon Sep 17 00:00:00 2001
From: Ignacio <ignacio.decasso@imdea.org>
Date: Thu, 24 Mar 2022 01:15:44 +0100
Subject: [PATCH] fixed bug

---
 lisp/org/org.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/org/org.el b/lisp/org/org.el
index 67c8f1cedf..0fff28af81 100644
--- a/lisp/org/org.el
+++ b/lisp/org/org.el
@@ -9063,7 +9063,8 @@ org-offer-links-in-entry
 	 (org-back-to-heading t)
 	 (setq end (save-excursion (outline-next-heading) (point)))
 	 (while (re-search-forward org-link-any-re end t)
-	   (push (match-string 0) links))
+           (when (eq (org-element-type (org-element-context)) 'link)
+	     (push (match-string 0) links)))
 	 (setq links (org-uniquify (reverse links))))
        (cond
 	((null links)
-- 
2.25.1


[-- Attachment #3: Type: text/plain, Size: 217 bytes --]



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

Re: [BUG] C-c C-o on headline evaluates source code blocks with links inside [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

> The following patch should fix it:
>
> [4. patch --- text/x-diff; 0001-fixed-bug.patch]
> From bc5092fdef512280b7d7d3aa04f1ba887360a759 Mon Sep 17 00:00:00 2001
> From: Ignacio <ignacio.decasso@imdea.org>
> Date: Thu, 24 Mar 2022 01:15:44 +0100
> Subject: [PATCH] fixed bug
>
> ---
>  lisp/org/org.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/org/org.el b/lisp/org/org.el
> index 67c8f1cedf..0fff28af81 100644
> --- a/lisp/org/org.el
> +++ b/lisp/org/org.el
> @@ -9063,7 +9063,8 @@ org-offer-links-in-entry
>  	 (org-back-to-heading t)
>  	 (setq end (save-excursion (outline-next-heading) (point)))
>  	 (while (re-search-forward org-link-any-re end t)
> -	   (push (match-string 0) links))
> +           (when (eq (org-element-type (org-element-context)) 'link)
> +	     (push (match-string 0) links)))
>  	 (setq links (org-uniquify (reverse links))))
>         (cond
>  	((null links)

Actually, this fix would suffer from the same problem as the fix for the
issue of org-agenda recognizing timestamps inside source code blocks,
and now links inside property drawers would no longer be opened using
C-c C-o with point on the headline.

So if we want to preserve that behavior the new lines should probably
be

 (when (memq (org-element-type (org-element-context)) '(link node-property))
   (push (match-string 0) links))

or something like that, with probably more element types in the list
beside link and node-property.

Re: [BUG] C-c C-o on headline evaluates source code blocks with links inside [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

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

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> 1) `org-open-at-point' with point in a source code block calls and used
> to call `org-babel-open-src-block-result'. This is not documented on the
> docstring, but happens at least since Emacs 27' org built-in version,
> 9.4.4.

Confirmed.

> 3) `org-open-at-point' with point in a headline collects all org links
> in the body of the entry and offers to select one and open it, or if
> there was only one it opens it directly. Links are collected using a
> regular expression, so they match links inside a source code block. For
> the selected link, `org-open-at-point' moves the point to the link and
> calls itself recursively. In old versions of org, that means that it
> opened the link. But in new versions, that means it evaluates the source
> code block.
>
> To reproduce this behavior, just copy the following entry into an org
> buffer and type C-c C-o with point in the heading. It will evaluate the
> source code block, instead of just messaging "No Links" as it would do
> with a source code block without links.
>
> * Heading
>   #+begin_src emacs-lisp
>     (message "https://orgmode.org/manual/")
>   #+end_src

Confirmed.

> The following patch should fix it:
>
> -	   (push (match-string 0) links))
> +           (when (eq (org-element-type (org-element-context)) 'link)
> +	     (push (match-string 0) links)))

This will skip links inside property drawers, for example.
Attaching an alternative patch.

Best,
Ihor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-open-at-point-Do-not-list-links-under-headline-t.patch --]
[-- Type: text/x-patch, Size: 12129 bytes --]

From ca6afac5d68a5e83af6d8078d09a163c34d62e2e Mon Sep 17 00:00:00 2001
Message-Id: <ca6afac5d68a5e83af6d8078d09a163c34d62e2e.1648282516.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Sat, 26 Mar 2022 16:08:57 +0800
Subject: [PATCH] org-open-at-point: Do not list links under headline that
 cannot be opened

* lisp/org-element.el (org-element-context): Do not alter match-data.
* lisp/org.el (org-open-at-point): Update docstring listing that
`org-open-at-point' opens src-blocks and citations.
(org-offer-links-in-entry): Do not display links within invalid contexts.

Reported in https://list.orgmode.org/PAXPR06MB77609E8C8E769CD7D769FA4BC6199@PAXPR06MB7760.eurprd06.prod.outlook.com/
---
 lisp/org-element.el | 211 ++++++++++++++++++++++----------------------
 lisp/org.el         |  13 ++-
 2 files changed, 117 insertions(+), 107 deletions(-)

diff --git a/lisp/org-element.el b/lisp/org-element.el
index 28339c1b8..c2ca9c62c 100644
--- a/lisp/org-element.el
+++ b/lisp/org-element.el
@@ -7770,113 +7770,114 @@ (defun org-element-context (&optional element)
 Providing it allows for quicker computation."
   (catch 'objects-forbidden
     (org-with-wide-buffer
-     (let* ((pos (point))
-	    (element (or element (org-element-at-point)))
-	    (type (org-element-type element))
-	    (post (org-element-property :post-affiliated element)))
-       ;; If point is inside an element containing objects or
-       ;; a secondary string, narrow buffer to the container and
-       ;; proceed with parsing.  Otherwise, return ELEMENT.
-       (cond
-	;; At a parsed affiliated keyword, check if we're inside main
-	;; or dual value.
-	((and post (< pos post))
-	 (beginning-of-line)
-	 (let ((case-fold-search t)) (looking-at org-element--affiliated-re))
-	 (cond
-	  ((not (member-ignore-case (match-string 1)
+     (save-match-data
+       (let* ((pos (point))
+	      (element (or element (org-element-at-point)))
+	      (type (org-element-type element))
+	      (post (org-element-property :post-affiliated element)))
+         ;; If point is inside an element containing objects or
+         ;; a secondary string, narrow buffer to the container and
+         ;; proceed with parsing.  Otherwise, return ELEMENT.
+         (cond
+	  ;; At a parsed affiliated keyword, check if we're inside main
+	  ;; or dual value.
+	  ((and post (< pos post))
+	   (beginning-of-line)
+	   (let ((case-fold-search t)) (looking-at org-element--affiliated-re))
+	   (cond
+	    ((not (member-ignore-case (match-string 1)
 				    org-element-parsed-keywords))
-	   (throw 'objects-forbidden element))
-	  ((< (match-end 0) pos)
-	   (narrow-to-region (match-end 0) (line-end-position)))
-	  ((and (match-beginning 2)
-		(>= pos (match-beginning 2))
-		(< pos (match-end 2)))
-	   (narrow-to-region (match-beginning 2) (match-end 2)))
+	     (throw 'objects-forbidden element))
+	    ((< (match-end 0) pos)
+	     (narrow-to-region (match-end 0) (line-end-position)))
+	    ((and (match-beginning 2)
+		  (>= pos (match-beginning 2))
+		  (< pos (match-end 2)))
+	     (narrow-to-region (match-beginning 2) (match-end 2)))
+	    (t (throw 'objects-forbidden element)))
+	   ;; Also change type to retrieve correct restrictions.
+	   (setq type 'keyword))
+	  ;; At an item, objects can only be located within tag, if any.
+	  ((eq type 'item)
+	   (let ((tag (org-element-property :tag element)))
+	     (if (or (not tag) (/= (line-beginning-position) post))
+	         (throw 'objects-forbidden element)
+	       (beginning-of-line)
+	       (search-forward tag (line-end-position))
+	       (goto-char (match-beginning 0))
+	       (if (and (>= pos (point)) (< pos (match-end 0)))
+		   (narrow-to-region (point) (match-end 0))
+	         (throw 'objects-forbidden element)))))
+	  ;; At an headline or inlinetask, objects are in title.
+	  ((memq type '(headline inlinetask))
+	   (let ((case-fold-search nil))
+	     (goto-char (org-element-property :begin element))
+	     (looking-at org-complex-heading-regexp)
+	     (let ((end (match-end 4)))
+	       (if (not end) (throw 'objects-forbidden element)
+	         (goto-char (match-beginning 4))
+	         (when (looking-at org-element-comment-string)
+		   (goto-char (match-end 0)))
+	         (if (>= (point) end) (throw 'objects-forbidden element)
+		   (narrow-to-region (point) end))))))
+	  ;; At a paragraph, a table-row or a verse block, objects are
+	  ;; located within their contents.
+	  ((memq type '(paragraph table-row verse-block))
+	   (let ((cbeg (org-element-property :contents-begin element))
+	         (cend (org-element-property :contents-end element)))
+	     ;; CBEG is nil for table rules.
+	     (if (and cbeg cend (>= pos cbeg)
+		      (or (< pos cend) (and (= pos cend) (eobp))))
+	         (narrow-to-region cbeg cend)
+	       (throw 'objects-forbidden element))))
 	  (t (throw 'objects-forbidden element)))
-	 ;; Also change type to retrieve correct restrictions.
-	 (setq type 'keyword))
-	;; At an item, objects can only be located within tag, if any.
-	((eq type 'item)
-	 (let ((tag (org-element-property :tag element)))
-	   (if (or (not tag) (/= (line-beginning-position) post))
-	       (throw 'objects-forbidden element)
-	     (beginning-of-line)
-	     (search-forward tag (line-end-position))
-	     (goto-char (match-beginning 0))
-	     (if (and (>= pos (point)) (< pos (match-end 0)))
-		 (narrow-to-region (point) (match-end 0))
-	       (throw 'objects-forbidden element)))))
-	;; At an headline or inlinetask, objects are in title.
-	((memq type '(headline inlinetask))
-	 (let ((case-fold-search nil))
-	   (goto-char (org-element-property :begin element))
-	   (looking-at org-complex-heading-regexp)
-	   (let ((end (match-end 4)))
-	     (if (not end) (throw 'objects-forbidden element)
-	       (goto-char (match-beginning 4))
-	       (when (looking-at org-element-comment-string)
-		 (goto-char (match-end 0)))
-	       (if (>= (point) end) (throw 'objects-forbidden element)
-		 (narrow-to-region (point) end))))))
-	;; At a paragraph, a table-row or a verse block, objects are
-	;; located within their contents.
-	((memq type '(paragraph table-row verse-block))
-	 (let ((cbeg (org-element-property :contents-begin element))
-	       (cend (org-element-property :contents-end element)))
-	   ;; CBEG is nil for table rules.
-	   (if (and cbeg cend (>= pos cbeg)
-		    (or (< pos cend) (and (= pos cend) (eobp))))
-	       (narrow-to-region cbeg cend)
-	     (throw 'objects-forbidden element))))
-	(t (throw 'objects-forbidden element)))
-       (goto-char (point-min))
-       (let ((restriction (org-element-restriction type))
-	     (parent element)
-	     last)
-	 (catch 'exit
-	   (while t
-	     (let ((next (org-element--object-lex restriction)))
-	       (when next (org-element-put-property next :parent parent))
-	       ;; Process NEXT, if any, in order to know if we need to
-	       ;; skip it, return it or move into it.
-	       (if (or (not next) (> (org-element-property :begin next) pos))
-		   (throw 'exit (or last parent))
-		 (let ((end (org-element-property :end next))
-		       (cbeg (org-element-property :contents-begin next))
-		       (cend (org-element-property :contents-end next)))
-		   (cond
-		    ;; Skip objects ending before point.  Also skip
-		    ;; objects ending at point unless it is also the
-		    ;; end of buffer, since we want to return the
-		    ;; innermost object.
-		    ((and (<= end pos) (/= (point-max) end))
-		     (goto-char end)
-		     ;; For convenience, when object ends at POS,
-		     ;; without any space, store it in LAST, as we
-		     ;; will return it if no object starts here.
-		     (when (and (= end pos)
-				(not (memq (char-before) '(?\s ?\t))))
-		       (setq last next)))
-		    ;; If POS is within a container object, move into
-		    ;; that object.
-		    ((and cbeg cend
-			  (>= pos cbeg)
-			  (or (< pos cend)
-			      ;; At contents' end, if there is no
-			      ;; space before point, also move into
-			      ;; object, for consistency with
-			      ;; convenience feature above.
-			      (and (= pos cend)
-				   (or (= (point-max) pos)
-				       (not (memq (char-before pos)
-						  '(?\s ?\t)))))))
-		     (goto-char cbeg)
-		     (narrow-to-region (point) cend)
-		     (setq parent next)
-		     (setq restriction (org-element-restriction next)))
-		    ;; Otherwise, return NEXT.
-		    (t (throw 'exit next)))))))))))))
+         (goto-char (point-min))
+         (let ((restriction (org-element-restriction type))
+	       (parent element)
+	       last)
+	   (catch 'exit
+	     (while t
+	       (let ((next (org-element--object-lex restriction)))
+	         (when next (org-element-put-property next :parent parent))
+	         ;; Process NEXT, if any, in order to know if we need to
+	         ;; skip it, return it or move into it.
+	         (if (or (not next) (> (org-element-property :begin next) pos))
+		     (throw 'exit (or last parent))
+		   (let ((end (org-element-property :end next))
+		         (cbeg (org-element-property :contents-begin next))
+		         (cend (org-element-property :contents-end next)))
+		     (cond
+		      ;; Skip objects ending before point.  Also skip
+		      ;; objects ending at point unless it is also the
+		      ;; end of buffer, since we want to return the
+		      ;; innermost object.
+		      ((and (<= end pos) (/= (point-max) end))
+		       (goto-char end)
+		       ;; For convenience, when object ends at POS,
+		       ;; without any space, store it in LAST, as we
+		       ;; will return it if no object starts here.
+		       (when (and (= end pos)
+				  (not (memq (char-before) '(?\s ?\t))))
+		         (setq last next)))
+		      ;; If POS is within a container object, move into
+		      ;; that object.
+		      ((and cbeg cend
+			    (>= pos cbeg)
+			    (or (< pos cend)
+			        ;; At contents' end, if there is no
+			        ;; space before point, also move into
+			        ;; object, for consistency with
+			        ;; convenience feature above.
+			        (and (= pos cend)
+				     (or (= (point-max) pos)
+				         (not (memq (char-before pos)
+						    '(?\s ?\t)))))))
+		       (goto-char cbeg)
+		       (narrow-to-region (point) cend)
+		       (setq parent next)
+		       (setq restriction (org-element-restriction next)))
+		      ;; Otherwise, return NEXT.
+		      (t (throw 'exit next))))))))))))))
 
 (defun org-element-lineage (datum &optional types with-self)
   "List all ancestors of a given element or object.
diff --git a/lisp/org.el b/lisp/org.el
index 5e3d0b333..56dd6cc7d 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -8980,7 +8980,9 @@ (defvar org-open-at-point-functions nil
 they must return nil.")
 
 (defun org-open-at-point (&optional arg)
-  "Open link, timestamp, footnote or tags at point.
+  "Open thing at point.
+The thing can be a link, citation, timestamp, footnote, src-block or
+tags.
 
 When point is on a link, follow it.  Normally, files will be
 opened by an appropriate application.  If the optional prefix
@@ -8995,6 +8997,10 @@ (defun org-open-at-point (&optional arg)
 found.  If it is on a reference, move to the associated
 definition.
 
+When point is on a src-block of inline src-block, open its result.
+
+When point is on a citation, follow it.
+
 When point is on a headline, display a list of every link in the
 entry, so it is possible to pick one, or all, of them.  If point
 is on a tag, call `org-tags-view' instead.
@@ -9113,7 +9119,10 @@ (defun org-offer-links-in-entry (buffer marker &optional nth zero)
 	 (org-back-to-heading t)
 	 (setq end (save-excursion (outline-next-heading) (point)))
 	 (while (re-search-forward org-link-any-re end t)
-	   (push (match-string 0) links))
+           ;; Only consider valid links or links openable via
+           ;; `org-open-at-point'.
+           (when (memq (org-element-type (org-element-context)) '(link comment comment-block node-property keyword))
+	     (push (match-string 0) links)))
 	 (setq links (org-uniquify (reverse links))))
        (cond
 	((null links)
-- 
2.34.1

Re: [BUG] C-c C-o on headline evaluates source code blocks with links inside [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ihor Radchenko <yantar92@gmail.com> writes:

> This will skip links inside property drawers, for example.
> Attaching an alternative patch.

Applied onto main via 057df6cce.

Best,
Ihor