GNU bug report logs - #74905
[PATCH] Implement search for nnvirtual Gnus groups

Previous Next

To reply to this bug, email your comments to 74905 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Implement search for nnvirtual Gnus groups
Date: Mon, 16 Dec 2024 01:55:37 +0200

[Message part 1 (text/plain, inline)]

Tags: patch


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.

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.



In GNU Emacs 31.0.50 (build 1, x86_64-suse-linux-gnu, GTK+ Version
3.24.43, cairo version 1.18.2)
Repository revision: b25fba0ef8f38f95a9e65548f9ebb6c92739857e
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101014
System Description: openSUSE Tumbleweed

Configured using:
 'configure --disable-build-details --without-pop --with-mailutils
 --without-hesiod --with-gameuser=:games --with-kerberos
 --with-kerberos5 --with-file-notification=inotify --with-modules
 --enable-autodepend --enable-link-time-optimization --prefix=/usr
 --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share
 --localstatedir=/var --sharedstatedir=/var/lib
 --libexecdir=/usr/libexec --with-file-notification=yes
 --libdir=/usr/lib64 --with-native-compilation=aot
 --enable-locallisppath=/usr/share/emacs/31.0.50/site-lisp:/usr/share/emacs/site-lisp
 --with-x --with-xim --with-sound --with-xpm --with-jpeg --with-tiff
 --with-gif --with-png --with-rsvg --with-dbus --with-xft --without-gpm
 --with-tree-sitter --with-x-toolkit=gtk --without-pgtk
 --with-toolkit-scroll-bars --x-includes=/usr/include
 --x-libraries=/usr/lib64 --with-libotf --with-m17n-flt --with-cairo
 --build=x86_64-suse-linux --with-dumping=pdumper
 build_alias=x86_64-suse-linux 'CC=sccache cc' 'CFLAGS=-O2 -Wall
 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong
 -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection
 -Werror=return-type -flto=auto -march=znver3 -mmmx -mpopcnt -msse
 -msse2 -msse3 -mssse3 -msse4.1 -msse4.2 -mavx -mavx2 -msse4a -mno-fma4
 -mno-xop -mfma -mbmi -mbmi2 -maes -mpclmul -mno-gfni -mvpclmulqdq
 -mno-3dnow -madx -mabm -mno-cldemote -mclflushopt -mclwb -mclzero
 -mcx16 -mno-enqcmd -mf16c -mfsgsbase -mfxsr -mno-hle -msahf -mno-lwp
 -mlzcnt -mmovbe -mno-movdir64b -mno-movdiri -mmwaitx -mno-pconfig -mpku
 -mprfchw -mno-ptwrite -mrdpid -mrdrnd -mrdseed -mno-rtm -mno-serialize
 -mno-sgx -msha -mshstk -mno-tbm -mno-tsxldtrk -mvaes -mno-waitpkg
 -mwbnoinvd -mxsave -mxsavec -mxsaveopt -mxsaves -mno-amx-tile
 -mno-amx-int8 -mno-amx-bf16 -mno-uintr -mno-hreset -mno-kl -mno-widekl
 -mno-avxvnni -mno-avxifma -mno-avxvnniint8 -mno-avxneconvert
 -mno-cmpccxadd -mno-amx-fp16 -mno-prefetchi -mno-raoint
 -mno-amx-complex --param l1-cache-size=32 --param l1-cache-line-size=64
 --param l2-cache-size=512 -mtune=znver3 -fno-optimize-sibling-calls -O2
 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong
 -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection
 -Werror=return-type -flto=auto -g -D_GNU_SOURCE
 -DGDK_DISABLE_DEPRECATION_WARNINGS -DGLIB_DISABLE_DEPRECATION_WARNINGS
 -pipe -Wno-pointer-sign -Wno-unused-variable -Wno-unused-label
 -DPDMP_BASE='\''"emacs-gtk"'\''' LDFLAGS=-Wl,-O2 'CXX=sccache c++'
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

[0001-Implement-search-for-nnvirtual-Gnus-groups.patch (text/patch, attachment)]

Message #8 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: James Thomas <jimjoe <at> gmx.net>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Wed, 18 Dec 2024 05:58:00 +0530

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.

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

--

Message #11 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: James Thomas <jimjoe <at> gmx.net>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Wed, 18 Dec 2024 17:44:46 +0200

James Thomas <jimjoe <at> gmx.net> 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.
Should verify that nnvirtual has arguments? So far it is possible
to add nnvirtual to select methods with "" but this is invalid.

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

Message #14 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: James Thomas <jimjoe <at> gmx.net>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Thu, 19 Dec 2024 04:01:47 +0530

Björn Bidar wrote:

> James Thomas <jimjoe <at> gmx.net> 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)

--

Message #17 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: James Thomas <jimjoe <at> gmx.net>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Thu, 19 Dec 2024 04:51:24 +0530

James Thomas wrote:

> Björn Bidar wrote:
>
>> James Thomas <jimjoe <at> gmx.net> 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.

No, sorry, I was a little confused about this. But it seems to me that
-request-group removes each group from the component list (unlike your
-request-list). Is that relevant? IDK. The latter seems to do info
updation as well, based on -always-rescan.

--

Message #20 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: James Thomas <jimjoe <at> gmx.net>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Thu, 19 Dec 2024 02:42:14 +0200

James Thomas <jimjoe <at> gmx.net> writes:

> James Thomas wrote:
>
>> Björn Bidar wrote:
>>
>>> James Thomas <jimjoe <at> gmx.net> 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.
>
> No, sorry, I was a little confused about this. But it seems to me that
> -request-group removes each group from the component list (unlike your
> -request-list). Is that relevant? IDK. The latter seems to do info
> updation as well, based on -always-rescan.
>

Request list supposed to list the available groups on the server it is
also for example used in gnus-search-imap class gnus-search-run-search
function. Since for the case of nnvirtual all groups on the server
are all groups contained in the group it is the right function.

I took all the explenation for each functions purpose from the manual
section linked below:
(info "(gnus) Required Back End Functions")

To me the code just reads like that that for nnvirtual
the groups are activated this way only because
it doesn't have request list.

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

It does have arguments exactly but you can add nnvirtual without
arguments or technically an empty argument to select methods.

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

I guess so but that isn't the intended way of using nnvirtual,
also it wouldn't take care of the request of the setup such
as creating the associated group.
Regular activation meant in this context that the request-list function
is called but without a argument the nnvirtual-server doesn't contain
any groups.

Message #23 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: James Thomas <jimjoe <at> gmx.net>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Fri, 20 Dec 2024 12:45:47 +0530

Björn Bidar wrote:

> James Thomas <jimjoe <at> gmx.net> writes:
>
>> James Thomas wrote:
>>
>>> Björn Bidar wrote:
>>>
>>>> James Thomas <jimjoe <at> gmx.net> 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.
>>
>> No, sorry, I was a little confused about this. But it seems to me that
>> -request-group removes each group from the component list (unlike your
>> -request-list). Is that relevant? IDK. The latter seems to do info
>> updation as well, based on -always-rescan.
>>
>
> Request list supposed to list the available groups on the server it is
> also for example used in gnus-search-imap class gnus-search-run-search
> function. Since for the case of nnvirtual all groups on the server
> are all groups contained in the group it is the right function.

True, but since it returns the group data like -request-group and often
acts as a _substitute_, shouldn't it mimic its operation? If not, is
there a good reason?

> I took all the explenation for each functions purpose from the manual
> section linked below:
> (info "(gnus) Required Back End Functions")

Please do look at how they're called, as well; for a fuller picture.

> To me the code just reads like that that for nnvirtual
> the groups are activated this way only because
> it doesn't have request list.
>
>>
>> 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.
>>
>
> It does have arguments exactly but you can add nnvirtual without
> arguments or technically an empty argument to select methods.
>
>>> 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?
>
> I guess so but that isn't the intended way of using nnvirtual,
> also it wouldn't take care of the request of the setup such
> as creating the associated group.

Well now you've made me look at it... :-)

Try this:

- In *Groups*: j nnvirtual:Test RET
- S l 3 RET
- e "nnvirtual:Test" C-c C-c
- q
- Add to -secondary-select-methods:

  (nnvirtual "Test"
		   (nnvirtual-component-regexp
                   ;; Put in some group you have.
		   "^$\\|\\(^nnml\\+archive:sent$\\)"))
                   
M-x gnus

> Regular activation meant in this context that the request-list function
> is called but without a argument the nnvirtual-server doesn't contain
> any groups.

OK. But I don't think I've come across that term.

Also, your -request-list makes the component groups subscribable from
the Server buffer. That seems wrong too: the backend ought to have only
one (umbrella) group, no?

--

Message #26 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: James Thomas <jimjoe <at> gmx.net>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Fri, 20 Dec 2024 10:17:38 +0200

James Thomas <jimjoe <at> gmx.net> writes:

> Björn Bidar wrote:
>
>> James Thomas <jimjoe <at> gmx.net> writes:
>>
>>> James Thomas wrote:
>>>
>>>> Björn Bidar wrote:
>>>>
>>>>> James Thomas <jimjoe <at> gmx.net> 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.
>>>
>>> No, sorry, I was a little confused about this. But it seems to me that
>>> -request-group removes each group from the component list (unlike your
>>> -request-list). Is that relevant? IDK. The latter seems to do info
>>> updation as well, based on -always-rescan.
>>>
>>
>> Request list supposed to list the available groups on the server it is
>> also for example used in gnus-search-imap class gnus-search-run-search
>> function. Since for the case of nnvirtual all groups on the server
>> are all groups contained in the group it is the right function.
>
> True, but since it returns the group data like -request-group and often
> acts as a _substitute_, shouldn't it mimic its operation? If not, is
> there a good reason?

It does mimic it's operation or doesn't it?

>> I took all the explenation for each functions purpose from the manual
>> section linked below:
>> (info "(gnus) Required Back End Functions")
>
> Please do look at how they're called, as well; for a fuller picture.
>
>> To me the code just reads like that that for nnvirtual
>> the groups are activated this way only because
>> it doesn't have request list.
>>
>>>
>>> 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.
>>>
>>
>> It does have arguments exactly but you can add nnvirtual without
>> arguments or technically an empty argument to select methods.
>>
>>>> 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?
>>
>> I guess so but that isn't the intended way of using nnvirtual,
>> also it wouldn't take care of the request of the setup such
>> as creating the associated group.
>
> Well now you've made me look at it... :-)
>
> Try this:
>
> - In *Groups*: j nnvirtual:Test RET
> - S l 3 RET
> - e "nnvirtual:Test" C-c C-c
> - q
> - Add to -secondary-select-methods:
>
>   (nnvirtual "Test"
> 		   (nnvirtual-component-regexp
>                    ;; Put in some group you have.
> 		   "^$\\|\\(^nnml\\+archive:sent$\\)"))
>
> M-x gnus
>
>> Regular activation meant in this context that the request-list function
>> is called but without a argument the nnvirtual-server doesn't contain
>> any groups.
>
> OK. But I don't think I've come across that term.
>
> Also, your -request-list makes the component groups subscribable from
> the Server buffer. That seems wrong too: the backend ought to have only
> one (umbrella) group, no?

It only has one group containing other groups yes. The backend is not
supposed to end up in the Server but is only there if it falsely was
added to the select-methods.

Anyway I need a backtrace from the case where the search did not work
with nnml groups.

Message #29 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: James Thomas <jimjoe <at> gmx.net>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Sun, 22 Dec 2024 17:07:39 +0530

Björn Bidar wrote:

> James Thomas <jimjoe <at> gmx.net> writes:
>
>> Björn Bidar wrote:
>>
>>> James Thomas <jimjoe <at> gmx.net> writes:
>>>
>>>> James Thomas wrote:
>>>>
>>>>> Björn Bidar wrote:
>>>>>
>>>>>> James Thomas <jimjoe <at> gmx.net> 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.
>>>>
>>>> No, sorry, I was a little confused about this. But it seems to me that
>>>> -request-group removes each group from the component list (unlike your
>>>> -request-list). Is that relevant? IDK. The latter seems to do info
>>>> updation as well, based on -always-rescan.
>>>>
>>>
>>> Request list supposed to list the available groups on the server it is
>>> also for example used in gnus-search-imap class gnus-search-run-search
>>> function. Since for the case of nnvirtual all groups on the server
>>> are all groups contained in the group it is the right function.
>>
>> True, but since it returns the group data like -request-group and often
>> acts as a _substitute_, shouldn't it mimic its operation? If not, is
>> there a good reason?
>
> It does mimic it's operation or doesn't it?

I mean, I was asking if it _should_ or not. I think it would be a good
idea to get some 'grep'ed evidence of code coverage that -request-list
would not supplant the intentions of -request-group.

>>> I took all the explenation for each functions purpose from the manual
>>> section linked below:
>>> (info "(gnus) Required Back End Functions")
>>
>> Please do look at how they're called, as well; for a fuller picture.
>>
>>> To me the code just reads like that that for nnvirtual
>>> the groups are activated this way only because
>>> it doesn't have request list.
>>>
>>>>
>>>> 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.
>>>>
>>>
>>> It does have arguments exactly but you can add nnvirtual without
>>> arguments or technically an empty argument to select methods.
>>>
>>>>> 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?
>>>
>>> I guess so but that isn't the intended way of using nnvirtual,
>>> also it wouldn't take care of the request of the setup such
>>> as creating the associated group.
>>
>> Well now you've made me look at it... :-)
>>
>> Try this:
>>
>> - In *Groups*: j nnvirtual:Test RET
>> - S l 3 RET
>> - e "nnvirtual:Test" C-c C-c
>> - q
>> - Add to -secondary-select-methods:
>>
>>   (nnvirtual "Test"
>> 		   (nnvirtual-component-regexp
>>                    ;; Put in some group you have.
>> 		   "^$\\|\\(^nnml\\+archive:sent$\\)"))
>>
>> M-x gnus
>>
>>> Regular activation meant in this context that the request-list function
>>> is called but without a argument the nnvirtual-server doesn't contain
>>> any groups.
>>
>> OK. But I don't think I've come across that term.
>>
>> Also, your -request-list makes the component groups subscribable from
>> the Server buffer. That seems wrong too: the backend ought to have only
>> one (umbrella) group, no?

> It only has one group containing other groups yes. The backend is not
> supposed to end up in the Server but is only there if it falsely was
> added to the select-methods.
>
> Anyway I need a backtrace from the case where the search did not work
> with nnml groups.

No no, you misunderstand me: It was just to demonstrate that:

> that isn't the intended way of using nnvirtual

...can't be said. So please re-verify anything (there might've been)
about the patch predicated on that. I did not encounter errors.

I know you're exploiting the fact that the server and group are one, but
perhaps it's being too clever by half. Any such unintended effects of
that easy road ought to be acceptable to other users.

I don't have any other misgivings, personally (on the face of it, i.e.:
I haven't tested it well).

--

Message #32 received at 74905 <at> debbugs.gnu.org (full text, mbox):

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: James Thomas <jimjoe <at> gmx.net>
Cc: 74905 <at> debbugs.gnu.org
Subject: Re: bug#74905: [PATCH] Implement search for nnvirtual Gnus groups
Date: Sun, 22 Dec 2024 22:48:05 +0200

James Thomas <jimjoe <at> gmx.net> writes:

> Björn Bidar wrote:
>
>> James Thomas <jimjoe <at> gmx.net> writes:
>>
>>> Björn Bidar wrote:
>>>
>>>> James Thomas <jimjoe <at> gmx.net> writes:
>>>>
>>>>> James Thomas wrote:
>>>>>
>>>>>> Björn Bidar wrote:
>>>>>>
>>>>>>> James Thomas <jimjoe <at> gmx.net> 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.
>>>>>
>>>>> No, sorry, I was a little confused about this. But it seems to me that
>>>>> -request-group removes each group from the component list (unlike your
>>>>> -request-list). Is that relevant? IDK. The latter seems to do info
>>>>> updation as well, based on -always-rescan.
>>>>>
>>>>
>>>> Request list supposed to list the available groups on the server it is
>>>> also for example used in gnus-search-imap class gnus-search-run-search
>>>> function. Since for the case of nnvirtual all groups on the server
>>>> are all groups contained in the group it is the right function.
>>>
>>> True, but since it returns the group data like -request-group and often
>>> acts as a _substitute_, shouldn't it mimic its operation? If not, is
>>> there a good reason?
>>
>> It does mimic it's operation or doesn't it?
>
> I mean, I was asking if it _should_ or not. I think it would be a good
> idea to get some 'grep'ed evidence of code coverage that -request-list
> would not supplant the intentions of -request-group.

To verify my earlier thoughts and have an example I tested this.

-request-group functions are for retrieving the metedata of the group
and possibly update the existing metata against the new one.

-request-list function list all groups on the server with possibly
the status of group e.g. killed, zombie or none if subscribed, the
highest amount of articles and the group name.

The latter is the one we need.

>>>> I took all the explenation for each functions purpose from the manual
>>>> section linked below:
>>>> (info "(gnus) Required Back End Functions")
>>>
>>> Please do look at how they're called, as well; for a fuller picture.
>>>
>>>> To me the code just reads like that that for nnvirtual
>>>> the groups are activated this way only because
>>>> it doesn't have request list.
>>>>
>>>>>
>>>>> 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.
>>>>>
>>>>
>>>> It does have arguments exactly but you can add nnvirtual without
>>>> arguments or technically an empty argument to select methods.
>>>>
>>>>>> 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?
>>>>
>>>> I guess so but that isn't the intended way of using nnvirtual,
>>>> also it wouldn't take care of the request of the setup such
>>>> as creating the associated group.
>>>
>>> Well now you've made me look at it... :-)
>>>
>>> Try this:
>>>
>>> - In *Groups*: j nnvirtual:Test RET
>>> - S l 3 RET
>>> - e "nnvirtual:Test" C-c C-c
>>> - q
>>> - Add to -secondary-select-methods:
>>>
>>>   (nnvirtual "Test"
>>> 		   (nnvirtual-component-regexp
>>>                    ;; Put in some group you have.
>>> 		   "^$\\|\\(^nnml\\+archive:sent$\\)"))
>>>
>>> M-x gnus
>>>
>>>> Regular activation meant in this context that the request-list function
>>>> is called but without a argument the nnvirtual-server doesn't contain
>>>> any groups.
>>>
>>> OK. But I don't think I've come across that term.
>>>
>>> Also, your -request-list makes the component groups subscribable from
>>> the Server buffer. That seems wrong too: the backend ought to have only
>>> one (umbrella) group, no?
>
>> It only has one group containing other groups yes. The backend is not
>> supposed to end up in the Server but is only there if it falsely was
>> added to the select-methods.
>>
>> Anyway I need a backtrace from the case where the search did not work
>> with nnml groups.
>
> No no, you misunderstand me: It was just to demonstrate that:
>
>> that isn't the intended way of using nnvirtual
>
> ...can't be said. So please re-verify anything (there might've been)
> about the patch predicated on that. I did not encounter errors.
>
> I know you're exploiting the fact that the server and group are one, but
> perhaps it's being too clever by half. Any such unintended effects of
> that easy road ought to be acceptable to other users.

Yeah that's why I posted the patch here so more can try it with
different setups.

A review from a Gnus maintainer or contributor would also be awesome.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.