Gnus development mailing list
 help / color / mirror / Atom feed
* Bug fix for B c and B m
@ 2001-10-20  7:08 Matt Armstrong
  2001-10-20  9:50 ` Simon Josefsson
  0 siblings, 1 reply; 3+ messages in thread
From: Matt Armstrong @ 2001-10-20  7:08 UTC (permalink / raw)


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

I'd appreciate it if someone with CVS commit privs could look at this
and consider committing it.

Bug summary: "B c nnfolder:foo" when no nnfolder:foo group or even
nnfolder: server exists yet will end up putting the group with
nnfolder+archive: (at least partially).  In fact, an nnfolder:foo
group is created, but nnfolder+archive: stuff does end up getting set
in .newsrc.eld and things are generally weird afterward.

I tracked this down to a use of gnus-group-method, which seems to do a
prefix match on the server portion of the group name and consider it a
match.  So the nnfolder+archive server will satisfy a request for a
server for nnfolder:FOO.

We don't want this for "B c" and "B m" since the user is specifying an
exact server name.  A comment in gnus-group-method says "you probably
want to be using gnus-find-method-for-group", and so I changed
gnus-sum.el to do so.  The problem is fixed.
gnus-find-method-for-group seems to handle the "I'm creating a new
method" case correctly by making up a new '(nnfolder "") select
method.

Patch attached.

-- 
matt


[-- Attachment #2: Patch for \"B c\" bug --]
[-- Type: text/plain, Size: 1641 bytes --]

? patch.txt
Index: ChangeLog
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/ChangeLog,v
retrieving revision 6.779
diff -u -r6.779 ChangeLog
--- ChangeLog	2001/10/19 14:58:54	6.779
+++ ChangeLog	2001/10/20 07:04:24
@@ -1,3 +1,9 @@
+2001-10-20  Matt Armstrong  <matt@lickey.com>
+
+	* gnus-sum.el (gnus-summary-move-article): Use
+	gnus-find-method-for-group, not gnus-group-method.
+	(gnus-read-move-group-name): Ditto.
+
 2001-10-19  Per Abrahamsen  <abraham@dina.kvl.dk>
 
 	* mm-decode.el (mm-default-directory): Fix customize type.
Index: gnus-sum.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/gnus-sum.el,v
retrieving revision 6.117
diff -u -r6.117 gnus-sum.el
--- gnus-sum.el	2001/10/18 19:49:05	6.117
+++ gnus-sum.el	2001/10/20 07:04:49
@@ -8021,7 +8021,7 @@
       (set (intern (format "gnus-current-%s-group" action)) to-newsgroup))
     (setq to-method (or select-method
 			(gnus-server-to-method
-			 (gnus-group-method to-newsgroup))))
+			 (gnus-find-method-for-group to-newsgroup))))
     ;; Check the method we are to move this article to...
     (unless (gnus-check-backend-function
 	     'request-accept-article (car to-method))
@@ -9960,7 +9960,8 @@
 					  (nreverse split-name))
 				  nil nil nil
 				  'gnus-group-history))))
-	 (to-method (gnus-server-to-method (gnus-group-method to-newsgroup))))
+	 (to-method (gnus-server-to-method
+		     (gnus-find-method-for-group to-newsgroup))))
     (when to-newsgroup
       (if (or (string= to-newsgroup "")
 	      (string= to-newsgroup prefix))

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Bug fix for B c and B m
  2001-10-20  7:08 Bug fix for B c and B m Matt Armstrong
@ 2001-10-20  9:50 ` Simon Josefsson
  2001-10-20 23:24   ` Matt Armstrong
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Josefsson @ 2001-10-20  9:50 UTC (permalink / raw)
  Cc: ding

Matt Armstrong <matt@lickey.com> writes:

> I'd appreciate it if someone with CVS commit privs could look at this
> and consider committing it.
> 
> Bug summary: "B c nnfolder:foo" when no nnfolder:foo group or even
> nnfolder: server exists yet will end up putting the group with
> nnfolder+archive: (at least partially).  In fact, an nnfolder:foo
> group is created, but nnfolder+archive: stuff does end up getting set
> in .newsrc.eld and things are generally weird afterward.
> 
> I tracked this down to a use of gnus-group-method, which seems to do a
> prefix match on the server portion of the group name and consider it a
> match.  So the nnfolder+archive server will satisfy a request for a
> server for nnfolder:FOO.
> 
> We don't want this for "B c" and "B m" since the user is specifying an
> exact server name.  A comment in gnus-group-method says "you probably
> want to be using gnus-find-method-for-group", and so I changed
> gnus-sum.el to do so.  The problem is fixed.
> gnus-find-method-for-group seems to handle the "I'm creating a new
> method" case correctly by making up a new '(nnfolder "") select
> method.
> 
> Patch attached.

Does your patch work with new groups?  Compare with:

2000-03-01  Simon Josefsson  <jas@pdc.kth.se>

	* gnus-sum.el (gnus-read-move-group-name):
	(gnus-summary-move-article): Use `gnus-group-method' to find out
	what method the manually entered group belong to.
	`gnus-group-name-to-method' doesn't return any method parameters
	and `gnus-find-method-for-group' uses `gnus-group-name-to-method'
	for new groups so they wouldn't work.

Looking at `gnus-group-method' it looks it should do the right thing
for new groups, and it seem to work when I run it interactively as
well.  Can you edebug it for some example that gives the wrong answer
and post it here?

> 
> -- 
> matt
> 
> ? patch.txt
> Index: ChangeLog
> ===================================================================
> RCS file: /usr/local/cvsroot/gnus/lisp/ChangeLog,v
> retrieving revision 6.779
> diff -u -r6.779 ChangeLog
> --- ChangeLog	2001/10/19 14:58:54	6.779
> +++ ChangeLog	2001/10/20 07:04:24
> @@ -1,3 +1,9 @@
> +2001-10-20  Matt Armstrong  <matt@lickey.com>
> +
> +	* gnus-sum.el (gnus-summary-move-article): Use
> +	gnus-find-method-for-group, not gnus-group-method.
> +	(gnus-read-move-group-name): Ditto.
> +
>  2001-10-19  Per Abrahamsen  <abraham@dina.kvl.dk>
>  
>  	* mm-decode.el (mm-default-directory): Fix customize type.
> Index: gnus-sum.el
> ===================================================================
> RCS file: /usr/local/cvsroot/gnus/lisp/gnus-sum.el,v
> retrieving revision 6.117
> diff -u -r6.117 gnus-sum.el
> --- gnus-sum.el	2001/10/18 19:49:05	6.117
> +++ gnus-sum.el	2001/10/20 07:04:49
> @@ -8021,7 +8021,7 @@
>        (set (intern (format "gnus-current-%s-group" action)) to-newsgroup))
>      (setq to-method (or select-method
>  			(gnus-server-to-method
> -			 (gnus-group-method to-newsgroup))))
> +			 (gnus-find-method-for-group to-newsgroup))))
>      ;; Check the method we are to move this article to...
>      (unless (gnus-check-backend-function
>  	     'request-accept-article (car to-method))
> @@ -9960,7 +9960,8 @@
>  					  (nreverse split-name))
>  				  nil nil nil
>  				  'gnus-group-history))))
> -	 (to-method (gnus-server-to-method (gnus-group-method to-newsgroup))))
> +	 (to-method (gnus-server-to-method
> +		     (gnus-find-method-for-group to-newsgroup))))
>      (when to-newsgroup
>        (if (or (string= to-newsgroup "")
>  	      (string= to-newsgroup prefix))




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Bug fix for B c and B m
  2001-10-20  9:50 ` Simon Josefsson
@ 2001-10-20 23:24   ` Matt Armstrong
  0 siblings, 0 replies; 3+ messages in thread
From: Matt Armstrong @ 2001-10-20 23:24 UTC (permalink / raw)
  Cc: ding

Simon Josefsson <jas@extundo.com> writes:

> Does your patch work with new groups?  Compare with:
> 
> 2000-03-01  Simon Josefsson  <jas@pdc.kth.se>
> 
> 	* gnus-sum.el (gnus-read-move-group-name):
> 	(gnus-summary-move-article): Use `gnus-group-method' to find out
> 	what method the manually entered group belong to.
> 	`gnus-group-name-to-method' doesn't return any method parameters
> 	and `gnus-find-method-for-group' uses `gnus-group-name-to-method'
> 	for new groups so they wouldn't work.

Hmm...


> Looking at `gnus-group-method' it looks it should do the right thing
> for new groups, and it seem to work when I run it interactively as
> well.  Can you edebug it for some example that gives the wrong
> answer and post it here?

Sure, I did this last night.

The problem is gnus-group-method's behavior when there is no exact
match for the method.  In this case, gnus-group-method will use the
last server it saw that had the same back end.  It seems that when
this happens, things break.

For example, I have no "nnfolder" method.  I have an nnfolder+archive
and nnfolder+save (the latter I just created for testing purposes).

    (insert (pp (gnus-group-method "nnfolder+foo:bar")))
    (nnfolder "save"
              (nnfolder-directory "~/g/nnfolder+save")
              (nnfolder-active-file "~/g/nnfolder+save/active")
              (nnfolder-newsgroups-file "~/g/nnfolder+save/newsgroups")
              (nnfolder-get-new-mail nil))

    (insert (pp (gnus-find-method-for-group "nnfolder+foo:bar")))
    (nnfolder "foo")

I think the second one is more correct.

But I do see what you mean with gnus-find-method-for-group not
returning method parameters:

    (insert (pp (gnus-group-method "nnfolder+save:bar")))
    (nnfolder "save"
              (nnfolder-directory "~/g/nnfolder+save")
              (nnfolder-active-file "~/g/nnfolder+save/active")
              (nnfolder-newsgroups-file "~/g/nnfolder+save/newsgroups")
              (nnfolder-get-new-mail nil))

    (insert (pp (gnus-find-method-for-group "nnfolder+save:bar")))
    (nnfolder "save")



With current CVS (no local patches), I do a "B c nnfolder+foo:bar" and
Gnus prompts me if I want to create the group, I say yes.  But
problems are not far off because some code (namely, that code using
gnus-find-method-for-group) thinks the group's method is (nnfolder
"foo") and other code thinks it is (nnfolder "save" ...).


-- 
matt



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2001-10-20 23:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-10-20  7:08 Bug fix for B c and B m Matt Armstrong
2001-10-20  9:50 ` Simon Josefsson
2001-10-20 23:24   ` Matt Armstrong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).