bug#34973: 27.0.50; gnus-summary-move-article fails due to remhash

["Basil L. Contovounesios" <contovob@tcd.ie>]

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

tags 34973 patch
quit


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-Gnus-duplicate-article-unsuppression.patch --]
[-- Type: text/x-diff, Size: 3560 bytes --]

From 9480af8c32389c0916cf83d3d2f12d62b0f0efc2 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sun, 24 Mar 2019 14:12:58 +0000
Subject: [PATCH] Fix Gnus duplicate article unsuppression

* lisp/gnus/gnus-sum.el (gnus-summary-mode-group): Declare before
first use to silence byte-compiler.
(gnus-summary-select-article): Simplify boolean expression.
(gnus-summary-move-article): Do not try to unsuppress article when
duplicate suppression is disabled. (bug#34973, bug#34974)
---
 lisp/gnus/gnus-sum.el | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/lisp/gnus/gnus-sum.el b/lisp/gnus/gnus-sum.el
index 85c902a5e4..f09c0fbb58 100644
--- a/lisp/gnus/gnus-sum.el
+++ b/lisp/gnus/gnus-sum.el
@@ -3053,6 +3053,11 @@ bookmark-make-record-function
 \f
 (defvar bidi-paragraph-direction)
 
+(defvar gnus-summary-mode-group nil
+  "Variable for communication with `gnus-summary-mode'.
+Allows the `gnus-newsgroup-name' local variable to be set before
+the summary mode hooks are run.")
+
 (define-derived-mode gnus-summary-mode gnus-mode "Summary"
   "Major mode for reading articles.
 \\<gnus-summary-mode-map>
@@ -3468,11 +3473,6 @@ gnus-summary-set-article-display-arrow
 					       (point)
 					       (current-buffer))))))
 
-(defvar gnus-summary-mode-group nil
-  "Variable for communication with `gnus-summary-mode'.
-Allows the `gnus-newsgroup-name' local variable to be set before
-the summary mode hooks are run.")
-
 (defun gnus-summary-setup-buffer (group)
   "Initialize summary buffer for GROUP.
 This function does all setup work that relies on the specific
@@ -7744,7 +7744,7 @@ gnus-summary-select-article
   (unless (derived-mode-p 'gnus-summary-mode)
     (set-buffer gnus-summary-buffer))
   (let ((article (or article (gnus-summary-article-number)))
-	(all-headers (not (not all-headers))) ;Must be t or nil.
+        (all-headers (and all-headers t)) ; Must be t or nil.
 	gnus-summary-display-article-function)
     (and (not pseudo)
 	 (gnus-summary-article-pseudo-p article)
@@ -9978,13 +9978,12 @@ gnus-summary-move-article
 		 (crosspost "Crosspost" "Crossposting")))
 	(copy-buf (save-excursion
 		    (nnheader-set-temp-buffer " *copy article*")))
-	art-group to-method new-xref article to-groups
+        art-group to-method new-xref to-groups
 	articles-to-update-marks encoded)
     (unless (assq action names)
       (error "Unknown action %s" action))
     ;; Read the newsgroup name.
-    (when (and (not to-newsgroup)
-	       (not select-method))
+    (unless (or to-newsgroup select-method)
       (if (and gnus-move-split-methods
 	       (not
 		(and (memq gnus-current-article articles)
@@ -10029,8 +10028,7 @@ gnus-summary-move-article
 		  (or (car select-method)
 		      (gnus-group-decoded-name to-newsgroup))
 		  articles)
-    (while articles
-      (setq article (pop articles))
+    (dolist (article articles)
       ;; Set any marks that may have changed in the summary buffer.
       (when gnus-preserve-marks
 	(gnus-summary-push-marks-to-backend article))
@@ -10039,8 +10037,9 @@ gnus-summary-move-article
        (cond
 	;; Move the article.
 	((eq action 'move)
-	 ;; Remove this article from future suppression.
-	 (gnus-dup-unsuppress-article article)
+         (when gnus-suppress-duplicates
+           ;; Remove this article from future suppression.
+           (gnus-dup-unsuppress-article article))
 	 (let* ((from-method (gnus-find-method-for-group
 			      gnus-newsgroup-name))
 		(to-method (or select-method
-- 
2.20.1


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


Deus Max <deusmax@gmx.com> writes:

> Reading the manual
> (https://www.gnu.org/software/emacs/manual/html_node/elisp/Hash-Access.html#Hash-Access)
> on remhash, it says: "In Emacs Lisp, remhash always returns nil.", so I
> modified the calling function gnus-dup-unsuppress-article in gnus-dup.el, as follows:
>
> 157c157,159
> <       (remhash id gnus-dup-hashtb))))
> ---
>>       (if (hash-table-p gnus-dup-hashtb)
>>           (remhash id gnus-dup-hashtb)
>>         nil))))
>
> This patch seems to have fixed the problem and now
> gnus-summary-move-article works fine. !
> the value of gnus-dup-hashtb was nil, for the problem runs.

The return value of gnus-dup-unsuppress-article doesn't matter, only
that remhash not be passed a nil gnus-dup-hashtb.

In other words, gnus-dup-unsuppress-article should either not be called
or should handle the case when it is called before gnus-dup-open has
initialised gnus-dup-hashtb.

I attach a patch which follows the apparent gnus-sum.el convention of
guarding calls to gnus-dup.el functions based on the value of
gnus-suppress-duplicates.

This fixes the immediate issue at hand, but I think there may still be a
problem when gnus-suppress-duplicates is enabled at a later time.  I
think that deserves a separate bug report, though.

Thanks,

-- 
Basil