Subject: Re: bug#33263: 27.0.50; Tidying up Gnus modes

From: Eric Abrahamsen <[email-masked]>
Date: Mon, 05 Nov 2018 09:51:22 -0800

Noam Postavsky <[email-masked]> writes:

> Eric Abrahamsen <[email-masked]> writes:
>
>> +(defvar gnus-browse-mode-map (make-keymap))
>>
>>  (unless gnus-browse-mode-map
>> -  (setq gnus-browse-mode-map (make-keymap))
>> +  (defvar gnus-browse-mode-map (make-keymap))
>
> This doesn't look right.

Hmm, you're right, I think I got confused there. TBH I don't really know
why these things are wrapped in `unless', but I think what should have
happened is that `gnus-browse-mode-map' is first defvar'ed to nil, and
later setq'ed to (make-keymap).

>> -(defun gnus-summary-mode (&optional group)
>> +(define-derived-mode gnus-summary-mode gnus-mode "Summary"
>>    "Major mode for reading articles.
>
>> -      (gnus-summary-mode group)
>> +      (setq gnus-newsgroup-name group)
>> +      (gnus-summary-mode)
>>        (when (gnus-group-quit-config group)
>
> Where does GROUP come from now?

GROUP has always been passed down through gnus-group-read-group (which
picks it up from the group under point) => gnus-summary-read-group =>
gnus-summary-read-group-1 => gnus-summary-setup-buffer =>
gnus-summary-mode.

When gnus-summary-mode was a function it was passed GROUP as an argument
and set it as the (buffer-local) gnus-newsgroup-name variable. I
couldn't see anything in the rest of gnus-summary-mode that would need
that variable set, but you never know if Gnus is checking something four
levels deep. So I pulled that up and set it in
gnus-summary-setup-buffer, before the call to the major mode, forgetting
that calling gnus-summary-mode would of course clear out buffer-local
variables. *And* failing to notice that gnus-summary-setup-buffer sets
the variable *again* later on. So it was originally set twice.

I've been testing for a while, and don't think anything called from the
gnus-summary-mode code needs to be aware of gnus-newsgroup-name, but I
will look a little closer. Anything that *does* rely on knowing which
group we're opening should definitely be hoisted up to
gnus-summary-setup-buffer. Then I'll just delete the first setting of
gnus-newsgroup-name and leave the later one.

Thanks for looking at this.