Subject: Re: bug#70994: [PATCH] Make cache regeneration work in group names with /

From: James Thomas <[email-masked]>
Date: Thu, 08 Aug 2024 06:46:48 +0530

Eric Abrahamsen wrote:

> James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <[email-masked]> writes:
>
>> Eric Abrahamsen wrote:
>>
>>> Stefan Kangas <[email-masked]> writes:
>>>
>>>> James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
>>>> text editors" <[email-masked]> writes:
>>>>
>>>>> Reproduction steps for bug:
>>>>>
>>>>> - emacs -Q
>>>>> - (setq gnus-secondary-select-methods
>>>>>     '((nnatom "github.com/vedang/pdf-tools/commits.atom")))
>>>>>   (setq gnus-select-method '(nnnil ""))
>>>>> - M-x gnus
>>>>> - Open a message in the new group and press *
>>>>> - Add the cache virtual server (info "(gnus) Creating a Virtual Server")
>>>>> - ^ (server buffer) and: g on the cache
>>>>> - RET to open (fails)
>>>>>
>>>>> This is a possible fix that I've tested only on my (limited) setup, for
>>>>> now:
>>>>
>>>> Eric, what do you think of the below patch?
>>>
>>> Thanks for the bump...
>>>
>>> James, wasn't this what bug#69517 was supposed to be fixing?
>>
>> You're right, but that was specifically the 'cache'. In regenerate, all
>> it sees is that the backend is nnml and there's nothing else special
>> about the server.
>
> Okay, thanks.
>
>>> I'm still feeling like we're patching pinhole leaks in a fundamentally
>>> broken system.
>>
>> Sorry if my patch made you think so, but I don't feel that way 🙂. This
>> feature is just tangential and things like slashes in group names are
>> bound to complicate things.
>
> I wasn't complaining about your code :) Just generally grumbling that
> this is so complex.
>
>> But let me see if I can whip up an alternative patch that does things in
>> a simpler way (I did say: 'possible' patch). One thing to decide is
>> whether '%'s are uncommon enough in group names to warrant special
>> treatment in a backend as fundamental as nnml.

I've gone ahead and assumed the above; so now the patch is way simpler.
(Btw I meant to say 'nnmail' above, not 'nnml'). It shouldn't be a
problem: think I remember only Gmail using a % at some point - and a
simple renaming fixes that - perhaps there should be a NEWS entry.

From dfe4498e361262eb1eb25e288bc7228678ac1a43 Mon Sep 17 00:00:00 2001
From: James Thomas <[email-masked]>
Date: Thu, 8 Aug 2024 06:40:15 +0530
Subject: [PATCH] Bugfix

* lisp/gnus/nnheader.el (nnheader-file-to-group): Reverse the
hexing in bug#69517.
---
 lisp/gnus/nnheader.el | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/lisp/gnus/nnheader.el b/lisp/gnus/nnheader.el
index ea679759f3e..db83eebe3cd 100644
--- a/lisp/gnus/nnheader.el
+++ b/lisp/gnus/nnheader.el
@@ -827,16 +827,17 @@ nnheader-replace-duplicate-chars-in-string

 (defun nnheader-file-to-group (file &optional top)
   "Return a group name based on FILE and TOP."
-  (nnheader-replace-chars-in-string
-   (if (not top)
-       file
-     (condition-case ()
-	 (substring (expand-file-name file)
-		    (length
-		     (expand-file-name
-		      (file-name-as-directory top))))
-       (error "")))
-   nnheader-directory-separator-character ?.))
+  (url-unhex-string
+   (nnheader-replace-chars-in-string
+    (if (not top)
+        file
+      (condition-case ()
+	  (substring (expand-file-name file)
+		     (length
+		      (expand-file-name
+		       (file-name-as-directory top))))
+        (error "")))
+    nnheader-directory-separator-character ?.)))

 (defun nnheader-message (level &rest args)
   "Message if the Gnus backends are talkative."
--
2.43.0

Note that for this to work with nnatom in the current upstream you'll
also need (ahem!) my patch in bug#65467: bug#71888 must be responsible.

> Without diving into this right now, it seems like this is something that
> should be governed by the nnmail abstract backend, from which nnml and
> friends inherit. I would dearly, dearly love it if all backends that
> might potentially create an on-disk directory from a group name would
> use the same code (applying the same user options) to do it, essentially
> transparently. It makes me nervous when various functions in various
> places repeat similar-but-not-quite-identical routines in encoding and
> decoding group names. I suppose that URL encoding/decoding functions
> might end up being an okay tool, but I wonder if Elisp doesn't already
> have some prior art here. I'll do a bit of reading.

That's worthwhile of course, but here, for the time being, I've decided
I'm only grappling with the new allowance of '/'s in group names. :-)

(A further improvement involves replacing the '/'s in the code with
'-directory-separator-character', but that's for another report)

Regards,
James