[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#69517: [PATCH] Make gnus cache work with group names having '/'
From: |
Eric Abrahamsen |
Subject: |
bug#69517: [PATCH] Make gnus cache work with group names having '/' |
Date: |
Sat, 30 Mar 2024 15:21:01 -0700 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
On 03/29/24 05:39 AM, James Thomas wrote:
> Eli Zaretskii wrote:
>
>> Ping! Is there anything else left to be done here, or should we close
>> this?
>>
>>> From: Daniel Semyonov <daniel@dsemy.com>
>>> Cc: 69517@debbugs.gnu.org, Eric Abrahamsen <eric@ericabrahamsen.net>, Eli
>>> Zaretskii <eliz@gnu.org>
>>> Date: Fri, 15 Mar 2024 19:33:42 +0200
>>>
>>> >>>>> James Thomas writes:
>>>
>>> > 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: [patch]
>>>
>>> > 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?
>>>
>>> I tested it and it seems to work, but I'm pretty sure it will also break
>>> existing groups with % or / in their names in several backends.
>>> For example, the `nnmh' and `nndiary' backends use this function to
>>> locate groups on disk, which will fail for those groups (unless users
>>> rename the files manually).
>>>
>>> FWIW I think this approach is good, but since Gnus doesn't even emit a
>>> warning currently when creating a group with % in its name, I don't
>>> think breaking these groups is a good idea.
>>>
>>> Daniel
>>>
>
> I've been using this since then, on master, with no problems. Let me add
> to my earlier reasons for being confident about this not breaking
> existing setups:
>
> James Thomas wrote:
>
>> I think only one of these combinations is likely to be a problem in
>> practice: groups such as [Gmail]/Drafts. I've never seen a % in a
>> group name. The cache wouldn't work for one with / anyway (this bug)
>> and as far as the agent is concerned, none of the Gmail groups with
>> spaces in them work anyway (bug#65467: note that the patch on this is
>> also needed for the agent to work on these groups with /). I haven't
>> used nnmh or nndiary but I don't think they normally use group names
>> with '/'.
>
> The only ones with '/' (and no spaces) I know of are the Gmail groups,
> namely:
>
> [Gmail]/Bin
> [Gmail]/Drafts
> [Gmail]/Important
> [Gmail]/Spam
> [Gmail]/Starred
>
> ...which are unlikely to be agentized because of their nature (though
> ideally the patch at bug#65467 ought to be applied as well, after this).
>
> Moreover, getting Gmail to work with Gnus requires a paid account and a
> complicated setup with OAuth2 (AFAIK, because I use one) which only few
> people are likely to have managed.
>
> Lastly, the fix for such an unlikely existing setup is a simple rename
> of the directory: say, from [Gmail]_Drafts to [Gmail]%2FDrafts.
Thanks, I've now applied this, and am closing the bug. Sorry this took
so long -- I kept thinking I would dive a little deeper into the nnmail
file/directory shenanigans and get a better grasp on the internals, but
obviously was not finding the time. In it goes!
Eric
- bug#69517: [PATCH] Make gnus cache work with group names having '/', James Thomas, 2024/03/02
- bug#69517: [PATCH] Make gnus cache work with group names having '/', Eli Zaretskii, 2024/03/09
- bug#69517: [PATCH] Make gnus cache work with group names having '/', James Thomas, 2024/03/09
- bug#69517: [PATCH] Make gnus cache work with group names having '/', Eric Abrahamsen, 2024/03/10
- bug#69517: [PATCH] Make gnus cache work with group names having '/', Eric Abrahamsen, 2024/03/10
- bug#69517: [PATCH] Make gnus cache work with group names having '/', James Thomas, 2024/03/13
- bug#69517: [PATCH] Make gnus cache work with group names having '/', Daniel Semyonov, 2024/03/15
- bug#69517: [PATCH] Make gnus cache work with group names having '/', James Thomas, 2024/03/15
- bug#69517: [PATCH] Make gnus cache work with group names having '/', Eli Zaretskii, 2024/03/28
- bug#69517: [PATCH] Make gnus cache work with group names having '/', James Thomas, 2024/03/28
- bug#69517: [PATCH] Make gnus cache work with group names having '/',
Eric Abrahamsen <=
- bug#69517: [PATCH] Make gnus cache work with group names having '/', James Thomas, 2024/03/10