Re: master b2e6e95581: Only reset buffer-local buffer-stale-function in make-indirect-buffer

Re: master b2e6e95581: Only reset buffer-local buffer-stale-function in make-indirect-buffer

[Stefan Monnier]

> +      if (!NILP (Flocal_variable_p (Qbuffer_stale_function, base_buffer)))
> +	Fset (Qbuffer_stale_function, Qbuffer_stale__default_function);

Shouldn't this use `kill-local-variable` instead or something like that?



        Stefan

Re: master b2e6e95581: Only reset buffer-local buffer-stale-function in make-indirect-buffer

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Mon, 11 Jul 2022 11:59:20 -0400
> 
> > +      if (!NILP (Flocal_variable_p (Qbuffer_stale_function, base_buffer)))
> > +	Fset (Qbuffer_stale_function, Qbuffer_stale__default_function);
> 
> Shouldn't this use `kill-local-variable` instead or something like that?

I'm also not sure it is wise not to use nil here, since
buffer-stale--default-function is only defined when files.el is
loaded, so this could cause trouble during bootstrap (if not now, then
in some distant future).  At least Ffboundp test is in order, I think.

Re: master b2e6e95581: Only reset buffer-local buffer-stale-function in make-indirect-buffer

[Eli Zaretskii]

> Date: Mon, 11 Jul 2022 20:20:31 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org, larsi@gnus.org
> 
> > > +      if (!NILP (Flocal_variable_p (Qbuffer_stale_function, base_buffer)))
> > > +	Fset (Qbuffer_stale_function, Qbuffer_stale__default_function);
> > 
> > Shouldn't this use `kill-local-variable` instead or something like that?
> 
> I'm also not sure it is wise not to use nil here, since
> buffer-stale--default-function is only defined when files.el is
> loaded, so this could cause trouble during bootstrap (if not now, then
> in some distant future).  At least Ffboundp test is in order, I think.

Come to think of this: why do we test that the variable is
buffer-local?  This is in make-indirect-buffer, where the buffer was
not yet "released" into the world, so how come it could have this
variable as buffer-local already?

I think the code should instead _force_ the variable to be
buffer-local, _and_ unconditionally set its buffer-local value to nil.

Re: master b2e6e95581: Only reset buffer-local buffer-stale-function in make-indirect-buffer

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +      if (!NILP (Flocal_variable_p (Qbuffer_stale_function, base_buffer)))
>> +	Fset (Qbuffer_stale_function, Qbuffer_stale__default_function);
>
> Shouldn't this use `kill-local-variable` instead or something like that?

Yup; now adjusted.

Eli Zaretskii <eliz@gnu.org> writes:

> I'm also not sure it is wise not to use nil here, since
> buffer-stale--default-function is only defined when files.el is
> loaded, so this could cause trouble during bootstrap (if not now, then
> in some distant future).  At least Ffboundp test is in order, I think.

nil is documented to be a legacy value, so it shouldn't be used.

Eli Zaretskii <eliz@gnu.org> writes:

> Come to think of this: why do we test that the variable is
> buffer-local?  This is in make-indirect-buffer, where the buffer was
> not yet "released" into the world, so how come it could have this
> variable as buffer-local already?

clone_per_buffer_values copies the local variables from the base buffer.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master b2e6e95581: Only reset buffer-local buffer-stale-function in make-indirect-buffer

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: emacs-devel@gnu.org
> Date: Tue, 12 Jul 2022 14:38:53 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'm also not sure it is wise not to use nil here, since
> > buffer-stale--default-function is only defined when files.el is
> > loaded, so this could cause trouble during bootstrap (if not now, then
> > in some distant future).  At least Ffboundp test is in order, I think.
> 
> nil is documented to be a legacy value, so it shouldn't be used.

What about the scenario I presented, where
buffer-stale--default-function is not yet known?

Re: master b2e6e95581: Only reset buffer-local buffer-stale-function in make-indirect-buffer

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> nil is documented to be a legacy value, so it shouldn't be used.
>
> What about the scenario I presented, where
> buffer-stale--default-function is not yet known?

I pushed Stefan's suggested fix, which just calls kill-local-variable,
which should to the right thing in any case.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no