[BUG] Issues in ol-gnus when storing links in nnvirtual and nnselect articles [9.7-pre (release_9.6.7-570-gd6f3ae.dirty @ /home/jschmidt/work/org-mode/lisp/)]

[Jens Schmidt <jschmidt4gnu@vodafonemail.de>]

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

Tags: patch

`org-store-link' has a number of related issues when storing links from
article buffers related to nnvirtual, nnselect, or nnir groups.  I
describe them here in prose without providing a full repro case, which
would be somewhat difficult to set up.  Just let me know if you think
you need more information, I have the data available.

The most obvious symptom is this:

- Create an nnselect group and open an article from that.  In the
   article buffer, do M-x org-store-link RET, then paste the link with
   C-c C-l in some Org mode buffer.  The resulting link looks like

     gnus:#E18xcfu-0004HT-00@fencepost.gnu.org

   That is, it lacks the group name before the hash sign.  Correct would
   have been:

     gnus:nnml+archive:test01#E18xcfu-0004HT-00@fencepost.gnu.org

   Starting with Emacs 30, you even more obviously get an error:

     Debugger entered--Lisp error: (wrong-type-argument 
number-or-marker-p nil)
       nnselect-article-group(nil)
       org-gnus-store-link()
       org-store-link(nil 1)
       funcall-interactively(org-store-link nil 1)
       call-interactively(org-store-link record nil)
       command-execute(org-store-link record)
       execute-extended-command(nil "org-store-link" nil)
       funcall-interactively(execute-extended-command nil 
"org-store-link" nil)
       call-interactively(execute-extended-command nil nil)
       command-execute(execute-extended-command)

Less obvious, occuring for nnvirtual groups:

- Create an nnvirtual group and open an article from that.  In the
   article buffer, do M-x org-store-link RET.  Observe the "current
   article arrow" in the fringe being set in the article header, even
   though that arrow should be used only in a summary buffer.

The root cause is that some of the Gnus functions used in
`org-gnus-store-link' must be called only in summary buffers, and not in
article buffers.  These are:

   gnus-summary-article-number
   nnselect-article-group

Not sure about these, but it is probably also better to call these in
summary buffers only:

   nnvirtual-map-article
   nnir-article-group

The remedy for these issues is simple: When calling above functions just
temporarily and unconditionally switch to the summary buffer with

   (with-current-buffer gnus-summary-buffer ...)

where buffer-local variable `gnus-summary-buffer' in an article buffer
points to the summary buffer where the articles comes from.  (And for
a summary buffer the variable points to the summary buffer itself.)

Finally, there is a related inefficiency when determining the article
header structure in function `org-gnus-store-link': Here the authors
indeed switch to the summary buffer when currently in the article
buffer, but using "user-land" interactive function
`gnus-article-show-summary' to do so where a simple

   (with-current-buffer gnus-summary-buffer ...)

would suffice.

Emacs  : GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 
3.24.24, cairo version 1.16.0)
  of 2023-07-20
Package: Org mode version 9.7-pre (release_9.6.7-570-gd6f3ae.dirty @ 
/home/jschmidt/work/org-mode/lisp/)

[-- Attachment #2: 0001-ol-gnus.el-Fix-issue-when-storing-links-from-Gnus-ar.patch --]
[-- Type: text/x-patch, Size: 2451 bytes --]

From e1bc9aefd4fd0080012c172d1c21c684a5b2fe51 Mon Sep 17 00:00:00 2001
From: farblos <43711228+farblos@users.noreply.github.com>
Date: Sat, 22 Jul 2023 10:30:19 +0200
Subject: [PATCH] ol-gnus.el: Fix issue when storing links from Gnus article
 buffers

* lisp/ol-gnus.el (org-gnus-store-link): Switch to
`gnus-summary-buffer' when calling functions that are intended to be
called only there.
---
 lisp/ol-gnus.el | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/lisp/ol-gnus.el b/lisp/ol-gnus.el
index 7c07ce045..f0e04ce66 100644
--- a/lisp/ol-gnus.el
+++ b/lisp/ol-gnus.el
@@ -137,27 +137,23 @@ If `org-store-link' was called with a prefix arg the meaning of
      (let* ((group
 	     (pcase (gnus-find-method-for-group gnus-newsgroup-name)
 	       (`(nnvirtual . ,_)
-		(save-excursion
-		  (car (nnvirtual-map-article (gnus-summary-article-number)))))
+		(with-current-buffer gnus-summary-buffer
+		  (save-excursion
+		    (car (nnvirtual-map-article (gnus-summary-article-number))))))
 	       (`(,(or `nnselect `nnir) . ,_)  ; nnir is for Emacs < 28.
-		(save-excursion
-		  (cond
-		   ((fboundp 'nnselect-article-group)
-		    (nnselect-article-group (gnus-summary-article-number)))
-		   ((fboundp 'nnir-article-group)
-		    (nnir-article-group (gnus-summary-article-number)))
-		   (t
-		    (error "No article-group variant bound")))))
+		(with-current-buffer gnus-summary-buffer
+		  (save-excursion
+		    (cond
+		     ((fboundp 'nnselect-article-group)
+		      (nnselect-article-group (gnus-summary-article-number)))
+		     ((fboundp 'nnir-article-group)
+		      (nnir-article-group (gnus-summary-article-number)))
+		     (t
+		      (error "No article-group variant bound"))))))
 	       (_ gnus-newsgroup-name)))
-	    (header (if (eq major-mode 'gnus-article-mode)
-			;; When in an article, first move to summary
-			;; buffer, with point on the summary of the
-			;; current article before extracting headers.
-			(save-window-excursion
-			  (save-excursion
-			    (gnus-article-show-summary)
-			    (gnus-summary-article-header)))
-		      (gnus-summary-article-header)))
+	    (header (with-current-buffer gnus-summary-buffer
+		      (save-excursion
+			(gnus-summary-article-header))))
 	    (from (mail-header-from header))
 	    (message-id (org-unbracket-string "<" ">" (mail-header-id header)))
 	    (date (org-trim (mail-header-date header)))
-- 
2.30.2