Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Michael Albinus]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

Hi,

> branch: master
> commit 1601c5a518dfa208af4827c56cf9570f3b90e15d
> Author: Daniel Semyonov <daniel@dsemy.com>
> Commit: Eric Abrahamsen <eric@ericabrahamsen.net>
>
>     Gnus: Add back end for Atom feeds (nnatom)
>
>     * lisp/gnus/gnus.el (gnus-valid-select-methods): Add entry for nnatom.
>     * lisp/gnus/nnfeed.el: New file implementing an abstract web feed back
>     end.
>     * lisp/gnus/nnatom.el: New file implementing a back end for Atom feeds.
>     * doc/misc/gnus.texi (Browsing the Web, Back End Interface):
>     * etc/NEWS (Gnus): Document nnatom and nnfeed.
> ---
>  doc/misc/gnus.texi  |  77 ++++++
>  etc/NEWS            |  10 +
>  lisp/gnus/gnus.el   |   1 +
>  lisp/gnus/nnatom.el | 276 +++++++++++++++++++++
>  lisp/gnus/nnfeed.el | 683 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 1047 insertions(+)

This fails the tests:

--8<---------------cut here---------------start------------->8---
# make -C test test-custom-opts

...

The following options might have problems:
variable: gnus-valid-select-methods
   value: (("nntp" post address prompt-address physical-address cloud) ("nnspool" post address) ("nnvirtual" post-mail virtual prompt-address) ("nnmbox" mail respool address) ("nnml" post-mail respool address) ("nnmh" mail respool address) ("nndir" post-mail prompt-address physical-address) ("nneething" none address prompt-address physical-address) ("nndoc" none address prompt-address) ("nnbabyl" mail address respool) ("nndraft" post-mail) ("nnfolder" mail respool address) ("nngateway" post-mail address prompt-address physical-address) ("nnweb" none) ("nnrss" none global) ("nnagent" post-mail) ("nnimap" post-mail address prompt-address physical-address respool server-marks cloud) ("nnmaildir" mail respool address server-marks) ("nnatom" address) ("nnnil" none))
    type: (repeat (group (string :tag "Name") (radio-button-choice (const :format "%v " post) (const :format "%v " mail) (const :format "%v " none) (const post-mail)) (checklist :inline t :greedy t (const :format "%v " address) (const cloud) (const global) (const :format "%v " prompt-address) (const :format "%v " physical-address) (const virtual) (const :format "%v " respool) (const server-marks))))
variable: gnus-valid-select-methods
   value: (("nntp" post address prompt-address physical-address cloud) ("nnspool" post address) ("nnvirtual" post-mail virtual prompt-address) ("nnmbox" mail respool address) ("nnml" post-mail respool address) ("nnmh" mail respool address) ("nndir" post-mail prompt-address physical-address) ("nneething" none address prompt-address physical-address) ("nndoc" none address prompt-address) ("nnbabyl" mail address respool) ("nndraft" post-mail) ("nnfolder" mail respool address) ("nngateway" post-mail address prompt-address physical-address) ("nnweb" none) ("nnrss" none global) ("nnagent" post-mail) ("nnimap" post-mail address prompt-address physical-address respool server-marks cloud) ("nnmaildir" mail respool address server-marks) ("nnatom" address) ("nnnil" none) ("nndiary" post-mail respool address) ("nnmairix" mail address virtual) ("nnselect" post-mail virtual) ("nnatom" address))
    type: (repeat (group (string :tag "Name") (radio-button-choice (const :format "%v " post) (const :format "%v " mail) (const :format "%v " none) (const post-mail)) (checklist :inline t :greedy t (const :format "%v " address) (const cloud) (const global) (const :format "%v " prompt-address) (const :format "%v " physical-address) (const virtual) (const :format "%v " respool) (const server-marks))))
Test check-for-wrong-custom-opts backtrace:
  signal(ert-test-failed (((should (null (cus-test-opts t))) :form (nu
  ert-fail(((should (null (cus-test-opts t))) :form (null ((gnus-valid
  #[0 "\301C\3021\f\0\10C0\202\27\0\1\303\240\210\211@\1AD\262\1\304C\
  #[0 "\303\300!\11>\204\17\0\304\305\306\300D\"\210\300\307H\303\1!\n
  handler-bind-1(#[0 "\303\300!\11>\204\17\0\304\305\306\300D\"\210\30
  ert--run-test-internal(#s(ert--test-execution-info :test ... :result
  ert-run-test(#s(ert-test :name check-for-wrong-custom-opts :document
  ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
  ert-run-tests((not (tag :unstable)) #[385 "\1\306\267\202X\4\11?\205
  ert-run-tests-batch((not (tag :unstable)))
  ert-run-tests-batch-and-exit((not (tag :unstable)))
  eval((ert-run-tests-batch-and-exit '(not (tag :unstable))) t)
  command-line-1(("-L" ":." "-l" "ert" "--eval" "(setq treesit-extra-l
  command-line()
  normal-top-level()
Test check-for-wrong-custom-opts condition:
    (ert-test-failed
     ((should (null (cus-test-opts t))) :form
      (null
       ((gnus-valid-select-methods :type-error ... ...)
	(gnus-valid-select-methods :type-error ... ...)))
      :value nil))
   FAILED  1/1  check-for-wrong-custom-opts (19.065213 sec) at misc/test-custom-opts.el:33
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Eric Abrahamsen]

On 04/28/24 12:53 PM, Michael Albinus wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
> Hi,
>
>> branch: master
>> commit 1601c5a518dfa208af4827c56cf9570f3b90e15d
>> Author: Daniel Semyonov <daniel@dsemy.com>
>> Commit: Eric Abrahamsen <eric@ericabrahamsen.net>
>>
>>     Gnus: Add back end for Atom feeds (nnatom)
>>
>>     * lisp/gnus/gnus.el (gnus-valid-select-methods): Add entry for nnatom.
>>     * lisp/gnus/nnfeed.el: New file implementing an abstract web feed back
>>     end.
>>     * lisp/gnus/nnatom.el: New file implementing a back end for Atom feeds.
>>     * doc/misc/gnus.texi (Browsing the Web, Back End Interface):
>>     * etc/NEWS (Gnus): Document nnatom and nnfeed.
>> ---
>>  doc/misc/gnus.texi  |  77 ++++++
>>  etc/NEWS            |  10 +
>>  lisp/gnus/gnus.el   |   1 +
>>  lisp/gnus/nnatom.el | 276 +++++++++++++++++++++
>>  lisp/gnus/nnfeed.el | 683 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 1047 insertions(+)
>
> This fails the tests:
>
> # make -C test test-custom-opts

Thanks, we were missing a "category" definition in the new option
default, fixing now.

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Eric Abrahamsen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 04/28/24 12:53 PM, Michael Albinus wrote:
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>> Hi,
>>
>>> branch: master
>>> commit 1601c5a518dfa208af4827c56cf9570f3b90e15d
>>> Author: Daniel Semyonov <daniel@dsemy.com>
>>> Commit: Eric Abrahamsen <eric@ericabrahamsen.net>
>>>
>>>     Gnus: Add back end for Atom feeds (nnatom)
>>>
>>>     * lisp/gnus/gnus.el (gnus-valid-select-methods): Add entry for nnatom.
>>>     * lisp/gnus/nnfeed.el: New file implementing an abstract web feed back
>>>     end.
>>>     * lisp/gnus/nnatom.el: New file implementing a back end for Atom feeds.
>>>     * doc/misc/gnus.texi (Browsing the Web, Back End Interface):
>>>     * etc/NEWS (Gnus): Document nnatom and nnfeed.
>>> ---
>>>  doc/misc/gnus.texi  |  77 ++++++
>>>  etc/NEWS            |  10 +
>>>  lisp/gnus/gnus.el   |   1 +
>>>  lisp/gnus/nnatom.el | 276 +++++++++++++++++++++
>>>  lisp/gnus/nnfeed.el | 683 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  5 files changed, 1047 insertions(+)
>>
>> This fails the tests:
>>
>> # make -C test test-custom-opts
>
> Thanks, we were missing a "category" definition in the new option
> default, fixing now.

This is still failing the tests, and I can't see why, I suspect the
reason is in the "... ..." below:

Test check-for-wrong-custom-opts condition:
    (ert-test-failed
     ((should (null (cus-test-opts t))) :form
      (null ((gnus-valid-select-methods :type-error ... ...))) :value nil))
   FAILED  1/1  check-for-wrong-custom-opts (13.744560 sec) at misc/test-custom-opts.el:33

I've edited test-custom-opts.el to let-bind
`ert-batch-print-level/length' to nil around the (cus-test-opts t) call,
but that doesn't change anything -- can anyone tell me how to expand the
output there?

Thanks,
Eric

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Michael Albinus]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

Hi Eric,

> This is still failing the tests, and I can't see why, I suspect the
> reason is in the "... ..." below:
>
> Test check-for-wrong-custom-opts condition:
>     (ert-test-failed
>      ((should (null (cus-test-opts t))) :form
>       (null ((gnus-valid-select-methods :type-error ... ...))) :value nil))
>    FAILED  1/1  check-for-wrong-custom-opts (13.744560 sec) at misc/test-custom-opts.el:33
>
> I've edited test-custom-opts.el to let-bind
> `ert-batch-print-level/length' to nil around the (cus-test-opts t) call,
> but that doesn't change anything -- can anyone tell me how to expand the
> output there?

--8<---------------cut here---------------start------------->8---
# make -C test test-custom-opts EMACS_EXTRAOPT="--eval '(setopt ert-batch-print-length nil ert-batch-print-level nil)'"
--8<---------------cut here---------------end--------------->8---

> Thanks,
> Eric

Best regards, Michael.

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Mattias Engdegård]

29 apr. 2024 kl. 18.42 skrev Michael Albinus <michael.albinus@gmx.de>:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> 
>> This is still failing the tests, and I can't see why, I suspect the
>> reason is in the "... ..." below:

No, you should ignore that and look at the text

> The following options might have problems:
> variable: gnus-valid-select-methods
>    value: (...)
>     type: (...)


that precedes the ERT error. In this case, it appears that the top-level list includes an element

  ("nnatom" address)

which doesn't match the declared type which requires that the string be followed by one of {post, mail, none, post-mail}.

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Daniel Semyonov]

>>>>> Mattias Engdegård writes:

    > 29 apr. 2024 kl. 18.42 skrev Michael Albinus <michael.albinus@gmx.de>:
    >> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
    >> 
    >>> This is still failing the tests, and I can't see why, I suspect the
    >>> reason is in the "... ..." below:

    > No, you should ignore that and look at the text

    >> The following options might have problems:
    >> variable: gnus-valid-select-methods
    >> value: (...)
    >> type: (...)


    > that precedes the ERT error. In this case, it appears that the top-level list includes an element

    >   ("nnatom" address)

    > which doesn't match the declared type which requires that the string be followed by one of {post, mail, none, post-mail}.


This should be changed to `("nnatom" none address)', and the docstring
should probably be changed to clarify this too.

Honestly though, I don't understand why this is a user option in the
first place, it only seems useful if you're implementing a new backend
(in which case, you're probably using `gnus-declare-backend', which
modifies `gnus-valid-select-methods' without checking the value anyway).

Daniel

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Eric Abrahamsen]

On 04/29/24 21:39 PM, Daniel Semyonov wrote:
>>>>>> Mattias Engdegård writes:
>
>     > 29 apr. 2024 kl. 18.42 skrev Michael Albinus <michael.albinus@gmx.de>:
>     >> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>     >> 
>     >>> This is still failing the tests, and I can't see why, I suspect the
>     >>> reason is in the "... ..." below:
>
>     > No, you should ignore that and look at the text
>
>     >> The following options might have problems:
>     >> variable: gnus-valid-select-methods
>     >> value: (...)
>     >> type: (...)
>
>
>     > that precedes the ERT error. In this case, it appears that the
>     > top-level list includes an element
>
>     >   ("nnatom" address)
>
>     > which doesn't match the declared type which requires that the
>     > string be followed by one of {post, mail, none, post-mail}.
>
>
> This should be changed to `("nnatom" none address)', and the docstring
> should probably be changed to clarify this too.

That's what I've changed it to, and it was still failing, that's why I
was trying to dig more information out of the failure.

HOWEVER. Moments ago I thought to look at nnatom.el itself, and this bit
at the bottom:

(gnus-declare-backend (symbol-name nnatom-backend) 'address)

Also ends up setting gnus-valid-select-methods, and needs the equivalent
change:

(gnus-declare-backend (symbol-name nnatom-backend) 'none 'address)

> Honestly though, I don't understand why this is a user option in the
> first place, it only seems useful if you're implementing a new backend
> (in which case, you're probably using `gnus-declare-backend', which
> modifies `gnus-valid-select-methods' without checking the value anyway).

And here you mention that exact fact :)

I agree it doesn't make much sense, it doesn't do anything for the user.
I wonder if `gnus-declare-backend' was originally just meant to be used
by out-of-tree backend libraries.

Anyway, it's redundant, but no great harm done, and the tests pass.

Eric

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Daniel Semyonov]

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

>>>>> Eric Abrahamsen writes:

    > On 04/29/24 21:39 PM, Daniel Semyonov wrote:
    >>>>>>> Mattias Engdegård writes:
    >> 
    >> > 29 apr. 2024 kl. 18.42 skrev Michael Albinus <michael.albinus@gmx.de>:
    >> >> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
    >> >> 
    >> >>> This is still failing the tests, and I can't see why, I suspect the
    >> >>> reason is in the "... ..." below:
    >> 
    >> > No, you should ignore that and look at the text
    >> 
    >> >> The following options might have problems:
    >> >> variable: gnus-valid-select-methods
    >> >> value: (...)
    >> >> type: (...)
    >> 
    >> 
    >> > that precedes the ERT error. In this case, it appears that the
    >> > top-level list includes an element
    >> 
    >> >   ("nnatom" address)
    >> 
    >> > which doesn't match the declared type which requires that the
    >> > string be followed by one of {post, mail, none, post-mail}.
    >> 
    >> 
    >> This should be changed to `("nnatom" none address)', and the docstring
    >> should probably be changed to clarify this too.

    > That's what I've changed it to, and it was still failing, that's why I
    > was trying to dig more information out of the failure.

    > HOWEVER. Moments ago I thought to look at nnatom.el itself, and this bit
    > at the bottom:

    > (gnus-declare-backend (symbol-name nnatom-backend) 'address)

    > Also ends up setting gnus-valid-select-methods, and needs the equivalent
    > change:

    > (gnus-declare-backend (symbol-name nnatom-backend) 'none 'address)

    >> Honestly though, I don't understand why this is a user option in the
    >> first place, it only seems useful if you're implementing a new backend
    >> (in which case, you're probably using `gnus-declare-backend', which
    >> modifies `gnus-valid-select-methods' without checking the value anyway).

    > And here you mention that exact fact :)

    > I agree it doesn't make much sense, it doesn't do anything for the user.
    > I wonder if `gnus-declare-backend' was originally just meant to be used
    > by out-of-tree backend libraries.

Maybe the Gnus manual should indicate that `gnus-valid-select-methods'
should be modified directly for built-in backends (currently it only
describes using `gnus-declare-backend' in Hooking New Backends Into Gnus).

    > Anyway, it's redundant, but no great harm done, and the tests pass.

Thanks.
I also realized there is an example in nnfeed.el which is wrong in the
same way, I've attached a patch which fixes it.

Daniel


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

From e4e8ed37b92125a7b68d91755cdf77d00af4b14c Mon Sep 17 00:00:00 2001
From: Daniel Semyonov <daniel@dsemy.com>
Date: Mon, 29 Apr 2024 23:40:50 +0300
Subject: [PATCH] ; * lisp/gnus/nnfeed.el: Fix example

---
 lisp/gnus/nnfeed.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/gnus/nnfeed.el b/lisp/gnus/nnfeed.el
index 0bf599553e4..d6963b2e929 100644
--- a/lisp/gnus/nnfeed.el
+++ b/lisp/gnus/nnfeed.el
@@ -46,7 +46,7 @@
 ;;    (defvoo nnfoo-read-feed-function #'nnfoo--read-feed
 ;;     nil nnfeed-read-feed-function)
 ;;    ...
-;;    (gnus-declare-backend (symbol-name nnfeed-backend) 'address)
+;;    (gnus-declare-backend (symbol-name nnfeed-backend) 'none 'address)
 ;;
 ;;    (provide 'nnfoo)
 ;;
-- 
2.44.0

Re: master 1601c5a518d: Gnus: Add back end for Atom feeds (nnatom)

[Michael Albinus]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Anyway, it's redundant, but no great harm done, and the tests pass.

Confirmed.

> Eric

Best regards, Michael.