bug#69517: [PATCH] Make gnus cache work with group names having '/'

[James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Eric Abrahamsen wrote:

> The real problem (well, one of the real problems) is that we should just
> have two central routines for reading and writing active files, so that
> there are only two places to keep in sync. Instead we have those two
> places, and then a smattering of other functions in other places that do
> something similar, and also have to be kept in sync, and I haven't done
> that. At the very least I'll need to apply your patch from 65467.
>
> The other real problem is that gnus-cache is confused about whether it
> wants to be a select method, or a modified version of article saving.
> The presence of `gnus-use-long-file-name' indicates the latter, but the
> manual's instructions about the nnml select method indicates the former.
>
> I think it should be a select method, which means that the group
> directory should be created in "News/cache" the same way it is created
> at the top level: with the "/" replaced by "_", and everything else
> using the proper "left/right" group name. Perhaps "long file names" can
> still play a role, but if so that should be via
> `nnmail-use-long-file-names', and gnus-cache in general should behave
> like a nnmail backend.

Well, I have sort of, an approach based on my earlier patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-gnus-cache-work-with-group-names-having.patch --]
[-- Type: text/x-diff, Size: 3116 bytes --]

From 0f5d693dc439b11294b3bde3dc9129bdf45a2179 Mon Sep 17 00:00:00 2001
From: James Thomas <james.thomas@ahimsa.global>
Date: Thu, 14 Mar 2024 08:42:00 +0530
Subject: [PATCH] Make gnus cache work with group names having '/'

Make `gnus-cache-file-name` use the existing
`nnmail-group-pathname`.

* lisp/gnus/gnus-cache.el (gnus-cache-file-name)
(gnus-cache-update-article):
* lisp/gnus/nnmail.el (nnmail-group-pathname):
---
 lisp/gnus/gnus-cache.el | 27 +++++++--------------------
 lisp/gnus/nnmail.el     |  3 ++-
 2 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/lisp/gnus/gnus-cache.el b/lisp/gnus/gnus-cache.el
index 961219eee8f..7af02368d36 100644
--- a/lisp/gnus/gnus-cache.el
+++ b/lisp/gnus/gnus-cache.el
@@ -443,23 +443,9 @@ gnus-cache-member-of-class
       (and (not unread) (not ticked) (not dormant) (memq 'read class))))

 (defun gnus-cache-file-name (group article)
-  (expand-file-name
-   (if (stringp article) article (int-to-string article))
-   (file-name-as-directory
-    (expand-file-name
-     (nnheader-translate-file-chars
-      (if (gnus-use-long-file-name 'not-cache)
-	  group
-	(let ((group (nnheader-replace-duplicate-chars-in-string
-		      (nnheader-replace-chars-in-string group ?/ ?_)
-		      ?. ?_)))
-	  ;; Translate the first colon into a slash.
-	  (when (string-match ":" group)
-		  (setq group (concat (substring group 0 (match-beginning 0))
-				      "/" (substring group (match-end 0)))))
-	  (nnheader-replace-chars-in-string group ?. ?/)))
-      t)
-     gnus-cache-directory))))
+  (nnmail-group-pathname
+   group gnus-cache-directory
+   (if (stringp article) article (int-to-string article))))

 (defun gnus-cache-update-article (group article)
   "If ARTICLE is in the cache, remove it and re-enter it."
@@ -699,9 +685,10 @@ gnus-cache-generate-active
 			  (file-name-as-directory
 			   (expand-file-name gnus-cache-directory))))
 	     (directory-file-name directory))
-	    (nnheader-replace-chars-in-string
-	     (substring (directory-file-name directory) (match-end 0))
-	     ?/ ?.)))
+	    (url-unhex-string
+             (nnheader-replace-chars-in-string
+	      (substring (directory-file-name directory) (match-end 0))
+	      ?/ ?.))))
 	 nums alphs)
     (when top
       (gnus-message 5 "Generating the cache active file...")
diff --git a/lisp/gnus/nnmail.el b/lisp/gnus/nnmail.el
index fef12eebe09..a9f5b89c6fe 100644
--- a/lisp/gnus/nnmail.el
+++ b/lisp/gnus/nnmail.el
@@ -33,6 +33,7 @@
 (require 'mail-source)
 (require 'mm-util)
 (require 'gnus-int)
+(require 'browse-url)

 (autoload 'mail-send-and-exit "sendmail" nil t)

@@ -627,7 +628,7 @@ nnmail-group-pathname
   (concat
    (let ((dir (file-name-as-directory (expand-file-name dir))))
      (setq group (nnheader-replace-duplicate-chars-in-string
-		  (nnheader-replace-chars-in-string group ?/ ?_)
+		  (browse-url-url-encode-chars group "[/%]")
 		  ?. ?_))
      (setq group (nnheader-translate-file-chars group))
      ;; If this directory exists, we use it directly.
--
2.34.1


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


James Thomas wrote:

> +	(if (not nnmail-use-long-file-names)
> +            (nnheader-replace-chars-in-string group ?. ?/)
> +	  group))

Since directory names cannot have '/' they used to be replaced by '_' in
group names before conversion. But this makes it impossible, when
generating (non-existent) active files to know whether a '_' in the
directory name was _ or / originally.

The above patch tries a possible solution inspired from [1] but would
break existing users of the cache or agent (xref-find-references
"nnmail-group-pathname") who have groups with % or / in their names.

Seems to work in my limited testing. WDYT?

Regards,
James

1:
https://stackoverflow.com/questions/9847288/is-it-possible-to-use-in-a-filename#answer-9847319