gnus-agent typo?

gnus-agent typo?

[Kai Großjohann]

What do you think about the following patch for gnus-agent.el?  I
guess that unseen was meant instead of seen.

@@ -1102,7 +1108,7 @@
 	gnus-agent-cache)
     ;; Add article with marks to list of article headers we want to fetch.
     (dolist (arts (gnus-info-marks (gnus-get-info group)))
-      (unless (memq (car arts) '(seen recent))
+      (unless (memq (car arts) '(unseen recent))
 	(setq articles (gnus-range-add articles (cdr arts)))))
     (setq articles (sort (gnus-uncompress-sequence articles) '<))
     ;; Remove known articles.


kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Paul Jarc]

Kai.Grossjohann@CS.Uni-Dortmund.DE (Kai Großjohann) wrote:
> What do you think about the following patch for gnus-agent.el?  I
> guess that unseen was meant instead of seen.
>
> @@ -1102,7 +1108,7 @@
>  	gnus-agent-cache)
>      ;; Add article with marks to list of article headers we want to fetch.
>      (dolist (arts (gnus-info-marks (gnus-get-info group)))
> -      (unless (memq (car arts) '(seen recent))
> +      (unless (memq (car arts) '(unseen recent))
>  	(setq articles (gnus-range-add articles (cdr arts)))))
>      (setq articles (sort (gnus-uncompress-sequence articles) '<))
>      ;; Remove known articles.

Gnus has no unseen mark, AFAIK.  Gnus has a seen mark, and displays
"." in the *Summary* buffer for articles that *lack* the seen mark.
So I think it ought to be:
      (unless (eq (car arts) 'recent)
And then the 'seen articles should be removed from the range
afterwards.


paul

Re: gnus-agent typo?

[Kai Großjohann]

prj@po.cwru.edu (Paul Jarc) writes:

> Kai.Grossjohann@CS.Uni-Dortmund.DE (Kai Großjohann) wrote:
>> What do you think about the following patch for gnus-agent.el?  I
>> guess that unseen was meant instead of seen.
>>
>> @@ -1102,7 +1108,7 @@
>>  	gnus-agent-cache)
>>      ;; Add article with marks to list of article headers we want to fetch.
>>      (dolist (arts (gnus-info-marks (gnus-get-info group)))
>> -      (unless (memq (car arts) '(seen recent))
>> +      (unless (memq (car arts) '(unseen recent))
>>  	(setq articles (gnus-range-add articles (cdr arts)))))
>>      (setq articles (sort (gnus-uncompress-sequence articles) '<))
>>      ;; Remove known articles.
>
> Gnus has no unseen mark, AFAIK.  Gnus has a seen mark, and displays
> "." in the *Summary* buffer for articles that *lack* the seen mark.
> So I think it ought to be:
>       (unless (eq (car arts) 'recent)
> And then the 'seen articles should be removed from the range
> afterwards.

How about this?  Is this the right logic?

(when (or (eq (car arts) 'seen)
          (not (eq (car arts) 'recent)))
  (setq articles (gnus-range-add articles (cdr arts))))

I switched from unless to when because I don't like no double
negatives...

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Paul Jarc]

Kai.Grossjohann@CS.Uni-Dortmund.DE (Kai Großjohann) wrote:
> prj@po.cwru.edu (Paul Jarc) writes:
>> So I think it ought to be:
>>       (unless (eq (car arts) 'recent)
>> And then the 'seen articles should be removed from the range
>> afterwards.

Actually, this is not quite right; see below.

> How about this?  Is this the right logic?
>
> (when (or (eq (car arts) 'seen)
>           (not (eq (car arts) 'recent)))
>   (setq articles (gnus-range-add articles (cdr arts))))

I don't think it can be made that simple.  Your version can be
optimized to:
(when (not (eq (car arts) 'recent))
  (setq articles (gnus-range-add articles (cdr arts))))
because if (eq (car arts) 'seen), then since (not (eq 'seen 'recent)),
it follows that (not (eq (car arts) 'recent)); so the first clause of
the or form is redundant.  And since this optimized version doesn't
use 'seen at all, it's clearly not what we want.

AIUI, the range should not contain recent or unseen articles, but
should contain all others - i.e., it should contain exactly those
articles that are not unseen and not recent (IOW, seen and not
recent).  (That's my guess based on a rather small piece of code;
maybe I'm wrong.  The original logic made a range of articles that
were unseen but not recent, but that doesn't sound useful.)  The way
to get that is to subtract the 'recent range from (a copy of) the
'seen range.  (Is there a gnus-range function for that?)  We don't
even need the dolist at all.


paul

Re: gnus-agent typo?

[Kai Großjohann]

prj@po.cwru.edu (Paul Jarc) writes:

> AIUI, the range should not contain recent or unseen articles, but
> should contain all others - i.e., it should contain exactly those
> articles that are not unseen and not recent (IOW, seen and not
> recent).  (That's my guess based on a rather small piece of code;
> maybe I'm wrong.  The original logic made a range of articles that
> were unseen but not recent, but that doesn't sound useful.)

No, the original logic took a list of articles and then _added_
marked ones, except that it didn't add seen or recent ones.

Maybe that's the best solution.

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Paul Jarc]

kai.grossjohann@uni-duisburg.de (Kai Großjohann) wrote:
> No, the original logic took a list of articles and then _added_
> marked ones, except that it didn't add seen or recent ones.
>
> Maybe that's the best solution.

You guessed that the original logic was buggy, and I guess the same,
because it seems odd to treat seen and recent articles similarly.
(Could someone who knows what that code is really supposed to do
comment here?)  We would expect *un*seen and recent articles to be
treated similarly, since they're semantically similar - but since
they're implemented differently (recent is a normal mark; unseen is
the lack of the seen mark), the code should look different than it
does now.  So to fix the apparent bug, but otherwise produce the same
range, we should take the initial list of article and add (seen -
recent).  (This is different from ((initial + seen) - recent).)


paul

Re: gnus-agent typo?

[Kai Großjohann]

prj@po.cwru.edu (Paul Jarc) writes:

> You guessed that the original logic was buggy, and I guess the same,
> because it seems odd to treat seen and recent articles similarly.
> (Could someone who knows what that code is really supposed to do
> comment here?)  We would expect *un*seen and recent articles to be
> treated similarly, since they're semantically similar - but since
> they're implemented differently (recent is a normal mark; unseen is
> the lack of the seen mark), the code should look different than it
> does now.  So to fix the apparent bug, but otherwise produce the same
> range, we should take the initial list of article and add (seen -
> recent).  (This is different from ((initial + seen) - recent).)

The original code is supposed to add ticked and dormant messages, I
think.  And messages with other marks.  Except for the unseen and the
recent mark.

I think adding (seen - recent) does not achieve the objective of
abstaining from adding unseen articles.

What the code does is to go through the list of ticked articles and
add them.  Then it goes through the list of dormant articles and adds
them.

But it should not go through the list of unseen articles and add
them.  So it should also not go through the list of seen articles and
remove them.

See?

The original logic was correct after all, even if it is
counter-intuitive.

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Paul Jarc]

kai.grossjohann@uni-duisburg.de (Kai Großjohann) wrote:
> The original code is supposed to add ticked and dormant messages, I
> think.  And messages with other marks.  Except for the unseen and the
> recent mark.

So it's supposed to add articles that have the ticked/dormant/whatever
mark, regardless of whether they are seen/unseen/recent/non-recent,
but it should not add or remove article solely on the basis of being
seen/unseen/recent/non-recent.  Is that right?  In that case, the
original code was ok, so your original patch (changing seen to unseen)
should be reverted.  (Right?)  And apparently, an explanatory comment
is needed. :)

> I think adding (seen - recent) does not achieve the objective of
> abstaining from adding unseen articles.

Maybe it adds some that it shouldn't (lacking ticked/dormant/...), but
it doesn't add any unseen articles.  It adds a subset of the seen
range, which by definition has no intersection with unseen.

> What the code does is to go through the list of ticked articles and
> add them.  Then it goes through the list of dormant articles and adds
> them.

And expirable, and replied, and forwarded, and ....  Is that intended?


paul

Re: gnus-agent typo?

[Kai Großjohann]

prj@po.cwru.edu (Paul Jarc) writes:

> kai.grossjohann@uni-duisburg.de (Kai Großjohann) wrote:
>> The original code is supposed to add ticked and dormant messages, I
>> think.  And messages with other marks.  Except for the unseen and the
>> recent mark.
>
> So it's supposed to add articles that have the ticked/dormant/whatever
> mark, regardless of whether they are seen/unseen/recent/non-recent,
> but it should not add or remove article solely on the basis of being
> seen/unseen/recent/non-recent.  Is that right?

That's my understanding.  For ticked and dormant messages, it is
clear that it is useful to download them.

(mapcar 'car (gnus-info-marks (gnus-get-info gnus-newsgroup-name)))
=> (seen cache score expire reply tick)

It is, however, somewhat unclear why it would be useful to fetch
cached or scored messages.

> In that case, the original code was ok, so your original patch
> (changing seen to unseen) should be reverted.  (Right?)  And
> apparently, an explanatory comment is needed. :)

I reverted my change and added a comment some days ago.

>> I think adding (seen - recent) does not achieve the objective of
>> abstaining from adding unseen articles.
>
> Maybe it adds some that it shouldn't (lacking ticked/dormant/...), but
> it doesn't add any unseen articles.  It adds a subset of the seen
> range, which by definition has no intersection with unseen.

But it adds some that it shouldn't.

>> What the code does is to go through the list of ticked articles and
>> add them.  Then it goes through the list of dormant articles and adds
>> them.
>
> And expirable, and replied, and forwarded, and ....  Is that intended?

I wonder.  Does anyone know?

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Simon Josefsson]

kai.grossjohann@uni-duisburg.de (Kai Großjohann) writes:

> prj@po.cwru.edu (Paul Jarc) writes:
>
>> kai.grossjohann@uni-duisburg.de (Kai Großjohann) wrote:
>>> The original code is supposed to add ticked and dormant messages, I
>>> think.  And messages with other marks.  Except for the unseen and the
>>> recent mark.
>>
>> So it's supposed to add articles that have the ticked/dormant/whatever
>> mark, regardless of whether they are seen/unseen/recent/non-recent,
>> but it should not add or remove article solely on the basis of being
>> seen/unseen/recent/non-recent.  Is that right?
>
> That's my understanding.  For ticked and dormant messages, it is
> clear that it is useful to download them.
>
> (mapcar 'car (gnus-info-marks (gnus-get-info gnus-newsgroup-name)))
> => (seen cache score expire reply tick)
>
> It is, however, somewhat unclear why it would be useful to fetch
> cached or scored messages.

I think the reason to download all marked articles was that the Agent
would remove flags on non-existing articles otherwise.  I guess if we
make Agent not remove flags for non-existing articles, we can remove
the workaround to download all marked articles.

Re: gnus-agent typo?

[Kai Großjohann]

Simon Josefsson <jas@extundo.com> writes:

> I think the reason to download all marked articles was that the Agent
> would remove flags on non-existing articles otherwise.

Really?  Why does it do that?

> I guess if we make Agent not remove flags for non-existing articles,
> we can remove the workaround to download all marked articles.

Ah.

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Simon Josefsson]

kai.grossjohann@uni-duisburg.de (Kai Großjohann) writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> I think the reason to download all marked articles was that the Agent
>> would remove flags on non-existing articles otherwise.
>
> Really?  Why does it do that?

I have no idea. :-) Maybe because it was difficult to distinguish
between non-existing and non-downloaded articles.  Hm.  But it is easy
to make that distinction.  It is probably best to remove the code and
see what breaks and fix that...

Re: gnus-agent typo?

[Kai Großjohann]

Simon Josefsson <jas@extundo.com> writes:

> kai.grossjohann@uni-duisburg.de (Kai Großjohann) writes:
>
>> Simon Josefsson <jas@extundo.com> writes:
>>
>>> I think the reason to download all marked articles was that the Agent
>>> would remove flags on non-existing articles otherwise.
>>
>> Really?  Why does it do that?
>
> I have no idea. :-) Maybe because it was difficult to distinguish
> between non-existing and non-downloaded articles.  Hm.  But it is easy
> to make that distinction.  It is probably best to remove the code and
> see what breaks and fix that...

Right.  Did you see where it happens?  Do you have time to delete the
code?

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Kai Großjohann]

Simon Josefsson <jas@extundo.com> writes:

> I have no idea. :-) Maybe because it was difficult to distinguish
> between non-existing and non-downloaded articles.  Hm.  But it is easy
> to make that distinction.  It is probably best to remove the code and
> see what breaks and fix that...

I see that things are done with the marks when the agent expires.
For example, all articles up to the first one from
gnus-agent-article-alist are marked as read.

Is this the code you're talking about?

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Simon Josefsson]

kai.grossjohann@uni-duisburg.de (Kai Großjohann) writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> I have no idea. :-) Maybe because it was difficult to distinguish
>> between non-existing and non-downloaded articles.  Hm.  But it is easy
>> to make that distinction.  It is probably best to remove the code and
>> see what breaks and fix that...
>
> I see that things are done with the marks when the agent expires.
> For example, all articles up to the first one from
> gnus-agent-article-alist are marked as read.
>
> Is this the code you're talking about?

Sorry, I don't remember and don't have time to look at the agent
currently.  Maybe the code has even been removed since I last debugged
it.  Easiest is probably to remove the hack and see what breaks (if
anything).

Re: gnus-agent typo?

[Kai Großjohann]

Simon Josefsson <jas@extundo.com> writes:

> Sorry, I don't remember and don't have time to look at the agent
> currently.  Maybe the code has even been removed since I last debugged
> it.  Easiest is probably to remove the hack and see what breaks (if
> anything).

I presume the "hack" is the code that downloads marked articles,
too?  Okay, I will remove that.

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Simon Josefsson]

kai.grossjohann@uni-duisburg.de (Kai Großjohann) writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> Sorry, I don't remember and don't have time to look at the agent
>> currently.  Maybe the code has even been removed since I last debugged
>> it.  Easiest is probably to remove the hack and see what breaks (if
>> anything).
>
> I presume the "hack" is the code that downloads marked articles,
> too?  Okay, I will remove that.

Yup.  I think I remember now -- the marks code in gnus-sum optimized
the group marks so it doesn't contain marks for articles that doesn't
exist (when you quit a group), I think it was that code that removed
the marks.  If quitting a group doesn't lose marks on non-downloaded
marked articles now, I think this concern doesn't apply any longer.

Re: gnus-agent typo?

[Kai Großjohann]

Simon Josefsson <jas@extundo.com> writes:

> Yup.  I think I remember now -- the marks code in gnus-sum optimized
> the group marks so it doesn't contain marks for articles that doesn't
> exist (when you quit a group), I think it was that code that removed
> the marks.  If quitting a group doesn't lose marks on non-downloaded
> marked articles now, I think this concern doesn't apply any longer.

Actually, I have no idea what that code might do, I didn't change
gnus-sum.el at all.  I shall investigate to see if I understand the
code.

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Clemens Fischer]

Simon Josefsson <jas@extundo.com>:

> [...]  It is probably best to remove the code and
> see what breaks and fix that...

may i use this statement on a funnies page?  ;)

clemens

Re: gnus-agent typo?

[Paul Jarc]

kai.grossjohann@uni-duisburg.de (Kai Großjohann) wrote:
> I reverted my change and added a comment some days ago.

Ah, there it is.  I didn't see it before for some reason.  But I think
that comment would leave me with (almost) the same impression that I
got from the original code: that seen and recent articles should not
be fetched, regardless of what other marks they may have.  But that's
not what the code does.  I think this would be clearer (also
incorporating Simon's note):

--- lisp/gnus-agent.el	2002/10/23 20:32:01	6.91
+++ lisp/gnus-agent.el	2002/10/24 15:10:57
@@ -1108,9 +1108,11 @@
 	(gnus-decode-encoded-word-function 'identity)
 	(file (gnus-agent-article-name ".overview" group))
 	gnus-agent-cache)
-    ;; Add article with marks to list of article headers we want to
-    ;; fetch.  We don't want to fetch all the seen articles, and we
-    ;; don't want do fetch the recent ones, though.
+    ;; Add articles with marks to the list of article headers we want to
+    ;; fetch.  Don't fetch articles solely on the basis of a recent or seen
+    ;; mark, but do fetch recent or seen articles if they have other, more
+    ;; interesting marks.  (We have to fetch articles with boring marks
+    ;; like 'reply because otherwise the agent will remove their marks.)
     (dolist (arts (gnus-info-marks (gnus-get-info group)))
       (unless (memq (car arts) '(seen recent))
 	(setq articles (gnus-range-add articles (cdr arts)))))


paul

Re: gnus-agent typo?

[Kai Großjohann]

prj@po.cwru.edu (Paul Jarc) writes:

> Ah, there it is.  I didn't see it before for some reason.  But I think
> that comment would leave me with (almost) the same impression that I
> got from the original code: that seen and recent articles should not
> be fetched, regardless of what other marks they may have.  But that's
> not what the code does.  I think this would be clearer (also
> incorporating Simon's note):

I like your patch.  Thanks a lot!  Are you committing it?

kai
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: gnus-agent typo?

[Paul Jarc]

kai.grossjohann@uni-duisburg.de (Kai Großjohann) wrote:
> I like your patch.  Thanks a lot!  Are you committing it?

I have now.


paul