Subject: Re: bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook

From: Eli Zaretskii <[email-masked]>
Date: Fri, 26 Apr 2019 14:26:31 +0300

> Cc: [email-masked], [email-masked], [email-masked]
> From: martin rudalics <[email-masked]>
> Date: Fri, 26 Apr 2019 13:00:06 +0200
> 
> 
> I attach a preliminary patch.

Thanks.  This will need documentation changes when pushed.

> +/**
> + * run_buffer_list_update_hook:
> + *
> + * Run any functions on 'buffer-list-update-hook'.  Do not run the
> + * functions when BUFFER is a buffer and its inhibit_buffer_hooks
> + * structure element is set.  Do not run any functions either when we
> + * are not allowed to run hooks.
> + */

Can we please use our style in commentary?  We don't use bock comments
in Emacs, so I'd like not to proliferate them.

> +  if (!inhibit_buffer_hooks)
> +    /* Run buffer-list-update-hook.  */
> +    run_buffer_list_update_hook (buffer);

It is somewhat strange that part of the reasons for not running hooks
are tested inside run_buffer_list_update_hook, and others explicitly
here.  Any special reasons for this inconsistency?

> +DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0,
> +       doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed.
> +If BUFFER-OR-NAME is a string and a live buffer with that name exists,
> +return that buffer.  If no such buffer exists, create a new buffer with
> +that name and return it.  If BUFFER-OR-NAME starts with a space, the new
> +buffer does not keep undo information.
> +
> +If BUFFER-OR-NAME is a buffer instead of a string, return it as given,
> +even if it is dead.  The return value is never nil.  */)
> +     (Lisp_Object buffer_or_name)
> +{
> +  return get_buffer_create (buffer_or_name, false);
> +}

Should this function also acquire an additional optional argument?  If
not, why not?

> +Optional second argument INHIBIT-BUFFER-HOOKS non-nil means to not run
> +any buffer hooks ('kill-buffer-hook', 'buffer-list-update-hook' or
> +'kill-buffer-query-functions') for this buffer.  This argument should

The hooks should be quoted `like this', right?  We do want them to
become hyperlinks.

> +be set only for internal buffers that are never presented to users or
> +passed on to other applications.  */)
> +  (Lisp_Object name, Lisp_Object inhibit_buffer_hooks)
> +{
> +  Lisp_Object buffer_name = Fgenerate_new_buffer_name (name, Qnil);
> +  Lisp_Object buffer = get_buffer_create (buffer_name,
> +					  !NILP (inhibit_buffer_hooks));
> +
> +  if (!NILP (inhibit_buffer_hooks))
> +    {
> +     struct buffer *b = XBUFFER (buffer);
> +
> +     b->inhibit_buffer_hooks = true;
> +    }

Should this flag be set inside get_buffer_create?

> +Neither 'kill-buffer-query-functions' nor 'kill-buffer-hook' are run
> +for buffers created by 'generate-new-buffer' with the second argument
> +'inhibit-buffer-hooks' non-nil.

Quoting again.

> @@ -6268,9 +6334,9 @@ The function `kill-all-local-variables' runs this before doing anything else.  *
>  	       doc: /* Hook run when the buffer list changes.
>  Functions (implicitly) running this hook are `get-buffer-create',
>  `make-indirect-buffer', `rename-buffer', `kill-buffer', `bury-buffer'
> -and `select-window'.  Functions run by this hook should avoid calling
> -`select-window' with a nil NORECORD argument or `with-temp-buffer'
> -since either may lead to infinite recursion.  */);
> +and `select-window'.  This hook is not run for buffers created by
> +'generate-new-buffer' with the second argument 'inhibit-buffer-hooks'
> +non-nil.  */);

And here.