bug#33263: 27.0.50; Tidying up Gnus modes

[Eric Abrahamsen]

Noam Postavsky <npostavs@gmail.com> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Noam Postavsky <npostavs@gmail.com> writes:
>>
>>> Eric Abrahamsen <eric@ericabrahamsen.net> 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).
>
> The usual idiom for this is
>
>     (defvar gnus-browse-mode-map
>       (let ((map (make-sparse-keymap)))
>         (define-key map ...)
>         ...
>         map))
>
> It seems that (the expansion of) gnus-define-keys relies on dynamic
> binding of the map variable (i.e., it must be called after the defvar,
> not inside), so that's the reason to split in two parts.

Right, I got that much, I was mostly wondering why the `unless' -- it's
unlikely these files would be loaded multiple times, and even if they
were, that wouldn't break anything, would it?