B DEL is being treated as expiry?

B DEL is being treated as expiry?

[Karl Kleinpaste]

Lately, when I do `B DEL' on messages in nnml:personal.general, the
articles get expired rather than deleted, which means that instead of
disappearing, they get moved to nnml:archive.current.personal.general,
because that's the value of expiry-target in group parameters.

This is very wrong.  When I use `B DEL' in personal.general, it's
usually because spam managed to slide past my nnmail-split-rules and
got into the wrong place.

So when I hit `B DEL', I want an article to go away, now and forever.
Deletion is not expiry.  Looking over the ChangeLog, this is the
comment reflecting the change which caused this.

2002-07-09  Nevin Kapur  <Nevin Kapur <nevin@jhu.edu>
	* gnus-sum.el (gnus-summary-delete-article): Respect group
	parameters while expiring.

I see what -delete-article is doing now, and it's just plain wrong.
If an -expire-right-now function is wanted, then that should be
something separate.  "Delete" is not the same thing.

Re: B DEL is being treated as expiry?

[Kai Großjohann]

Karl Kleinpaste <karl@charcoal.com> writes:

> I see what -delete-article is doing now, and it's just plain wrong.
> If an -expire-right-now function is wanted, then that should be
> something separate.  "Delete" is not the same thing.

The current situation is better than before Nevin's change, because
before Nevin's change, _some_ parameters got respected and others
didn't.  So depending on how you set expiry-target, B DEL would
respect it.  Now at least it's consistent.

But a variant that ignores expiry-target would be useful, I agree.

kai
-- 
A large number of young women don't trust men with beards.  (BFBS Radio)

Re: B DEL is being treated as expiry?

[Josh Huber]

Kai.Grossjohann@CS.Uni-Dortmund.DE (Kai Großjohann) writes:

> Karl Kleinpaste <karl@charcoal.com> writes:
>
>> I see what -delete-article is doing now, and it's just plain wrong.
>> If an -expire-right-now function is wanted, then that should be
>> something separate.  "Delete" is not the same thing.
>
> The current situation is better than before Nevin's change, because
> before Nevin's change, _some_ parameters got respected and others
> didn't.  So depending on how you set expiry-target, B DEL would
> respect it.  Now at least it's consistent.
>
> But a variant that ignores expiry-target would be useful, I agree.

I think the current command should delete, and if another command is
needed (expire-immediate?), then that should be made.  The help text,
and everyone's expectations seem to think it should immediately delete
the message:

"This command actually deletes articles.  This is not a marking
command.  The article will disappear forever from your life, never to
return."

ttyl,

-- 
Josh Huber

Re: B DEL is being treated as expiry?

[Nevin Kapur]

Kai.Grossjohann@CS.Uni-Dortmund.DE (Kai Großjohann) writes:

> But a variant that ignores expiry-target would be useful, I agree.

So far everybody seems to agree that B DEL causing expiry is just
wrong.  How about we go ahead and change it so that B DEL really
deletes like the manual suggests?

-Nevin

Re: B DEL is being treated as expiry?

[Karl Kleinpaste]

Kai.Grossjohann@CS.Uni-Dortmund.DE (Kai Großjohann) writes:
> The current situation is better than before Nevin's change, 

No, it isn't, because it used to be the case that when I said, "delete
this," *it was deleted*.  Perhaps there was some flaw in how deletion
was done, but *at least articles went away* as ordered.  It makes no
sense to say, "the current situation is better, now that the basic
operation of the function in question is to do something completely
foreign to what the function is designed and advertised to do."

> because before Nevin's change, _some_ parameters got respected and
> others didn't.  So depending on how you set expiry-target, B DEL
> would respect it.  Now at least it's consistent.

The only group parameter consistency that has meaning here is to
ignore them all, because the article is going away forever, and the
ongoing operation of group parameters can have no meaning for an
article file which doesn't even exist.

At the very least, the change which induced expiry should be reverted
so that we get deletions back.

Re: B DEL is being treated as expiry?

[Paul Jarc]

Karl Kleinpaste <karl@charcoal.com> wrote:
> Kai.Grossjohann@CS.Uni-Dortmund.DE (Kai Großjohann) writes:
>> The current situation is better than before Nevin's change,
>
> No, it isn't, because it used to be the case that when I said, "delete
> this," *it was deleted*.

That's only because you don't set the nnmail-expiry-target variable.
If you did, this would have been already broken before the change.
B DEL has obeyed n-e-t for a long time, and now it obeys expiry-target
parameters for consistency.  This way, the problem annoys *everyone*,
and so it gets fixed sooner.  See how that's better? :)

> At the very least, the change which induced expiry should be reverted
> so that we get deletions back.

I don't think that would be an improvement now.

We want both functions available (delete immediately/expire
immediately), right?  So how shall we express this in the backend
interface?  Right now, there's just one "force" argument to
nnchoke-request-expire-articles.  Should we just pass different values
there to get expiry/deletion?  Or would we ever want deletion without
immediacy?  I.e., operate on only the "old enough" articles, but
delete them instead of expiring them?  If we want that, we'd probably
have to add another argument; "force" could indicate immediacy, and
"mode" could indicate expiry vs. deletion.


paul

Re: B DEL is being treated as expiry?

[Karl Kleinpaste]

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

prj@po.cwru.edu (Paul Jarc) writes:
> That's only because you don't set the nnmail-expiry-target variable.

Fine; easily fixed:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: \"consistent\" handling of article deletion --]
[-- Type: text/x-patch, Size: 501 bytes --]

--- gnus-sum.el.~1~	Sun Jul 21 08:26:48 2002
+++ gnus-sum.el	Tue Jul 23 12:20:21 2002
@@ -8837,9 +8837,7 @@
     (error "Couldn't open server"))
   ;; Compute the list of articles to delete.
   (let ((articles (sort (copy-sequence (gnus-summary-work-articles n)) '<))
-	(nnmail-expiry-target
-	 (or (gnus-group-find-parameter gnus-newsgroup-name 'expiry-target)
-	     nnmail-expiry-target))
+	(nnmail-expiry-target 'delete)
 	not-deleted)
     (if (and gnus-novice-user
 	     (not (gnus-yes-or-no-p

Re: B DEL is being treated as expiry?

[Paul Jarc]

Karl Kleinpaste <karl@charcoal.com> wrote:
> --- gnus-sum.el.~1~	Sun Jul 21 08:26:48 2002
> +++ gnus-sum.el	Tue Jul 23 12:20:21 2002
> @@ -8837,9 +8837,7 @@
>      (error "Couldn't open server"))
>    ;; Compute the list of articles to delete.
>    (let ((articles (sort (copy-sequence (gnus-summary-work-articles n)) '<))
> -	(nnmail-expiry-target
> -	 (or (gnus-group-find-parameter gnus-newsgroup-name 'expiry-target)
> -	     nnmail-expiry-target))
> +	(nnmail-expiry-target 'delete)
>  	not-deleted)
>      (if (and gnus-novice-user
>  	     (not (gnus-yes-or-no-p

If this works for you, that's fine, but it's not a generally correct
fix.  We need to sort out the design issues I mentioned.  Also, the
backend interface is not specified in terms of nnmail-expiry-target;
this wouldn't fix nnmaildir, for example.


paul

Re: B DEL is being treated as expiry?

[Karl Kleinpaste]

prj@po.cwru.edu (Paul Jarc) writes:
> If this works for you, that's fine, but it's not a generally correct
> fix.  We need to sort out the design issues I mentioned.

No, we don't.

gnus-summary-delete-article is intended to delete articles, period.
My 1-line fix makes it do that, again, for both the non-default
nnmail-expiry-target case as well as the group parms expiry-target
case.  This makes -delete-article operationally correct, deleting
articles unconditionally, as advertised and intended (according to
both help text and novice-user assist query).

If you have other issues, such as nnmaildir not coping, then you have
a problem specific to that, needing a bugfix for nnmaildir.  And if an
-expire-article-right-now function is wanted, that is a separate
question, completely disjoint from whether -delete-article works.  You
may have a design issue related to how to do that, but my bugfix takes
care of the outright flaw of -delete-article so that it works properly.

Re: B DEL is being treated as expiry?

[Paul Jarc]

Karl Kleinpaste <karl@charcoal.com> wrote:
> gnus-summary-delete-article is intended to delete articles, period.

Right.

> My 1-line fix makes it do that, again, for both the non-default
> nnmail-expiry-target case as well as the group parms expiry-target
> case.

But only by chance, and only for certain backends.  The backend
interface must define how this is expressed, and all backends must be
made to conform to that specification.  g-s-d-a is not supposed to
know the details of how backends decide what to do with expirable
articles; it's only supposed to know what to pass to
nnchoke-request-expire-articles.

> If you have other issues, such as nnmaildir not coping, then you have
> a problem specific to that, needing a bugfix for nnmaildir.

The problem isn't specific to nnmaildir; your fix is specific to
nnmail-derived backends.  I don't know, but I'd guess that it also
fails for nnimap.

> And if an -expire-article-right-now function is wanted, that is a
> separate question, completely disjoint from whether -delete-article
> works.

It isn't completely disjoint, because it's likely that we'll want to
use the same backend function for both purposes.  (Otherwise we'll be
duplicating a significant amount of code.)  So the interface for that
function must define how the two cases are distinguished.

> You may have a design issue related to how to do that, but my bugfix
> takes care of the outright flaw of -delete-article so that it works
> properly.

No, it only makes the symptom disappear for cases like yours.


paul

Re: B DEL is being treated as expiry?

[Karl Kleinpaste]

prj@po.cwru.edu (Paul Jarc) writes:
> The backend interface must define how this is expressed, and all
> backends must be made to conform to that specification.

So make them conform.

Why is this hard?

I solved today's problem, so that things just plain *work* according
to established intent -- so that -delete-article *stops making mistakes*.

You have a view toward a different problem; feel free to solve that.
I am of the opinion that this different problem is most readily solved
merely by convincing the currently non-conforming backends to
understand nnmail-expiry-target, plus adding 1 general function +
keybinding to provide expire-right-now as well.

> g-s-d-a is not supposed to know the details of how backends decide
> what to do with expirable articles; it's only supposed to know what
> to pass to nnchoke-request-expire-articles.

-delete-article doesn't know squat about anything to do with expiry at
all -- it isn't *doing* expiry so it doesn't *care* about expiry,
other than to turn it off unconditionally.  It deletes, nothing more
and nothing less.  Having been broken, it now does so again, more
consistently and properly than it used to, apparently.

It sounds like the problem you have is that there exists any use of
nnmail-expiry-target.  Feel free to redesign that, if the urge moves
you, but I think you're working too hard.

> The problem isn't specific to nnmaildir; your fix is specific to
> nnmail-derived backends.  I don't know, but I'd guess that it also
> fails for nnimap.

The fix is general with regard to _today's_ expectation that
nnmail-expiry-target is the specified way to define expiry habits, by
means of overriding any general expiry intention so that deletion, and
only deletion, is what occurs.  gnus-request-expire-articles works
this way, and -delete-article accommodates it.

> It isn't completely disjoint, because it's likely that we'll want to
> use the same backend function for both purposes.  (Otherwise we'll
> be duplicating a significant amount of code.)

Please.  The totality of -delete-article is 40 lines.  You can find
numerous examples of duplicated code in Gnus much larger than that.
Just wander through the various backends which are derived from one
another.  (Once upon a midnight dreary, nnml came out of nnspool.)

Re: B DEL is being treated as expiry?

[Paul Jarc]

Karl Kleinpaste <karl@charcoal.com> wrote:
> prj@po.cwru.edu (Paul Jarc) writes:
>> The backend interface must define how this is expressed, and all
>> backends must be made to conform to that specification.
>
> So make them conform.
>
> Why is this hard?

I never said it was hard.  I really don't think it is hard to fix the
backend interface documentation and then to fix the backends.

> I solved today's problem, so that things just plain *work* according
> to established intent -- so that -delete-article *stops making mistakes*.

It stops making mistakes *for you*.

> You have a view toward a different problem; feel free to solve that.

I'm looking at the more general case of the same problem, as well as a
different but related problem.

> I am of the opinion that this different problem is most readily solved
> merely by convincing the currently non-conforming backends to
> understand nnmail-expiry-target, plus adding 1 general function +
> keybinding to provide expire-right-now as well.

Currently, all backends conform equally well to the backend interface
spec (AFAIK).  The spec does not say that backends should delete
instead of expiring in any particular case, and the spec does not give
callers enough information to know (in general) how to tell backends
to delete instead of expiring.

And since your fix has not been installed AFAIK, all backends fail to
DTRT for B DEL.  If you say all backends should be fixed, I agree.
But I don't think your patch is the right way to do that.

>> g-s-d-a is not supposed to know the details of how backends decide
>> what to do with expirable articles; it's only supposed to know what
>> to pass to nnchoke-request-expire-articles.
>
> -delete-article doesn't know squat about anything to do with expiry at
> all -- it isn't *doing* expiry so it doesn't *care* about expiry,
> other than to turn it off unconditionally.

But it doesn't know how to do that via nn*-request-expire-articles.
The backend interace doesn't say that that's even possible at all.

> It sounds like the problem you have is that there exists any use of
> nnmail-expiry-target.  Feel free to redesign that, if the urge moves
> you, but I think you're working too hard.

No, I don't have a problem with that.  I have a problem with the
disconnect between the code and the documentation.  If the code does
not conform to the backend interface definition, such as by making
unwarranted assumptions about how to configure expiry behavior for all
backends, then the backend interface definition is worse than useless;
it's harmful, because it leads people to believe things that the code
does not agree with.

Ideally, I think it would be nice for all the expiry code to be
removed from the backend interface.  Given
nn*-request-{move,accept}-article, Gnus can do expiry itself, and more
uniformly than when backends do it themselves.

>> The problem isn't specific to nnmaildir; your fix is specific to
>> nnmail-derived backends.  I don't know, but I'd guess that it also
>> fails for nnimap.
>
> The fix is general with regard to _today's_ expectation that
> nnmail-expiry-target is the specified way to define expiry habits, by
> means of overriding any general expiry intention so that deletion, and
> only deletion, is what occurs.  gnus-request-expire-articles works
> this way, and -delete-article accommodates it.

I don't mind too much that those Gnus functions help the
nnmail-derived backends this way, although it's a bit ugly.  But the
fix for this problem should work for all backends, not just the nnmail
ones.  The expectation that n-e-t is the way to define expiry behavior
is itself derived from using nnmail-derived backends.  Other backends
exist and must be accounted for.

>> It isn't completely disjoint, because it's likely that we'll want to
>> use the same backend function for both purposes.  (Otherwise we'll
>> be duplicating a significant amount of code.)
>
> Please.  The totality of -delete-article is 40 lines.

I'm talking about nn*-request-expire-articles.  Separate
nn*-request-delete-articles functions would duplicate much of that
code, I think.

But we already have nn*-request-move-article, which can be used to
remove articles instead of expiring them.  Why don't we change B DEL
to use that?  That should work for all backends.

> You can find numerous examples of duplicated code in Gnus much
> larger than that.

That doesn't sound like a good reason for adding more.


paul

Re: B DEL is being treated as expiry?

[Kai Großjohann]

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

> Ideally, I think it would be nice for all the expiry code to be
> removed from the backend interface.  Given
> nn*-request-{move,accept}-article, Gnus can do expiry itself, and more
> uniformly than when backends do it themselves.

Yay!  Way to go!  Just implement nnchoke-request-delete-article in
all backends, and Gnus does the rest.

Hm.  Except for computing the time for expiry.  I think different
backends use different methods.  That's not good.  Maybe some
additional mechanism is needed for the time computation.

kai
-- 
A large number of young women don't trust men with beards.  (BFBS Radio)

Re: B DEL is being treated as expiry?

[Paul Jarc]

Kai.Grossjohann@CS.Uni-Dortmund.DE (Kai Großjohann) wrote:
> prj@po.cwru.edu (Paul Jarc) writes:
>> Ideally, I think it would be nice for all the expiry code to be
>> removed from the backend interface.  Given
>> nn*-request-{move,accept}-article, Gnus can do expiry itself, and more
>> uniformly than when backends do it themselves.
>
> Yay!  Way to go!  Just implement nnchoke-request-delete-article in
> all backends, and Gnus does the rest.

nnchoke-request-move-article can already be used for that purpose,
given a suitable accept-form.  These are the backends that have
-request-expire-articles but not -request-move-article: nnrss,
nnslashdot, nnsoup, nnvirtual.  Do we care about them for B DEL, or
are they too special-purpose?  Could we add -request-move-article if
it's needed?

> Hm.  Except for computing the time for expiry.  I think different
> backends use different methods.  That's not good.  Maybe some
> additional mechanism is needed for the time computation.

Yes, that variation is confusing.  It would be nice to make all
backends the same by making Gnus decide when an article is old enough.
And if different ways of deciding that are useful, it should be an
explicit configuration setting, orthogonal to the choice of backend.
Gnus would need to be able to ask the backend when the article arrived
in the group, which isn't currently possible, althoguh the Date field
is always accessible.  Anyway, this is separate from the B DEL
problem.


paul

Re: B DEL is being treated as expiry?

[Simon Josefsson]

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

>> My 1-line fix makes it do that, again, for both the non-default
>> nnmail-expiry-target case as well as the group parms expiry-target
>> case.
>
> But only by chance, and only for certain backends.  The backend
> interface must define how this is expressed, and all backends must be
> made to conform to that specification.  g-s-d-a is not supposed to
> know the details of how backends decide what to do with expirable
> articles; it's only supposed to know what to pass to
> nnchoke-request-expire-articles.

I think the problem under discussion only happens for nnmail backends,
since the problem only happens when expiry targets which is a nnmail
thingie.  So making the problem go away for nnmail backends only would
solve it.

(It also works for nnimap.  Nnimap is not a nnmail backend strictly
speaking, but it uses most of the same variable anyway, for extra
confusion.)

Re: B DEL is being treated as expiry?

[Paul Jarc]

Simon Josefsson <jas@extundo.com> wrote:
> I think the problem under discussion only happens for nnmail backends,
> since the problem only happens when expiry targets which is a nnmail
> thingie.

No, nnmaildir has an equivalent but independent feature.  Anyway, it
seems that we want two functions: immediate deletion and time-based
expiration, where the behavior on expiration is configurable.  Are we
satisfied that those are the only features we want from
nnchoke-request-expire-articles?  If so, then let's update the backend
interface documentation to specify that the "force" argument indicates
both immediacy and deletion instead of expiration.  Then let's fix the
code to make it so.  Or, if we're not satisfied with those two
functions, let's figure out what else we want, and then update the
docs and code.


paul

Re: B DEL is being treated as expiry?

[Simon Josefsson]

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

> Simon Josefsson <jas@extundo.com> wrote:
>> I think the problem under discussion only happens for nnmail backends,
>> since the problem only happens when expiry targets which is a nnmail
>> thingie.
>
> No, nnmaildir has an equivalent but independent feature.  

Which is used even when `force' is non-nil?  I think the feature
should be deactivated in that case.

Hm.  Really, nnml and other backends shouldn't use the
nnmail-expiry-target feature when force is non-nil as well.  This
would be the proper fix, instead of binding nnmail-expiry-target to
'delete in gnus-sum.el.  Someone who like to take a look at it?

> Anyway, it seems that we want two functions: immediate deletion and
> time-based expiration, where the behavior on expiration is
> configurable.  Are we satisfied that those are the only features we
> want from nnchoke-request-expire-articles?  If so, then let's update
> the backend interface documentation to specify that the "force"
> argument indicates both immediacy and deletion instead of
> expiration.  Then let's fix the code to make it so.  Or, if we're
> not satisfied with those two functions, let's figure out what else
> we want, and then update the docs and code.

I thought the force parameter indicated immediate deletion.  Isn't the
docs clear?

`(nnchoke-request-expire-articles ARTICLES &optional GROUP SERVER FORCE)'
     This function should run the expiry process on all articles in the
     ARTICLES range (which is currently a simple list of article
     numbers.)  It is left up to the back end to decide how old articles
     should be before they are removed by this function.  If FORCE is
     non-`nil', all ARTICLES should be deleted, no matter how new they
     are.

     This function should return a list of articles that it did not/was
     not able to delete.

     There should be no result data returned.

Re: B DEL is being treated as expiry?

[Paul Jarc]

Simon Josefsson <jas@extundo.com> wrote:
> prj@po.cwru.edu (Paul Jarc) writes:
>> No, nnmaildir has an equivalent but independent feature.
>
> Which is used even when `force' is non-nil?

I think so, but nnmaildir has a little other brokenness in there that
I need to fix anyway.

> Hm.  Really, nnml and other backends shouldn't use the
> nnmail-expiry-target feature when force is non-nil as well.  This
> would be the proper fix, instead of binding nnmail-expiry-target to
> 'delete in gnus-sum.el.

That sounds good, if that is in fact what "force" is supposed to mean.

> I thought the force parameter indicated immediate deletion.  Isn't the
> docs clear?

It's unclear because it doesn't really mention the difference between
deletion and expiration at all, so the mere juxtaposition of "force"
with "deleted" isn't as suggestive as it might be.  If that is what
it's supposed to mean, then great - let's make the docs more explicit
and fix the backends accordingly.


paul

Re: B DEL is being treated as expiry?

[Simon Josefsson]

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

>> I thought the force parameter indicated immediate deletion.  Isn't the
>> docs clear?
>
> It's unclear because it doesn't really mention the difference between
> deletion and expiration at all, so the mere juxtaposition of "force"
> with "deleted" isn't as suggestive as it might be.  If that is what
> it's supposed to mean, then great - let's make the docs more explicit
> and fix the backends accordingly.

I have always interpreted it like that, but I may be wrong.  Opinions?
I'm not sure how it would be possible to express "really delete this
article NOW" using the backend interface if FORCE didn't have this
meaning though.

Re: B DEL is being treated as expiry?

[Nevin Kapur]

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

> We want both functions available (delete immediately/expire
> immediately), right?  

Do we really?  Expiring immediately is almost a oxymoron.  Expiry (as
it works in Gnus) suggests something passive and deletion something
active.  I like Karl's patch.  That's what I would like to see go
through.

-Nevin

Re: B DEL is being treated as expiry?

[Paul Jarc]

Nevin Kapur <nevin@jhu.edu> wrote:
> prj@po.cwru.edu (Paul Jarc) writes:
>> We want both functions available (delete immediately/expire
>> immediately), right?
>
> Do we really?  Expiring immediately is almost a oxymoron.  Expiry (as
> it works in Gnus) suggests something passive and deletion something
> active.  I like Karl's patch.  That's what I would like to see go
> through.

Regardless of whether we want immediate expiration, immediate deletion
should work for all backends; Karl's patch doesn't do that.  Are there
any objections to using -request-move-article for deletion in
gnus-summary-delete-article ?  I think that will work for all
backends.

Regarding immediate expiration, is that what
gnus-summary-expire-articles-now is supposed to do?  Or should it
delete as well?


paul

Re: B DEL is being treated as expiry?

[Nevin Kapur]

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

> Regardless of whether we want immediate expiration, immediate deletion
> should work for all backends; 

Indeed.

> Karl's patch doesn't do that.  Are there any objections to using
> -request-move-article for deletion in gnus-summary-delete-article ?
> I think that will work for all backends.

I haven't heard any objections to B DEL deleting without running the
expiry process so if -r-m-a works for all backends it ought to be
used.

> Regarding immediate expiration, is that what
> gnus-summary-expire-articles-now is supposed to do?  Or should it
> delete as well?

I don't this g-s-e-a-n should delete, though that seems contrary to
its documentation. 


-Nevin