Re: Changes in calendar/time-date.el

[Lute Kamstra]

Reiner Steib <address@hidden> writes:

>>> Wouldn't it be better to put (require 'parse-time) at the beginning of
>>> the file instead of inside `message-make-date'?
>>
>> message-make-date is the only function in message.el that uses
>> parse-time, so it's best to put the require there.  If you change
>> message-make-date so that it no longer uses parse-time, you will
>> probably notice the require and remove it.  Had you put require at the
>> beginning of the file, you would most likely forget this.  Putting a
>> require at the beginning of a file is useful if the file uses a
>> particular feature a lot.

I forgot to mention that putting the require inside the defun has the
advantage that the feature gets loaded at the moment it it needed.  If
the function is not called, the feature isn't loaded.

> Okay, I agree.  Putting it at the beginning of a file additionally
> avoids some "reference to free variable" warnings.  The following
> patch[1] would avoid them too.  (autoload 'parse-time-string ...)
> should be there for the standalone Gnus which is supposed to work with
> older Emacs versions.
>
> Index: message.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/gnus/message.el,v
> retrieving revision 1.75
> diff -u -r1.75 message.el
> --- message.el        30 Mar 2005 08:14:32 -0000      1.75
> +++ message.el        4 Apr 2005 11:36:12 -0000
> @@ -32,6 +32,8 @@
>  
>  (eval-when-compile
>    (require 'cl)
> +  (defvar parse-time-weekdays) ;; parse-time is required where necessary
> +  (defvar parse-time-months)
>    (defvar gnus-message-group-art)
>    (defvar gnus-list-identifiers)) ; gnus-sum is required where necessary
>  (require 'canlock)

Can't you move this closer to the definition of message-make-date?
It's only necessary to suppress compiler warnings for that function.
Alternatively, you can use (with-no-warnings ...) around references to
these variables.

> Index: nnimap.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/gnus/nnimap.el,v
> retrieving revision 1.23
> diff -u -r1.23 nnimap.el
> --- nnimap.el 18 Mar 2005 02:49:57 -0000      1.23
> +++ nnimap.el 4 Apr 2005 11:36:12 -0000
> @@ -69,6 +69,8 @@
>  (require 'gnus-range)
>  (require 'gnus-start)
>  (require 'gnus-int)
> + ;; Only for `parse-time-months' in `nnimap-date-days-ago':
> +(require 'parse-time)
>  
>  (eval-when-compile (require 'cl))
>  

Why not put the require in the body of nnimap-date-days-ago?

> Index: nnultimate.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/gnus/nnultimate.el,v
> retrieving revision 1.5
> diff -u -r1.5 nnultimate.el
> --- nnultimate.el     4 Sep 2004 13:13:44 -0000       1.5
> +++ nnultimate.el     4 Apr 2005 11:36:12 -0000
> @@ -39,6 +39,7 @@
>  (require 'mm-util)
>  (require 'mm-url)
>  (require 'nnweb)
> +(require 'parse-time)
>  (autoload 'w3-parse-buffer "w3-parse")
>  
>  (nnoo-declare nnultimate)
> Index: gnus-art.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/gnus/gnus-art.el,v
> retrieving revision 1.65
> diff -u -r1.65 gnus-art.el
> --- gnus-art.el       18 Mar 2005 02:49:57 -0000      1.65
> +++ gnus-art.el       4 Apr 2005 11:36:13 -0000
> @@ -46,6 +46,7 @@
>  (autoload 'gnus-msg-mail "gnus-msg" nil t)
>  (autoload 'gnus-button-mailto "gnus-msg")
>  (autoload 'gnus-button-reply "gnus-msg" nil t)
> +(autoload 'parse-time-string "parse-time" nil nil)
>  
>  (defgroup gnus-article nil
>    "Article display."
> Index: gnus-demon.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/gnus/gnus-demon.el,v
> retrieving revision 1.9
> diff -u -r1.9 gnus-demon.el
> --- gnus-demon.el     4 Sep 2004 13:13:43 -0000       1.9
> +++ gnus-demon.el     4 Apr 2005 11:36:13 -0000
> @@ -40,6 +40,8 @@
>        (require 'itimer)
>      (require 'timer)))
>  
> +(autoload 'parse-time-string "parse-time" nil nil)
> +
>  (defgroup gnus-demon nil
>    "Demonic behaviour."
>    :group 'gnus)
> Index: gnus-delay.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/gnus/gnus-delay.el,v
> retrieving revision 1.5
> diff -u -r1.5 gnus-delay.el
> --- gnus-delay.el     9 Feb 2005 15:50:39 -0000       1.5
> +++ gnus-delay.el     4 Apr 2005 11:36:13 -0000
> @@ -37,6 +37,7 @@
>  
>  (require 'nndraft)
>  (require 'gnus-draft)
> +(autoload 'parse-time-string "parse-time" nil nil)
>  
>  ;;;###autoload
>  (defgroup gnus-delay nil
>
> Does it look correct?

The rest looks fine to me.

Lute.