Definition reasoning of revert-buffer-function, should it be defvar-local?

Definition reasoning of revert-buffer-function, should it be defvar-local?

[Daniel Nagy]

Hello,

I want to understand the reasoning behind the definition and the intention of
the variable `revert-buffer-function`.

In `files.el` I see the following definition:

>    (put 'revert-buffer-function 'permanent-local t)
>    (defvar revert-buffer-function #'revert-buffer--default)

The first line suggests to me, that the intention of the variable is that it is
supposed to be buffer-local, but `defvar-local` was not used. This means, that
if I open a new buffer and `(setq revert-buffer-function #'something)` then that
variable is assigned globally, breaking its behaviour. I know that I can use
`setq-local`, but using `defvar-local` would prevent accidents.

My question is, why was `defvar-local` not used? Should we change it ? Should we
change all these constructs where it says `(put ... 'permanent-local t)(defvar
...)`?

In `bookmark.el` I can also see the invocation `(setq revert-buffer-function
...)` but that instance does not leak out its value to global Emacs because its
mode is derived from `tabulated-list-mode` and that already uses `setq-local` to
make the value buffer-local.

Thank you already for your help.

For the more curious, this question sparked during this GitHub issue:
https://github.com/magit/magit/issues/4432

Best,
Daniel

Re: Definition reasoning of revert-buffer-function, should it be defvar-local?

[Andreas Schwab]

On Aug 10 2021, Daniel Nagy wrote:

> I want to understand the reasoning behind the definition and the intention of
> the variable `revert-buffer-function`.
>
> In `files.el` I see the following definition:
>
>>    (put 'revert-buffer-function 'permanent-local t)
>>    (defvar revert-buffer-function #'revert-buffer--default)
>
> The first line suggests to me, that the intention of the variable is that it is
> supposed to be buffer-local, but `defvar-local` was not used.

When these lines were written, defvar-local didn't exist.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Definition reasoning of revert-buffer-function, should it be defvar-local?

[Tassilo Horn]

Andreas Schwab <schwab@linux-m68k.org> writes:

>> I want to understand the reasoning behind the definition and the
>> intention of the variable `revert-buffer-function`.
>>
>> In `files.el` I see the following definition:
>>
>>>    (put 'revert-buffer-function 'permanent-local t)
>>>    (defvar revert-buffer-function #'revert-buffer--default)
>>
>> The first line suggests to me, that the intention of the variable is that it is
>> supposed to be buffer-local, but `defvar-local` was not used.
>
> When these lines were written, defvar-local didn't exist.

Hehe, sure.  But why hasn't `make-variable-buffer-local' been used?  I
agree with Daniel that nobody should ever alter the global value of
`revert-buffer-function' but it is easy to do that accidentally.

Bye,
Tassilo