Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: Robert Pluim <rpluim@gmail.com>, emacs-devel@gnu.org,
>  55257-submitter@debbugs.gnu.org
> Date: Thu, 05 May 2022 15:07:41 -0700
> 
> -(eval-when-compile (require 'cl-lib))
> +(eval-when-compile
> +  (require 'cl-lib)
> +  (require 'subr-x))

Why did you need subr-x here?  AFAIR, doing this breaks bootstrap,
which is why if-let is now in subr.el.

> +(defun get-initial-buffer-create ()
> +  "Return the \*scratch\* buffer, creating a new one if needed."
> +  (if-let ((scratch (get-buffer "*scratch*")))
> +      scratch
> +    (prog1 (setq scratch (get-buffer-create "*scratch*"))
> +      (with-current-buffer scratch
> +        (when initial-scratch-message
> +          (insert (substitute-command-keys initial-scratch-message))
> +          (set-buffer-modified-p nil))
> +        (funcall initial-major-mode)))))

It's somewhat inelegant to explicitly test for the buffer's existence
before you call get-buffer-create.  Is that only to avoid changing its
contents?  If so, can't you test for that in some other way?

> +    return call0 (intern ("get-initial-buffer-create"));

Instead of calling intern each time this function is called from C, it
is better to define a symbol for it, usually named
Qget_initial_buffer_create, and then call0 it directly.

>  /* The following function is a safe variant of Fother_buffer: It doesn't
> @@ -1659,15 +1650,7 @@ other_buffer_safely (Lisp_Object buffer)
>      if (candidate_buffer (buf, buffer))
>        return buf;
>  
> -  AUTO_STRING (scratch, "*scratch*");
> -  buf = Fget_buffer (scratch);
> -  if (NILP (buf))
> -    {
> -      buf = Fget_buffer_create (scratch, Qnil);
> -      Fset_buffer_major_mode (buf);
> -    }
> -
> -  return buf;
> +  return call0 (intern ("get-initial-buffer-create"));

get-initial-buffer-create shows the initial-scratch-message, something
the C code you are replacing didn't do.  This is a change in behavior
that should at least be documented, if not fixed.

I also wonder whether we should use safe_call in these places.

Thanks.