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

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

[James Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Make cache regeneration work in group names with / --]
[-- Type: text/x-patch, Size: 3253 bytes --]

From 1252b4541b265f6da13c07d9a2ce16fdecd51731 Mon Sep 17 00:00:00 2001
From: James Thomas <jimjoe@gmx.net>
Date: Fri, 17 May 2024 10:18:14 +0530
Subject: [PATCH] Make cache regeneration work in group names with /

* lisp/gnus/nnheader.el (nnheader-file-to-group): Account for /
in the last part of the group name.
* lisp/gnus/nnmail.el (nnmail-group-pathname): Url-encode only
the last part of the group name.
---
 lisp/gnus/nnheader.el | 25 +++++++++++++++----------
 lisp/gnus/nnmail.el   | 19 ++++++++++++++++---
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/lisp/gnus/nnheader.el b/lisp/gnus/nnheader.el
index ea679759f3e..85a9f1bd8af 100644
--- a/lisp/gnus/nnheader.el
+++ b/lisp/gnus/nnheader.el
@@ -827,16 +827,21 @@ 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 ?.))
+  (setq file
+        (if (not top)
+            file
+          (condition-case ()
+	      (substring (expand-file-name file)
+		         (length
+		          (expand-file-name
+		           (file-name-as-directory top))))
+            (error ""))))
+  ;; The last part could have slashes.
+  (concat
+   (nnheader-replace-chars-in-string
+    (file-name-directory file)
+    nnheader-directory-separator-character ?.)
+   (url-unhex-string (file-name-nondirectory file))))

 (defun nnheader-message (level &rest args)
   "Message if the Gnus backends are talkative."
diff --git a/lisp/gnus/nnmail.el b/lisp/gnus/nnmail.el
index a9f5b89c6fe..0e1bcf849ce 100644
--- a/lisp/gnus/nnmail.el
+++ b/lisp/gnus/nnmail.el
@@ -627,9 +627,21 @@ nnmail-group-pathname
   "Make file name for GROUP."
   (concat
    (let ((dir (file-name-as-directory (expand-file-name dir))))
+     (setq group
+           ;; The last part is allowed slashes. So url-encode.
+           (let ((split-list (split-string group "\\.")))
+             (string-join
+              (append
+               (butlast split-list)
+               (list (browse-url-url-encode-chars
+                      (car (last split-list))
+                      (concat "[%"
+                       (char-to-string
+                        nnheader-directory-separator-character)
+                       "]"))))
+	      ".")))
      (setq group (nnheader-replace-duplicate-chars-in-string
-		  (browse-url-url-encode-chars group "[/%]")
-		  ?. ?_))
+		  group ?. ?_))
      (setq group (nnheader-translate-file-chars group))
      ;; If this directory exists, we use it directly.
      (file-name-as-directory
@@ -638,7 +650,8 @@ nnmail-group-pathname
 	  (expand-file-name group dir)
 	;; If not, we translate dots into slashes.
 	(expand-file-name
-	 (nnheader-replace-chars-in-string group ?. ?/)
+	 (nnheader-replace-chars-in-string
+          group ?. nnheader-directory-separator-character)
 	 dir))))
    (or file "")))

--
2.40.1


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


Regards,
James

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

[Stefan Kangas]

James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> 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?

> From 1252b4541b265f6da13c07d9a2ce16fdecd51731 Mon Sep 17 00:00:00 2001
> From: James Thomas <jimjoe@gmx.net>
> Date: Fri, 17 May 2024 10:18:14 +0530
> Subject: [PATCH] Make cache regeneration work in group names with /
>
> * lisp/gnus/nnheader.el (nnheader-file-to-group): Account for /
> in the last part of the group name.
> * lisp/gnus/nnmail.el (nnmail-group-pathname): Url-encode only
> the last part of the group name.
> ---
>  lisp/gnus/nnheader.el | 25 +++++++++++++++----------
>  lisp/gnus/nnmail.el   | 19 ++++++++++++++++---
>  2 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/lisp/gnus/nnheader.el b/lisp/gnus/nnheader.el
> index ea679759f3e..85a9f1bd8af 100644
> --- a/lisp/gnus/nnheader.el
> +++ b/lisp/gnus/nnheader.el
> @@ -827,16 +827,21 @@ 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 ?.))
> +  (setq file
> +        (if (not top)
> +            file
> +          (condition-case ()
> +	      (substring (expand-file-name file)
> +		         (length
> +		          (expand-file-name
> +		           (file-name-as-directory top))))
> +            (error ""))))
> +  ;; The last part could have slashes.
> +  (concat
> +   (nnheader-replace-chars-in-string
> +    (file-name-directory file)
> +    nnheader-directory-separator-character ?.)
> +   (url-unhex-string (file-name-nondirectory file))))
>
>  (defun nnheader-message (level &rest args)
>    "Message if the Gnus backends are talkative."
> diff --git a/lisp/gnus/nnmail.el b/lisp/gnus/nnmail.el
> index a9f5b89c6fe..0e1bcf849ce 100644
> --- a/lisp/gnus/nnmail.el
> +++ b/lisp/gnus/nnmail.el
> @@ -627,9 +627,21 @@ nnmail-group-pathname
>    "Make file name for GROUP."
>    (concat
>     (let ((dir (file-name-as-directory (expand-file-name dir))))
> +     (setq group
> +           ;; The last part is allowed slashes. So url-encode.
> +           (let ((split-list (split-string group "\\.")))
> +             (string-join
> +              (append
> +               (butlast split-list)
> +               (list (browse-url-url-encode-chars
> +                      (car (last split-list))
> +                      (concat "[%"
> +                       (char-to-string
> +                        nnheader-directory-separator-character)
> +                       "]"))))
> +	      ".")))
>       (setq group (nnheader-replace-duplicate-chars-in-string
> -		  (browse-url-url-encode-chars group "[/%]")
> -		  ?. ?_))
> +		  group ?. ?_))
>       (setq group (nnheader-translate-file-chars group))
>       ;; If this directory exists, we use it directly.
>       (file-name-as-directory
> @@ -638,7 +650,8 @@ nnmail-group-pathname
>  	  (expand-file-name group dir)
>  	;; If not, we translate dots into slashes.
>  	(expand-file-name
> -	 (nnheader-replace-chars-in-string group ?. ?/)
> +	 (nnheader-replace-chars-in-string
> +          group ?. nnheader-directory-separator-character)
>  	 dir))))
>     (or file "")))
>
> --
> 2.40.1
>
>
> Regards,
> James

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

[Eric Abrahamsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> 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? I'm still
feeling like we're patching pinhole leaks in a fundamentally broken
system.

For the cache active file, wouldn't it be easier just to wrap the group
name in double quotes?

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

[James Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eric Abrahamsen wrote:

> Stefan Kangas <stefankangas@gmail.com> writes:
>
>> James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
>> text editors" <bug-gnu-emacs@gnu.org> 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.

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

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.

Regards,
James

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

[Eric Abrahamsen]

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

> Eric Abrahamsen wrote:
>
>> Stefan Kangas <stefankangas@gmail.com> writes:
>>
>>> James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
>>> text editors" <bug-gnu-emacs@gnu.org> 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.

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.

Thank you for persisting with this stuff!

Eric

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

[James Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eric Abrahamsen wrote:

> James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
>
>> Eric Abrahamsen wrote:
>>
>>> Stefan Kangas <stefankangas@gmail.com> writes:
>>>
>>>> James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
>>>> text editors" <bug-gnu-emacs@gnu.org> 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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Bugfix --]
[-- Type: text/x-patch, Size: 1424 bytes --]

From dfe4498e361262eb1eb25e288bc7228678ac1a43 Mon Sep 17 00:00:00 2001
From: James Thomas <jimjoe@gmx.net>
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


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


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

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

[Eli Zaretskii]

Ping!  Can we make some progress here?

> Cc: 70994@debbugs.gnu.org, Stefan Kangas <stefankangas@gmail.com>,
>  Daniel Semyonov <daniel@dsemy.com>
> Date: Thu, 08 Aug 2024 06:46:48 +0530
> From:  James Thomas via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Eric Abrahamsen wrote:
> 
> > James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
> > text editors" <bug-gnu-emacs@gnu.org> writes:
> >
> >> Eric Abrahamsen wrote:
> >>
> >>> Stefan Kangas <stefankangas@gmail.com> writes:
> >>>
> >>>> James Thomas via "Bug reports for GNU Emacs, the Swiss army knife of
> >>>> text editors" <bug-gnu-emacs@gnu.org> 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.
> 
> 
> 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

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

[James Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii wrote:

> Ping!  Can we make some progress here?

I'd like to confess again that this doesn't seem to be a big deal:
'gnus-cache-generate-nov-databases' and 'gnus-cache-generate-active'
should work regardless. It's only for someone who might accidentally do
this.

Regards,
James
>
>> Cc: 70994@debbugs.gnu.org, Stefan Kangas <stefankangas@gmail.com>,
>>  Daniel Semyonov <daniel@dsemy.com>
>> Date: Thu, 08 Aug 2024 06:46:48 +0530
>> From:  James Thomas via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Eric Abrahamsen wrote:
>> 
>> > James Thomas via "Bug reports for GNU Emacs, the Swiss army knife
>> > of
>> > text editors" <bug-gnu-emacs@gnu.org> writes:
>> >
>> >> Eric Abrahamsen wrote:
>> >>
>> >>> Stefan Kangas <stefankangas@gmail.com> writes:
>> >>>
>> >>>> James Thomas via "Bug reports for GNU Emacs, the Swiss army
>> >>>> knife of
>> >>>> text editors" <bug-gnu-emacs@gnu.org> 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.
>> 
>> 
>> 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

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

[Eli Zaretskii]

Ping! Eric, any further comments, or should we install the patch?

> From: James Thomas <jimjoe@gmx.net>
> Cc: eric@ericabrahamsen.net,  70994@debbugs.gnu.org,
>   stefankangas@gmail.com,  daniel@dsemy.com
> Date: Sat, 24 Aug 2024 14:54:28 +0530
> 
> Eli Zaretskii wrote:
> 
> > Ping!  Can we make some progress here?
> 
> I'd like to confess again that this doesn't seem to be a big deal:
> 'gnus-cache-generate-nov-databases' and 'gnus-cache-generate-active'
> should work regardless. It's only for someone who might accidentally do
> this.
> 
> Regards,
> James
> >
> >> Cc: 70994@debbugs.gnu.org, Stefan Kangas <stefankangas@gmail.com>,
> >>  Daniel Semyonov <daniel@dsemy.com>
> >> Date: Thu, 08 Aug 2024 06:46:48 +0530
> >> From:  James Thomas via "Bug reports for GNU Emacs,
> >>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >> 
> >> Eric Abrahamsen wrote:
> >> 
> >> > James Thomas via "Bug reports for GNU Emacs, the Swiss army knife
> >> > of
> >> > text editors" <bug-gnu-emacs@gnu.org> writes:
> >> >
> >> >> Eric Abrahamsen wrote:
> >> >>
> >> >>> Stefan Kangas <stefankangas@gmail.com> writes:
> >> >>>
> >> >>>> James Thomas via "Bug reports for GNU Emacs, the Swiss army
> >> >>>> knife of
> >> >>>> text editors" <bug-gnu-emacs@gnu.org> 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.
> >> 
> >> 
> >> 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
>