* Re: gnus shouldn't be making general-purpose variables buffer-local
2008-12-22 18:55 ` gnus shouldn't be making general-purpose variables buffer-local Reiner Steib
@ 2008-12-22 20:53 ` Ami Fischman
2008-12-24 2:32 ` Katsumi Yamaoka
1 sibling, 0 replies; 3+ messages in thread
From: Ami Fischman @ 2008-12-22 20:53 UTC (permalink / raw)
To: Reiner Steib, Ami Fischman, ding, emacs-devel,
emacs-jabber-general
Thanks for adding me back to the thread, Reiner.
David's patch does solve my problem.
In case it's helpful for others who don't want to edit their copy of
gnus-sum.el, until now I'd been making do with:
(add-hook 'gnus-select-group-hook
(lambda ()
(kill-local-variable 'timestamp)))
with no apparent ill-effect.
-a
On Mon, Dec 22, 2008 at 10:55 AM, Reiner Steib
<reinersteib+gmane@imap.cc> wrote:
> [ Adding Ami Fischman, emacs-devel and emacs-jabber (again).
> (See
> <http://thread.gmane.org/gmane.emacs.gnus.general/67886/focus=67943>,
> <http://thread.gmane.org/gmane.emacs.devel/105256> also for the
> complete threads.) ]
>
> On Wed, Dec 17 2008, Katsumi Yamaoka wrote:
>
>> At least for `timestamp', the attached patch will solve (note
>> that you need to reload the patched gnus-group.elc because of
>> `defsubst'). At the first time you enter to a group, the buffer-
>> local variable `timestamp' is still alive, but it will be renamed
>> to `gnus-timestamp' when exiting the group. Maybe the change
>> will not slow Gnus.
> [...]
>> --- gnus-group.el~ 2008-10-03 05:47:11 +0000
>> +++ gnus-group.el 2008-12-17 10:18:31 +0000
>> @@ -4608,11 +4608,13 @@
>> (when gnus-newsgroup-name
>> (let ((time (current-time)))
>> (setcdr (cdr time) nil)
>> - (gnus-group-set-parameter gnus-newsgroup-name 'timestamp time))))
>> + (gnus-group-set-parameter gnus-newsgroup-name 'gnus-timestamp time)
>> + (gnus-group-remove-parameter gnus-newsgroup-name 'timestamp))))
>>
>> (defsubst gnus-group-timestamp (group)
>> "Return the timestamp for GROUP."
>> - (gnus-group-get-parameter group 'timestamp t))
>> + (or (gnus-group-get-parameter group 'gnus-timestamp t)
>> + (gnus-group-get-parameter group 'timestamp t)))
>
> After the initial report on emacs-devel, I wrote this quite similar
> patch:
>
> --8<---------------cut here---------------start------------->8---
> --- gnus-group.el 10 Sep 2008 03:28:52 +0200 1.77
> +++ gnus-group.el 03 Nov 2008 00:17:44 +0100
> @@ -4608,11 +4608,17 @@
> (when gnus-newsgroup-name
> (let ((time (current-time)))
> (setcdr (cdr time) nil)
> - (gnus-group-set-parameter gnus-newsgroup-name 'timestamp time))))
> + ;; Remove obsolete `timestamp' (used until 2008-11-03) if present.
> + ;; Note: This breaks down-grading.
> + (when (gnus-group-get-parameter group 'timestamp t)
> + (gnus-group-remove-parameter group 'timestamp))
> + (gnus-group-set-parameter gnus-newsgroup-name 'gnus-group-timestamp time))))
>
> (defsubst gnus-group-timestamp (group)
> "Return the timestamp for GROUP."
> - (gnus-group-get-parameter group 'timestamp t))
> + (or (gnus-group-get-parameter group 'gnus-group-timestamp t)
> + ;; For compatibility, check `timestamp' (used until 2008-11-03) as well.
> + (gnus-group-get-parameter group 'timestamp t)))
>
> (defun gnus-group-timestamp-delta (group)
> "Return the offset in seconds from the timestamp for GROUP to the current time, as a floating point number."
> --8<---------------cut here---------------end--------------->8---
>
> Beside the other name, it removes the old parameter `timestamp'.
>
> However, I wonder if the more general patch suggested by David Engster
> is better. Does anyone see a problem with it?
>
> Ami, does David's patch solve your problem?
>
> --8<---------------cut here---------------start------------->8---
> --- a/lisp/gnus-sum.el
> +++ b/lisp/gnus-sum.el
> @@ -3831,6 +3831,7 @@ This function is intended to be used in
> (and (consp elem) ; Has to be a cons.
> (consp (cdr elem)) ; The cdr has to be a list.
> (symbolp (car elem)) ; Has to be a symbol in there.
> + (boundp (car elem)) ; Has to be already bound
> (not (memq (car elem) vars))
> (ignore-errors ; So we set it.
> (push (car elem) vars)
> --8<---------------cut here---------------end--------------->8---
>
> Bye, Reiner.
> --
> ,,,
> (o o)
> ---ooO-(_)-Ooo--- | PGP key available | http://rsteib.home.pages.de/
>
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: gnus shouldn't be making general-purpose variables buffer-local
2008-12-22 18:55 ` gnus shouldn't be making general-purpose variables buffer-local Reiner Steib
2008-12-22 20:53 ` Ami Fischman
@ 2008-12-24 2:32 ` Katsumi Yamaoka
1 sibling, 0 replies; 3+ messages in thread
From: Katsumi Yamaoka @ 2008-12-24 2:32 UTC (permalink / raw)
To: ding; +Cc: emacs-jabber-general, Ami Fischman, emacs-devel
>>>>> Reiner Steib wrote:
> However, I wonder if the more general patch suggested by David Engster
> is better. Does anyone see a problem with it?
> Ami, does David's patch solve your problem?
> --- a/lisp/gnus-sum.el
> +++ b/lisp/gnus-sum.el
> @@ -3831,6 +3831,7 @@ This function is intended to be used in
> (and (consp elem) ; Has to be a cons.
> (consp (cdr elem)) ; The cdr has to be a list.
> (symbolp (car elem)) ; Has to be a symbol in there.
> + (boundp (car elem)) ; Has to be already bound
> (not (memq (car elem) vars))
> (ignore-errors ; So we set it.
> (push (car elem) vars)
>>>>> In <b4mfxkms7yl.fsf@jpl.org> Katsumi Yamaoka wrote:
> Cool! But I agree not to use it. :)
But I found no evil with that patch so far. Variables like gnus-*
globally bound need to get to be buffer-local but it's harmless.
Moreover, those parameters have been to be set as (VAR VAL), not
(VAR . VAL). OTOH, parameters used as just parameters, e.g.
`timestamp', should not need to be bound; those are set in the
`gnus-parameters' variable or the newsrc database. Only one anxiety
is the case that a user or some program binds such a variable, but
the fault will lie with the user or what should be complained will
be the program. So, I've installed David Engster's patch with a
comment: http://article.gmane.org/gmane.emacs.gnus.commits/6091
Regards,
^ permalink raw reply [flat|nested] 3+ messages in thread