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 <yantar92@gmail.com>]

[-- 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