Re: new Mail-Followup-To patch...please take a look...

[prj@po.cwru.edu (Paul Jarc)]

Josh Huber <huber@alum.wpi.edu> wrote:
> +(defvar gnus-group-dirty-flag nil)

The name's rather vague.  But if Per's functions are fast enough, then
I agree that caching is not worthwhile anyway.

> +(defun gnus-find-subscribed-addresses-real ()
> +  "Return a regexp matching the addresses of all subscribed mail groups.

Since this is used in message-subscribed-address-functions:

> +	 (mft-regexps (apply 'append message-subscribed-regexps
> +			     (mapcar 'regexp-quote
> +				     message-subscribed-addresses)
> +			     (mapcar 'funcall
> +				     message-subscribed-address-functions))))

Shouldn't it return a list of regexps (possibly just one, but still a
list)?

> +    (progn
> +      (setq gnus-group-dirty-flag nil)
> +      (setq gnus-group-parameter-cache gnus-parameters)
> +      (setq gnus-subscribed-addresses
> +	    (gnus-find-subscribed-addresses-real)))))

(Assuming caching is not discarded...)  Better to make that
gnus-atomic-progn, or to rearrange like so:
(progn
  (setq gnus-subscribed-addresses (gnus-find-subscribed-addresses-real))
  (setq gnus-group-dirty-flag nil)
  (setq gnus-group-parameter-cache gnus-parameters)
  gnus-subscribed-addresses)
  
Otherwise a C-g could come in after we've updated our flags, but
before we've recalculated the subscribed addresses, thus making our
regexp falsely seem up to date.

> +(defvar message-subscribed-address-functions '(ignore)
> +  "*Specifies functions for determining list subscription.

The structure of the return value needs to be documented, and
"specifies" seems to be unnecessary, judging by existing docstrings.
"*A list of functions, each returning a list of regular expressions
matching addresses of mailing lists the the user is subscribed to."?

> +If nil, do not attempt to determine list subscribtion with functions.
> +If non-nil, this variable contains a list of functions which return
> +regular expressions to match lists.

nil is a list; I don't think it needs to be separately documented.

> +(defun message-goto-mail-followup-to ()
> +  "Move point to the Mail-Followup-To header."
> +  (interactive)
> +  (message-position-on-field "Mail-Followup-To" "From"))
> +

This, at least (along with its key and menu bindings), can be
committed now, I think.


paul