Subject: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups

From: James Thomas <[email-masked]>
Date: Thu, 19 Dec 2024 04:01:47 +0530

Björn Bidar wrote:

> James Thomas <[email-masked]> writes:
>
>> Björn Bidar wrote:
>>
>>> This patch implements search for nnvirtual. I'm using publi-inbox's
>>> with nnvirtual to group each group into one.
>>> However searching wasn't possible in these nnvirtual groups.
>>> I implemented gnus-search-run-search based on the existing
>>> nnselect gnus-search-run-search function.
>>
>> Thanks! I haven't looked into it, but here are some quick comments:
>>
>>> I'm looking for feedback on the patch. I don't exactly know how
>>> the search function is called when multiple groups of the same type
>>> are
>>> involved. For nnvirtual each group is its on server, does that mean
>>> the
>>> function will be always called only for each group? In that case
>>> everything should be good.
>>
>> That seems to be the case: see
>> gnus-group-read-ephemeral-search-group
>> and gnus-group-make-search-group.
>
> OK good than my understanding from my tests matched with the rest of
> the
> code.
> Thanks for these examples I haven't looked at the create group
> functions
> as the searched methods don't have to create groups even when they
> start
> a new search by another backend just like e.g. if the user would call
> a
> search on another imap group.
>
>>> +(deffoo nnvirtual-request-list (&optional server)
>>> +  (when (nnvirtual-possibly-change-server server)
>>> +    (with-current-buffer nntp-server-buffer
>>> +      (erase-buffer)
>>> +      (dolist (group nnvirtual-component-groups)
>>> +        (insert (format "%S 0  1 y\n" group))))
>>> +    t))
>>
>> Did you check if gnus-start.el#L1801 withstands this? It seems to me
>> to
>> assume that nnvirtual doesn't have -request-list.
>
> It does. If the user has falsely add nnvirtual to one of the select
> methods than it will call it try to call the function which doesn't
> fail
> or do anything. The only thing that happens from that is it will show
> the false results as groups contained in the nnvirtual method without
> a
> parameter.

No, I mean, the point of that code seems to be that nnvirtual is
activated _last_, i.e. after any component groups of other backends. And
it's not just select methods: even Foreign Groups are included, no?

> Should verify that nnvirtual has arguments? So far it is possible
> to add nnvirtual to select methods with "" but this is invalid.

I'm not sure what you mean, but it does have arguments: the components
regexp, for one.

> If nnvirtual isn't added to select methods nothing happens besides
> the regular activation.

Couldn't someone have added one? Say, with the above argument?

(Btw I'm only helping out, y'know. Getting this in is up to others with
commit rights)

--