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

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

[Stefan Monnier]

>     * lisp/server.el: Require subr-x when compiling.
>     (server-execute): Initialize the *scratch* buffer in the same way that
>     the scratch-buffer command does, for consistency.

Could we put that code in a function so we don't need to duplicate it
(and sync it)?


        Stefan

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

[Sean Whitton]

Hello,

On Tue 03 May 2022 at 10:14pm -04, Stefan Monnier wrote:

>>     * lisp/server.el: Require subr-x when compiling.
>>     (server-execute): Initialize the *scratch* buffer in the same way that
>>     the scratch-buffer command does, for consistency.
>
> Could we put that code in a function so we don't need to duplicate it
> (and sync it)?

There's a similar thing in a lot of different places in C, too.

My own inclination is that it's simple, and in fact slightly different
in different places, so not worth factoring out, but YMMV.

-- 
Sean Whitton

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

[Stefan Monnier]

>> Could we put that code in a function so we don't need to duplicate it
>> (and sync it)?
> There's a similar thing in a lot of different places in C, too.
> My own inclination is that it's simple, and in fact slightly different
> in different places, so not worth factoring out, but YMMV.

If you could factor out the commonality (and remove the gratuitous
differences along the way), I'd be grateful.
In my experience, it's always worth it in the long run (and it's
generally an opportunity to improve the code, because once it only
exists at a single place, it's much easier to add functionality and
customizability to it).


        Stefan

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

[Sean Whitton]

Hello,

On Wed 04 May 2022 at 08:47am -04, Stefan Monnier wrote:

>>> Could we put that code in a function so we don't need to duplicate it
>>> (and sync it)?
>> There's a similar thing in a lot of different places in C, too.
>> My own inclination is that it's simple, and in fact slightly different
>> in different places, so not worth factoring out, but YMMV.
>
> If you could factor out the commonality (and remove the gratuitous
> differences along the way), I'd be grateful.
> In my experience, it's always worth it in the long run (and it's
> generally an opportunity to improve the code, because once it only
> exists at a single place, it's much easier to add functionality and
> customizability to it).

In order to factor it out of the C as well I need to define it in C with
the DEFUN macro, right?  Otherwise each C callsite would have to use
Ffuncall, which doesn't seem right.

-- 
Sean Whitton

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

[Robert Pluim]

>>>>> On Wed, 04 May 2022 07:23:32 -0700, Sean Whitton <spwhitton@spwhitton.name> said:
    Sean> In order to factor it out of the C as well I need to define it in C with
    Sean> the DEFUN macro, right?  Otherwise each C callsite would have to use
    Sean> Ffuncall, which doesn't seem right.

You'll need to weigh the cost of implementing it in C against the need
to use Ffuncall. For something like this thatʼs not going to be called
a lot Iʼd be strongly tempted to choose the easiest implementation
language, ie elisp.

Robert
-- 

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

[Stefan Monnier]

>> If you could factor out the commonality (and remove the gratuitous
>> differences along the way), I'd be grateful.
>> In my experience, it's always worth it in the long run (and it's
>> generally an opportunity to improve the code, because once it only
>> exists at a single place, it's much easier to add functionality and
>> customizability to it).
> In order to factor it out of the C as well I need to define it in C with
> the DEFUN macro, right?  Otherwise each C callsite would have to use
> Ffuncall, which doesn't seem right.

Yup.  Using Ffuncall (of `call1`, call2`, ...) is not in itself
a problem, especially for code like this which is not
performance critical.  But I suspect that using a DEFUN might be a better
option here for bootstrap reasons (if the code needs to be run before
things like `subr.el` have been loaded).


        Stefan

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

[Sean Whitton]

Hello,

On Wed 04 May 2022 at 04:34pm +02, Robert Pluim wrote:

>>>>>> On Wed, 04 May 2022 07:23:32 -0700, Sean Whitton <spwhitton@spwhitton.name> said:
>     Sean> In order to factor it out of the C as well I need to define it in C with
>     Sean> the DEFUN macro, right?  Otherwise each C callsite would have to use
>     Sean> Ffuncall, which doesn't seem right.
>
> You'll need to weigh the cost of implementing it in C against the need
> to use Ffuncall. For something like this thatʼs not going to be called
> a lot Iʼd be strongly tempted to choose the easiest implementation
> language, ie elisp.

The C thing is in other-buffer and other-buffer-safely -- I think those
get called quite often?

-- 
Sean Whitton

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

[Robert Pluim]

>>>>> On Wed, 04 May 2022 07:46:11 -0700, Sean Whitton <spwhitton@spwhitton.name> said:

    Sean> Hello,
    Sean> On Wed 04 May 2022 at 04:34pm +02, Robert Pluim wrote:

    >>>>>>> On Wed, 04 May 2022 07:23:32 -0700, Sean Whitton <spwhitton@spwhitton.name> said:
    Sean> In order to factor it out of the C as well I need to define it in C with
    Sean> the DEFUN macro, right?  Otherwise each C callsite would have to use
    Sean> Ffuncall, which doesn't seem right.
    >> 
    >> You'll need to weigh the cost of implementing it in C against the need
    >> to use Ffuncall. For something like this thatʼs not going to be called
    >> a lot Iʼd be strongly tempted to choose the easiest implementation
    >> language, ie elisp.

    Sean> The C thing is in other-buffer and other-buffer-safely -- I think those
    Sean> get called quite often?

Probably. And it looks like itʼs only 6 lines or so of code, so maybe
you can put those in a DEFUN.

Robert
-- 

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

[Sean Whitton]

Hello,

On Wed 04 May 2022 at 10:42am -04, Stefan Monnier wrote:

>>> If you could factor out the commonality (and remove the gratuitous
>>> differences along the way), I'd be grateful.
>>> In my experience, it's always worth it in the long run (and it's
>>> generally an opportunity to improve the code, because once it only
>>> exists at a single place, it's much easier to add functionality and
>>> customizability to it).
>> In order to factor it out of the C as well I need to define it in C with
>> the DEFUN macro, right?  Otherwise each C callsite would have to use
>> Ffuncall, which doesn't seem right.
>
> Yup.  Using Ffuncall (of `call1`, call2`, ...) is not in itself
> a problem, especially for code like this which is not
> performance critical.  But I suspect that using a DEFUN might be a better
> option here for bootstrap reasons (if the code needs to be run before
> things like `subr.el` have been loaded).

In order to make all these cases consistent, my new code will (insert
(substitute-command-keys initial-scratch-message)), so it's going to
require that both help.el and startup.el have been loaded.  So I guess
it should be in Lisp.

It looks like Fother_window is called only from Fcall_interactively and
Fkill_buffer, so there probably isn't a bootstrapping issue if I make
those Ffuncall my new `get-initial-buffer-create'.  It looks like
bootstrapping C code just makes an empty *scratch* and leaves it to
startup.el to initialise it.

-- 
Sean Whitton

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

[Stefan Monnier]

> It looks like Fother_window is called only from Fcall_interactively and
> Fkill_buffer, so there probably isn't a bootstrapping issue if I make
> those Ffuncall my new `get-initial-buffer-create'.  It looks like
> bootstrapping C code just makes an empty *scratch* and leaves it to
> startup.el to initialise it.

Even better,


        Stefan

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

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

Hello,

On Wed 04 May 2022 at 12:26PM -04, Stefan Monnier wrote:

>> It looks like Fother_window is called only from Fcall_interactively and
>> Fkill_buffer, so there probably isn't a bootstrapping issue if I make
>> those Ffuncall my new `get-initial-buffer-create'.  It looks like
>> bootstrapping C code just makes an empty *scratch* and leaves it to
>> startup.el to initialise it.
>
> Even better,

Here's my fix.  Haven't quite finished testing each and every call site
but seemed worth posting it for comments.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Factor-out-scratch-initialization.patch --]
[-- Type: text/x-patch, Size: 6268 bytes --]

From ac1c813ccd4cc266dd722704a36a9d178dab1e4c Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Thu, 5 May 2022 13:03:06 -0700
Subject: [PATCH] Factor out *scratch* initialization

* lisp/simple.el: Require subr-x when compiling.
(get-initial-buffer-create): New function, factored out of
scratch-buffer, and additionally clearing the modification flag and
calling substitute-command-keys (bug#55257).
(scratch-buffer):
lisp/server.el (server-execute):
lisp/startup.el (normal-no-mouse-startup-screen):
(command-line-1):
lisp/window.el (last-buffer):
src/buffer.c (Fother_buffer, other_buffer_safely): Use it.
lisp/startup.el (startup--get-buffer-create-scratch): Delete
now-unused function.
---
 lisp/server.el  |  2 +-
 lisp/simple.el  | 22 +++++++++++++++-------
 lisp/startup.el | 12 +++---------
 lisp/window.el  |  5 +----
 src/buffer.c    | 21 ++-------------------
 5 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 763cf27f7a..042962b8e9 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1367,7 +1367,7 @@ server-execute
 			 ((functionp initial-buffer-choice)
 			  (funcall initial-buffer-choice)))))
 	      (switch-to-buffer
-	       (if (buffer-live-p buf) buf (get-buffer-create "*scratch*"))
+	       (if (buffer-live-p buf) buf (get-initial-buffer-create))
 	       'norecord)))
 
           ;; Delete the client if necessary.
diff --git a/lisp/simple.el b/lisp/simple.el
index 861d9eefde..5a37e246f7 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -28,7 +28,9 @@
 
 ;;; Code:
 
-(eval-when-compile (require 'cl-lib))
+(eval-when-compile
+  (require 'cl-lib)
+  (require 'subr-x))
 
 (declare-function widget-convert "wid-edit" (type &rest args))
 (declare-function shell-mode "shell" ())
@@ -10213,16 +10215,22 @@ capitalize-dwim
 the number of seconds east of Greenwich.")
   )
 
+(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)))))
+
 (defun scratch-buffer ()
   "Switch to the \*scratch\* buffer.
 If the buffer doesn't exist, create it first."
   (interactive)
-  (if (get-buffer "*scratch*")
-      (pop-to-buffer-same-window "*scratch*")
-    (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
-    (when initial-scratch-message
-      (insert initial-scratch-message))
-    (funcall initial-major-mode)))
+  (pop-to-buffer-same-window (get-initial-buffer-create)))
 
 \f
 
diff --git a/lisp/startup.el b/lisp/startup.el
index c7cf86a01e..3fa25ddee9 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -2355,7 +2355,7 @@ normal-no-mouse-startup-screen
   (insert "\t\t")
   (insert-button "Open *scratch* buffer"
 		 'action (lambda (_button) (switch-to-buffer
-                                       (startup--get-buffer-create-scratch)))
+                                       (get-initial-buffer-create)))
 		 'follow-link t)
   (insert "\n")
   (save-restriction
@@ -2487,12 +2487,6 @@ display-about-screen
 (defalias 'about-emacs 'display-about-screen)
 (defalias 'display-splash-screen 'display-startup-screen)
 
-(defun startup--get-buffer-create-scratch ()
-  (or (get-buffer "*scratch*")
-      (with-current-buffer (get-buffer-create "*scratch*")
-        (set-buffer-major-mode (current-buffer))
-        (current-buffer))))
-
 ;; This avoids byte-compiler warning in the unexec build.
 (declare-function pdumper-stats "pdumper.c" ())
 
@@ -2784,7 +2778,7 @@ command-line-1
     (when (eq initial-buffer-choice t)
       ;; When `initial-buffer-choice' equals t make sure that *scratch*
       ;; exists.
-      (startup--get-buffer-create-scratch))
+      (get-initial-buffer-create))
 
     ;; If *scratch* exists and is empty, insert initial-scratch-message.
     ;; Do this before switching to *scratch* below to handle bug#9605.
@@ -2808,7 +2802,7 @@ command-line-1
 		   ((functionp initial-buffer-choice)
 		    (funcall initial-buffer-choice))
                    ((eq initial-buffer-choice t)
-                    (startup--get-buffer-create-scratch))
+                    (get-initial-buffer-create))
                    (t
                     (error "`initial-buffer-choice' must be a string, a function, or t")))))
         (unless (buffer-live-p buf)
diff --git a/lisp/window.el b/lisp/window.el
index 9f78784612..4ec329e0cf 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4886,10 +4886,7 @@ last-buffer
   (setq frame (or frame (selected-frame)))
   (or (get-next-valid-buffer (nreverse (buffer-list frame))
  			     buffer visible-ok frame)
-      (get-buffer "*scratch*")
-      (let ((scratch (get-buffer-create "*scratch*")))
-	(set-buffer-major-mode scratch)
-	scratch)))
+      (get-initial-buffer-create)))
 
 (defcustom frame-auto-hide-function #'iconify-frame
   "Function called to automatically hide frames.
diff --git a/src/buffer.c b/src/buffer.c
index f8a7a4f510..702b21f9fc 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1634,16 +1634,7 @@ DEFUN ("other-buffer", Fother_buffer, Sother_buffer, 0, 3, 0,
   if (!NILP (notsogood))
     return notsogood;
   else
-    {
-      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"));
 }
 
 /* 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"));
 }
 \f
 DEFUN ("buffer-enable-undo", Fbuffer_enable_undo, Sbuffer_enable_undo,
-- 
2.30.2

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

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

Hello,

On Thu 05 May 2022 at 03:07PM -07, Sean Whitton wrote:

> Hello,
>
> On Wed 04 May 2022 at 12:26PM -04, Stefan Monnier wrote:
>
>>> It looks like Fother_window is called only from Fcall_interactively and
>>> Fkill_buffer, so there probably isn't a bootstrapping issue if I make
>>> those Ffuncall my new `get-initial-buffer-create'.  It looks like
>>> bootstrapping C code just makes an empty *scratch* and leaves it to
>>> startup.el to initialise it.
>>
>> Even better,
>
> Here's my fix.  Haven't quite finished testing each and every call site
> but seemed worth posting it for comments.

Hopefully fixed the commit message in the attached.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Factor-out-scratch-initialization.patch --]
[-- Type: text/x-patch, Size: 6276 bytes --]

From 9731fa66ff5b6e2ed3287ef033b71c584abadd12 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Thu, 5 May 2022 13:03:06 -0700
Subject: [PATCH] Factor out *scratch* initialization

* lisp/simple.el: Require subr-x when compiling.
(get-initial-buffer-create): New function, factored out of
scratch-buffer, and additionally clearing the modification flag and
calling substitute-command-keys (bug#55257).
(scratch-buffer):
* lisp/server.el (server-execute):
* lisp/startup.el (normal-no-mouse-startup-screen, command-line-1):
* lisp/window.el (last-buffer):
* src/buffer.c (Fother_buffer, other_buffer_safely): Use it.
* lisp/startup.el (startup--get-buffer-create-scratch): Delete
now-unused function.
---
 lisp/server.el  |  2 +-
 lisp/simple.el  | 22 +++++++++++++++-------
 lisp/startup.el | 12 +++---------
 lisp/window.el  |  5 +----
 src/buffer.c    | 21 ++-------------------
 5 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 763cf27f7a..042962b8e9 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1367,7 +1367,7 @@ server-execute
 			 ((functionp initial-buffer-choice)
 			  (funcall initial-buffer-choice)))))
 	      (switch-to-buffer
-	       (if (buffer-live-p buf) buf (get-buffer-create "*scratch*"))
+	       (if (buffer-live-p buf) buf (get-initial-buffer-create))
 	       'norecord)))
 
           ;; Delete the client if necessary.
diff --git a/lisp/simple.el b/lisp/simple.el
index 861d9eefde..5a37e246f7 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -28,7 +28,9 @@
 
 ;;; Code:
 
-(eval-when-compile (require 'cl-lib))
+(eval-when-compile
+  (require 'cl-lib)
+  (require 'subr-x))
 
 (declare-function widget-convert "wid-edit" (type &rest args))
 (declare-function shell-mode "shell" ())
@@ -10213,16 +10215,22 @@ capitalize-dwim
 the number of seconds east of Greenwich.")
   )
 
+(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)))))
+
 (defun scratch-buffer ()
   "Switch to the \*scratch\* buffer.
 If the buffer doesn't exist, create it first."
   (interactive)
-  (if (get-buffer "*scratch*")
-      (pop-to-buffer-same-window "*scratch*")
-    (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
-    (when initial-scratch-message
-      (insert initial-scratch-message))
-    (funcall initial-major-mode)))
+  (pop-to-buffer-same-window (get-initial-buffer-create)))
 
 \f
 
diff --git a/lisp/startup.el b/lisp/startup.el
index c7cf86a01e..3fa25ddee9 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -2355,7 +2355,7 @@ normal-no-mouse-startup-screen
   (insert "\t\t")
   (insert-button "Open *scratch* buffer"
 		 'action (lambda (_button) (switch-to-buffer
-                                       (startup--get-buffer-create-scratch)))
+                                       (get-initial-buffer-create)))
 		 'follow-link t)
   (insert "\n")
   (save-restriction
@@ -2487,12 +2487,6 @@ display-about-screen
 (defalias 'about-emacs 'display-about-screen)
 (defalias 'display-splash-screen 'display-startup-screen)
 
-(defun startup--get-buffer-create-scratch ()
-  (or (get-buffer "*scratch*")
-      (with-current-buffer (get-buffer-create "*scratch*")
-        (set-buffer-major-mode (current-buffer))
-        (current-buffer))))
-
 ;; This avoids byte-compiler warning in the unexec build.
 (declare-function pdumper-stats "pdumper.c" ())
 
@@ -2784,7 +2778,7 @@ command-line-1
     (when (eq initial-buffer-choice t)
       ;; When `initial-buffer-choice' equals t make sure that *scratch*
       ;; exists.
-      (startup--get-buffer-create-scratch))
+      (get-initial-buffer-create))
 
     ;; If *scratch* exists and is empty, insert initial-scratch-message.
     ;; Do this before switching to *scratch* below to handle bug#9605.
@@ -2808,7 +2802,7 @@ command-line-1
 		   ((functionp initial-buffer-choice)
 		    (funcall initial-buffer-choice))
                    ((eq initial-buffer-choice t)
-                    (startup--get-buffer-create-scratch))
+                    (get-initial-buffer-create))
                    (t
                     (error "`initial-buffer-choice' must be a string, a function, or t")))))
         (unless (buffer-live-p buf)
diff --git a/lisp/window.el b/lisp/window.el
index 9f78784612..4ec329e0cf 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4886,10 +4886,7 @@ last-buffer
   (setq frame (or frame (selected-frame)))
   (or (get-next-valid-buffer (nreverse (buffer-list frame))
  			     buffer visible-ok frame)
-      (get-buffer "*scratch*")
-      (let ((scratch (get-buffer-create "*scratch*")))
-	(set-buffer-major-mode scratch)
-	scratch)))
+      (get-initial-buffer-create)))
 
 (defcustom frame-auto-hide-function #'iconify-frame
   "Function called to automatically hide frames.
diff --git a/src/buffer.c b/src/buffer.c
index f8a7a4f510..702b21f9fc 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1634,16 +1634,7 @@ DEFUN ("other-buffer", Fother_buffer, Sother_buffer, 0, 3, 0,
   if (!NILP (notsogood))
     return notsogood;
   else
-    {
-      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"));
 }
 
 /* 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"));
 }
 \f
 DEFUN ("buffer-enable-undo", Fbuffer_enable_undo, Sbuffer_enable_undo,
-- 
2.30.2

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.

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

[Juri Linkov]

> Here's my fix.  Haven't quite finished testing each and every call site
> but seemed worth posting it for comments.

As found in https://debbugs.gnu.org/9054#295
there is another place that needs get-initial-buffer-create
in switch-to-buffer:

  if (strcmp (SSDATA (BVAR (XBUFFER (buffer), name)), "*scratch*") == 0)
    function = find_symbol_value (intern ("initial-major-mode"));

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

[Stefan Monnier]

> +  (if-let ((scratch (get-buffer "*scratch*")))
> +      scratch

A.k.a

     (or (get-buffer "*scratch*")

>  (defun scratch-buffer ()
>    "Switch to the \*scratch\* buffer.
>  If the buffer doesn't exist, create it first."
>    (interactive)
> -  (if (get-buffer "*scratch*")
> -      (pop-to-buffer-same-window "*scratch*")
> -    (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
> -    (when initial-scratch-message
> -      (insert initial-scratch-message))
> -    (funcall initial-major-mode)))
> +  (pop-to-buffer-same-window (get-initial-buffer-create)))

I think the new function can be considered "internal", and I think it
would be better for its name to use "scratch-buffer" as a prefix, so
maybe `scratch-buffer--create`?

Other than that, it looks good to me, thank you very much.


        Stefan

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

[Sean Whitton]

Hello Stefan,

On Fri 06 May 2022 at 07:34am -04, Stefan Monnier wrote:

>> +  (if-let ((scratch (get-buffer "*scratch*")))
>> +      scratch
>
> A.k.a
>
>      (or (get-buffer "*scratch*")

Ah yes.

>>  (defun scratch-buffer ()
>>    "Switch to the \*scratch\* buffer.
>>  If the buffer doesn't exist, create it first."
>>    (interactive)
>> -  (if (get-buffer "*scratch*")
>> -      (pop-to-buffer-same-window "*scratch*")
>> -    (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
>> -    (when initial-scratch-message
>> -      (insert initial-scratch-message))
>> -    (funcall initial-major-mode)))
>> +  (pop-to-buffer-same-window (get-initial-buffer-create)))
>
> I think the new function can be considered "internal", and I think it
> would be better for its name to use "scratch-buffer" as a prefix, so
> maybe `scratch-buffer--create`?

It gets called in places like server.el, though.  And I can imagine
wanting to use it in third party code as a better version of
(get-buffer-create "*scratch"*).  So it seems to me not to be internal.

-- 
Sean Whitton

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

[Lars Ingebrigtsen]

Sean Whitton <spwhitton@spwhitton.name> writes:

> It gets called in places like server.el, though.  And I can imagine
> wanting to use it in third party code as a better version of
> (get-buffer-create "*scratch"*).  So it seems to me not to be internal.

scratch-buffer isn't meant as an internal function, but a command users
can use to recreate the *scratch* buffer.  (It's a question that comes
up from users once in a while.)

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

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

[Sean Whitton]

Hello,

On Fri 06 May 2022 at 08:40am +03, Eli Zaretskii wrote:

>> 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.

Ah, my mistake, I didn't know it had moved (though I'm going to get rid
of the if-let I think).

>> +(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?

I had the same intuition at first, but I don't think there is another
way -- the code wants to touch the buffer at all only if it wasn't
already there.  And the code path where it already exists will be by far
the most commonly called, so it seems best to avoid calling
with-current-buffer if we don't have to.

>> +    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.

Will do.

>>  /* 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.

This is deliberate -- to my mind I'm fixing the same bug as the one in
server.el.  other-buffer recreates *scratch* for the same sort of
reasons that 'emacsclient -nc' does.

Where were you thinking it should be documented?  The Emacs Lisp changes
section of NEWS?

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

Could you say more?

-- 
Sean Whitton

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

[Sean Whitton]

Hello,

On Fri 06 May 2022 at 10:41am +03, Juri Linkov wrote:

>> Here's my fix.  Haven't quite finished testing each and every call site
>> but seemed worth posting it for comments.
>
> As found in https://debbugs.gnu.org/9054#295
> there is another place that needs get-initial-buffer-create
> in switch-to-buffer:
>
>   if (strcmp (SSDATA (BVAR (XBUFFER (buffer), name)), "*scratch*") == 0)
>     function = find_symbol_value (intern ("initial-major-mode"));

Yes I think you're right.  I'll add that to my patch, thanks.

-- 
Sean Whitton

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

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: monnier@iro.umontreal.ca, rpluim@gmail.com, emacs-devel@gnu.org,
>  55257-submitter@debbugs.gnu.org
> Date: Fri, 06 May 2022 12:26:46 -0700
> 
> >> +(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?
> 
> I had the same intuition at first, but I don't think there is another
> way -- the code wants to touch the buffer at all only if it wasn't
> already there.

What do you mean by "touch"?  Doesn't get-buffer already "touch" the
buffer if it exists?  And determining whether the buffer has any stuff
in it (if this is the concern here) is just one function call away,
and is very fast.

> And the code path where it already exists will be by far
> the most commonly called, so it seems best to avoid calling
> with-current-buffer if we don't have to.

But get-buffer-create already does all that internally, and it exists
for this very purpose.  I don't really understand the objections, to
tell the truth.  Unless some more fundamental problem is involved,
which is why I asked about the reasons.

> >> +  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.
> 
> This is deliberate -- to my mind I'm fixing the same bug as the one in
> server.el.  other-buffer recreates *scratch* for the same sort of
> reasons that 'emacsclient -nc' does.
> 
> Where were you thinking it should be documented?  The Emacs Lisp changes
> section of NEWS?

This is not about Emacs Lisp, this is an incompatible behavior change,
and we have a section for that in NEWS.

> > I also wonder whether we should use safe_call in these places.
> 
> Could you say more?

My bother is that the function you call could signal an error at some
point, and that could cause trouble to some of the callers, perhaps.
Calling Lisp from C should always assume this could happen, because
basically the Lisp function you call is out of your control, and you
cannot reliably assume anything about what it does or will do at some
future time.

Does this answer your question?

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

[Stefan Monnier]

>> I had the same intuition at first, but I don't think there is another
>> way -- the code wants to touch the buffer at all only if it wasn't
>> already there.
> What do you mean by "touch"?

Modify the buffer in any way (change its content or some of its
buffer-local variables (e.g. the major mode)).

His code just reproduces the existing code's behavior, AFAICT.

> Doesn't get-buffer already "touch" the buffer if it exists?
> And determining whether the buffer has any stuff in it (if this is the
> concern here) is just one function call away, and is very fast.

Not sure what it is we'd be gaining.

The code is just trying to avoid modifying the buffer in any way (since
that would likely lose information or undo something the user did,
without its explicit request).

> But get-buffer-create already does all that internally, and it exists
> for this very purpose.  I don't really understand the objections, to
> tell the truth.  Unless some more fundamental problem is involved,
> which is why I asked about the reasons.

Indeed `get-buffer-create` begins by calling `get-buffer` so there's
redundancy at run-time.  But we don't export any `create-buffer`
function which presumes that there is no buffer by that name, so when
we want to create a buffer named *scratch* and we know there is no such
buffer yet, we still have to call `get-buffer-create` :-(

Since we want to preserve the invariant that there can't be two buffers
with the same name, we don't have much choice in this matter (we
couldn't offer a `create-buffer` and just trust users to only call it
when it's safe, tho we could do that in the C code that's not exported
to ELisp).

> My bother is that the function you call could signal an error at some
> point, and that could cause trouble to some of the callers, perhaps.
> Calling Lisp from C should always assume this could happen, because
> basically the Lisp function you call is out of your control, and you
> cannot reliably assume anything about what it does or will do at some
> future time.

I think this is not needed here, or at least we haven't needed it so
far: the old C code called `Fset_buffer_major_mode` which itself
calls `call0 (function);` where `function` is the `initial-major-mode`.


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Sean Whitton <spwhitton@spwhitton.name>,  rpluim@gmail.com,
>   emacs-devel@gnu.org,  55257-submitter@debbugs.gnu.org
> Date: Sat, 07 May 2022 09:51:54 -0400
> 
> > Doesn't get-buffer already "touch" the buffer if it exists?
> > And determining whether the buffer has any stuff in it (if this is the
> > concern here) is just one function call away, and is very fast.
> 
> Not sure what it is we'd be gaining.

We will gain that I won't raise my brow and won't want to change that
code each time my eyes fall on it.

> > My bother is that the function you call could signal an error at some
> > point, and that could cause trouble to some of the callers, perhaps.
> > Calling Lisp from C should always assume this could happen, because
> > basically the Lisp function you call is out of your control, and you
> > cannot reliably assume anything about what it does or will do at some
> > future time.
> 
> I think this is not needed here, or at least we haven't needed it so
> far: the old C code called `Fset_buffer_major_mode` which itself
> calls `call0 (function);` where `function` is the `initial-major-mode`.

We have recently learned that these calls are not safe enough, so I
think using safe_call's family here will be more future-proof, as more
and more code is moved to Lisp and more and more hooks are bing added.

So I'm still unconvinced, and would like this code to be safer.

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

[Sean Whitton]

Hello,

On Sat 07 May 2022 at 08:30am +03, Eli Zaretskii wrote:

> What do you mean by "touch"?  Doesn't get-buffer already "touch" the
> buffer if it exists?  And determining whether the buffer has any stuff
> in it (if this is the concern here) is just one function call away,
> and is very fast.

As Stefan said, if the user has deleted the initial scratch message, we
do not want to go reinserting it.  And similarly if they have changed
the mode away from whatever initial-major-mode specifies, we do not want
to change it back.  The only time we want to do anything is when the
buffer does not exist.

> This is not about Emacs Lisp, this is an incompatible behavior change,
> and we have a section for that in NEWS.

Okay.

> My bother is that the function you call could signal an error at some
> point, and that could cause trouble to some of the callers, perhaps.
> Calling Lisp from C should always assume this could happen, because
> basically the Lisp function you call is out of your control, and you
> cannot reliably assume anything about what it does or will do at some
> future time.
>
> Does this answer your question?

I'm still not clear on what safe_call protects us from here.  It's in
xdisp.c and the comment next to it talks about preventing redisplay, but
I don't see how redisplay is relevant to what this function does.  Does
safe_call also catch all signals and convert them to nil, or something?

-- 
Sean Whitton

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

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: monnier@iro.umontreal.ca, rpluim@gmail.com, emacs-devel@gnu.org,
>  55257-submitter@debbugs.gnu.org
> Date: Sat, 07 May 2022 09:29:50 -0700
> 
> I'm still not clear on what safe_call protects us from here.

It catches any errors and doesn't let the control flow escape to
top-level.

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

[Sean Whitton]

Hello,

On Sat 07 May 2022 at 07:41pm +03, Eli Zaretskii wrote:

>> From: Sean Whitton <spwhitton@spwhitton.name>
>> Cc: monnier@iro.umontreal.ca, rpluim@gmail.com, emacs-devel@gnu.org,
>>  55257-submitter@debbugs.gnu.org
>> Date: Sat, 07 May 2022 09:29:50 -0700
>>
>> I'm still not clear on what safe_call protects us from here.
>
> It catches any errors and doesn't let the control flow escape to
> top-level.

Okay, thanks.

-- 
Sean Whitton

bug#55257: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

Hello,

On Thu 05 May 2022 at 03:07PM -07, Sean Whitton wrote:

> Hello,
>
> On Wed 04 May 2022 at 12:26PM -04, Stefan Monnier wrote:
>
>>> It looks like Fother_window is called only from Fcall_interactively and
>>> Fkill_buffer, so there probably isn't a bootstrapping issue if I make
>>> those Ffuncall my new `get-initial-buffer-create'.  It looks like
>>> bootstrapping C code just makes an empty *scratch* and leaves it to
>>> startup.el to initialise it.
>>
>> Even better,
>
> Here's my fix.  Haven't quite finished testing each and every call site
> but seemed worth posting it for comments.

Here's an updated patch.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Factor-out-scratch-initialization.patch --]
[-- Type: text/x-patch, Size: 9132 bytes --]

From 4b80914abeb67065b22eb0a491cf684a5b1eff71 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Thu, 5 May 2022 13:03:06 -0700
Subject: [PATCH v2] Factor out *scratch* initialization

* lisp/simple.el (get-initial-buffer-create): New function, factored
out of scratch-buffer, and additionally clearing the modification flag
and calling substitute-command-keys (bug#55257).
(scratch-buffer):
* lisp/server.el (server-execute):
* lisp/startup.el (normal-no-mouse-startup-screen, command-line-1):
* lisp/window.el (last-buffer, window-normalize-buffer-to-switch-to):
* src/buffer.c (Fother_buffer, other_buffer_safely): Use it.
(syms_of_buffer): Add Qget_initial_buffer_create.
* lisp/startup.el (startup--get-buffer-create-scratch): Delete
now-unused function.
* doc/lispref/os.texi (Summary: Sequence of Actions at Startup):
* NEWS (Incompatible changes in Emacs 29.1): Document the change.
---
 doc/lispref/os.texi |  8 ++++----
 etc/NEWS            |  8 ++++++++
 lisp/server.el      |  2 +-
 lisp/simple.el      | 20 ++++++++++++++------
 lisp/startup.el     | 12 +++---------
 lisp/window.el      | 18 ++++++++----------
 src/buffer.c        | 22 +++-------------------
 7 files changed, 41 insertions(+), 49 deletions(-)

diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index 9df708532d..f4dd2e7072 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -329,10 +329,10 @@ Startup Summary
 @end defopt
 
 @defopt initial-scratch-message
-This variable, if non-@code{nil}, should be a string, which is
-treated as documentation to be
-inserted into the @file{*scratch*} buffer when Emacs starts up.  If it
-is @code{nil}, the @file{*scratch*} buffer is empty.
+This variable, if non-@code{nil}, should be a string, which is treated
+as documentation to be inserted into the @file{*scratch*} buffer when
+Emacs starts up or when that buffer is recreated.  If it is
+@code{nil}, the @file{*scratch*} buffer is empty.
 @end defopt
 
 @noindent
diff --git a/etc/NEWS b/etc/NEWS
index 6637eda00c..20ef8f7b52 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -238,6 +238,14 @@ encouraged to test timestamp-related code with this variable set to
 nil, as it will default to nil in a future Emacs version and will be
 removed some time after that.
 
++++
+** Functions which recreate the *scratch* buffer now also initialize it.
+When functions like 'other-buffer' and 'server-execute' recreate
+*scratch*, they now also insert 'initial-scratch-message' and change
+the major mode according to 'initial-major-mode', like at Emacs
+startup.  Previously, these functions ignored
+'initial-scratch-message' and left *scratch* in 'fundamental-mode'.
+
 \f
 * Changes in Emacs 29.1
 
diff --git a/lisp/server.el b/lisp/server.el
index 763cf27f7a..042962b8e9 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1367,7 +1367,7 @@ server-execute
 			 ((functionp initial-buffer-choice)
 			  (funcall initial-buffer-choice)))))
 	      (switch-to-buffer
-	       (if (buffer-live-p buf) buf (get-buffer-create "*scratch*"))
+	       (if (buffer-live-p buf) buf (get-initial-buffer-create))
 	       'norecord)))
 
           ;; Delete the client if necessary.
diff --git a/lisp/simple.el b/lisp/simple.el
index 861d9eefde..1ed82b0a48 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10213,16 +10213,24 @@ capitalize-dwim
 the number of seconds east of Greenwich.")
   )
 
+(defun get-initial-buffer-create ()
+  "Return the \*scratch\* buffer, creating a new one if needed."
+  (or (get-buffer "*scratch*")
+      (let ((scratch (get-buffer-create "*scratch*")))
+        ;; Don't touch the buffer contents or mode unless we know that
+        ;; we just created it.
+        (with-current-buffer scratch
+          (when initial-scratch-message
+            (insert (substitute-command-keys initial-scratch-message))
+            (set-buffer-modified-p nil))
+          (funcall initial-major-mode))
+        scratch)))
+
 (defun scratch-buffer ()
   "Switch to the \*scratch\* buffer.
 If the buffer doesn't exist, create it first."
   (interactive)
-  (if (get-buffer "*scratch*")
-      (pop-to-buffer-same-window "*scratch*")
-    (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
-    (when initial-scratch-message
-      (insert initial-scratch-message))
-    (funcall initial-major-mode)))
+  (pop-to-buffer-same-window (get-initial-buffer-create)))
 
 \f
 
diff --git a/lisp/startup.el b/lisp/startup.el
index c7cf86a01e..3fa25ddee9 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -2355,7 +2355,7 @@ normal-no-mouse-startup-screen
   (insert "\t\t")
   (insert-button "Open *scratch* buffer"
 		 'action (lambda (_button) (switch-to-buffer
-                                       (startup--get-buffer-create-scratch)))
+                                       (get-initial-buffer-create)))
 		 'follow-link t)
   (insert "\n")
   (save-restriction
@@ -2487,12 +2487,6 @@ display-about-screen
 (defalias 'about-emacs 'display-about-screen)
 (defalias 'display-splash-screen 'display-startup-screen)
 
-(defun startup--get-buffer-create-scratch ()
-  (or (get-buffer "*scratch*")
-      (with-current-buffer (get-buffer-create "*scratch*")
-        (set-buffer-major-mode (current-buffer))
-        (current-buffer))))
-
 ;; This avoids byte-compiler warning in the unexec build.
 (declare-function pdumper-stats "pdumper.c" ())
 
@@ -2784,7 +2778,7 @@ command-line-1
     (when (eq initial-buffer-choice t)
       ;; When `initial-buffer-choice' equals t make sure that *scratch*
       ;; exists.
-      (startup--get-buffer-create-scratch))
+      (get-initial-buffer-create))
 
     ;; If *scratch* exists and is empty, insert initial-scratch-message.
     ;; Do this before switching to *scratch* below to handle bug#9605.
@@ -2808,7 +2802,7 @@ command-line-1
 		   ((functionp initial-buffer-choice)
 		    (funcall initial-buffer-choice))
                    ((eq initial-buffer-choice t)
-                    (startup--get-buffer-create-scratch))
+                    (get-initial-buffer-create))
                    (t
                     (error "`initial-buffer-choice' must be a string, a function, or t")))))
         (unless (buffer-live-p buf)
diff --git a/lisp/window.el b/lisp/window.el
index 9f78784612..615c5c078a 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4886,10 +4886,7 @@ last-buffer
   (setq frame (or frame (selected-frame)))
   (or (get-next-valid-buffer (nreverse (buffer-list frame))
  			     buffer visible-ok frame)
-      (get-buffer "*scratch*")
-      (let ((scratch (get-buffer-create "*scratch*")))
-	(set-buffer-major-mode scratch)
-	scratch)))
+      (get-initial-buffer-create)))
 
 (defcustom frame-auto-hide-function #'iconify-frame
   "Function called to automatically hide frames.
@@ -8621,12 +8618,13 @@ window-normalize-buffer-to-switch-to
 `other-buffer'.  Else, if a buffer specified by BUFFER-OR-NAME
 exists, return that buffer.  If no such buffer exists, create a
 buffer with the name BUFFER-OR-NAME and return that buffer."
-  (if buffer-or-name
-      (or (get-buffer buffer-or-name)
-	  (let ((buffer (get-buffer-create buffer-or-name)))
-	    (set-buffer-major-mode buffer)
-	    buffer))
-    (other-buffer)))
+  (pcase buffer-or-name
+    ('nil (other-buffer))
+    ("*scratch*" (get-initial-buffer-create))
+    (_ (or (get-buffer buffer-or-name)
+	   (let ((buffer (get-buffer-create buffer-or-name)))
+	     (set-buffer-major-mode buffer)
+	     buffer)))))
 
 (defcustom switch-to-buffer-preserve-window-point t
   "If non-nil, `switch-to-buffer' tries to preserve `window-point'.
diff --git a/src/buffer.c b/src/buffer.c
index f8a7a4f510..a153ffe78c 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1634,16 +1634,7 @@ DEFUN ("other-buffer", Fother_buffer, Sother_buffer, 0, 3, 0,
   if (!NILP (notsogood))
     return notsogood;
   else
-    {
-      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 safe_call (1, Qget_initial_buffer_create);
 }
 
 /* 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 safe_call (1, Qget_initial_buffer_create);
 }
 \f
 DEFUN ("buffer-enable-undo", Fbuffer_enable_undo, Sbuffer_enable_undo,
@@ -5552,6 +5535,7 @@ syms_of_buffer (void)
   DEFSYM (Qbefore_change_functions, "before-change-functions");
   DEFSYM (Qafter_change_functions, "after-change-functions");
   DEFSYM (Qkill_buffer_query_functions, "kill-buffer-query-functions");
+  DEFSYM (Qget_initial_buffer_create, "get-initial-buffer-create");
 
   DEFSYM (Qvertical_scroll_bar, "vertical-scroll-bar");
   Fput (Qvertical_scroll_bar, Qchoice, list4 (Qnil, Qt, Qleft, Qright));
-- 
2.30.2

bug#55257: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Here's an updated patch.

LGTM, thank you (feel free to push, as far as I'm concerned).
See further comments below.


        Stefan


> +** Functions which recreate the *scratch* buffer now also initialize it.
> +When functions like 'other-buffer' and 'server-execute' recreate
> +*scratch*, they now also insert 'initial-scratch-message' and change
> +the major mode according to 'initial-major-mode', like at Emacs
> +startup.  Previously, these functions ignored
> +'initial-scratch-message' and left *scratch* in 'fundamental-mode'.

I'd say "set the major mode" rather than "change the major mode".

> +(defun get-initial-buffer-create ()

I know you didn't like my `scratch-buffer--create` suggestion because of
the double dash, but I think at least "scratch" would be very welcome in it.

> +  "Return the \*scratch\* buffer, creating a new one if needed."
> +  (or (get-buffer "*scratch*")
> +      (let ((scratch (get-buffer-create "*scratch*")))
> +        ;; Don't touch the buffer contents or mode unless we know that
> +        ;; we just created it.
> +        (with-current-buffer scratch
> +          (when initial-scratch-message
> +            (insert (substitute-command-keys initial-scratch-message))
> +            (set-buffer-modified-p nil))
> +          (funcall initial-major-mode))
> +        scratch)))
> +
>  (defun scratch-buffer ()
>    "Switch to the \*scratch\* buffer.
>  If the buffer doesn't exist, create it first."
>    (interactive)
> -  (if (get-buffer "*scratch*")
> -      (pop-to-buffer-same-window "*scratch*")
> -    (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
> -    (when initial-scratch-message
> -      (insert initial-scratch-message))
> -    (funcall initial-major-mode)))
> +  (pop-to-buffer-same-window (get-initial-buffer-create)))

Now that I look at it again, it occurs to me that maybe we should do
something like:

    (defun scratch-buffer (&optional display)
      "Create the \*scratch\* buffer.
    If the buffer doesn't exist, create it first.
    If DISPLAY (or when used interactively), switch to it."
      (interactive (list t))
      (let ((buf (get-buffer "*scratch*")))
        (unless buf
          ;; Don't touch the buffer contents or mode unless we know that
          ;; we just created it.
          (with-current-buffer (setq buf (get-buffer-create "*scratch*"))
            (when initial-scratch-message
              (insert (substitute-command-keys initial-scratch-message))
              (set-buffer-modified-p nil))
            (funcall initial-major-mode)))
        (when display (pop-to-buffer-same-window buf))
        buf))

i.e. combine the new function with the existing command, so we don't
need to come up with a new name.

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

[Stefan Monnier]

> Here's an updated patch.

LGTM, thank you (feel free to push, as far as I'm concerned).
See further comments below.


        Stefan


> +** Functions which recreate the *scratch* buffer now also initialize it.
> +When functions like 'other-buffer' and 'server-execute' recreate
> +*scratch*, they now also insert 'initial-scratch-message' and change
> +the major mode according to 'initial-major-mode', like at Emacs
> +startup.  Previously, these functions ignored
> +'initial-scratch-message' and left *scratch* in 'fundamental-mode'.

I'd say "set the major mode" rather than "change the major mode".

> +(defun get-initial-buffer-create ()

I know you didn't like my `scratch-buffer--create` suggestion because of
the double dash, but I think at least "scratch" would be very welcome in it.

> +  "Return the \*scratch\* buffer, creating a new one if needed."
> +  (or (get-buffer "*scratch*")
> +      (let ((scratch (get-buffer-create "*scratch*")))
> +        ;; Don't touch the buffer contents or mode unless we know that
> +        ;; we just created it.
> +        (with-current-buffer scratch
> +          (when initial-scratch-message
> +            (insert (substitute-command-keys initial-scratch-message))
> +            (set-buffer-modified-p nil))
> +          (funcall initial-major-mode))
> +        scratch)))
> +
>  (defun scratch-buffer ()
>    "Switch to the \*scratch\* buffer.
>  If the buffer doesn't exist, create it first."
>    (interactive)
> -  (if (get-buffer "*scratch*")
> -      (pop-to-buffer-same-window "*scratch*")
> -    (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
> -    (when initial-scratch-message
> -      (insert initial-scratch-message))
> -    (funcall initial-major-mode)))
> +  (pop-to-buffer-same-window (get-initial-buffer-create)))

Now that I look at it again, it occurs to me that maybe we should do
something like:

    (defun scratch-buffer (&optional display)
      "Create the \*scratch\* buffer.
    If the buffer doesn't exist, create it first.
    If DISPLAY (or when used interactively), switch to it."
      (interactive (list t))
      (let ((buf (get-buffer "*scratch*")))
        (unless buf
          ;; Don't touch the buffer contents or mode unless we know that
          ;; we just created it.
          (with-current-buffer (setq buf (get-buffer-create "*scratch*"))
            (when initial-scratch-message
              (insert (substitute-command-keys initial-scratch-message))
              (set-buffer-modified-p nil))
            (funcall initial-major-mode)))
        (when display (pop-to-buffer-same-window buf))
        buf))

i.e. combine the new function with the existing command, so we don't
need to come up with a new name.

bug#55257: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer

[Sean Whitton]

Hello,

On Mon 09 May 2022 at 02:11pm -04, Stefan Monnier wrote:

> I know you didn't like my `scratch-buffer--create` suggestion because of
> the double dash, but I think at least "scratch" would be very welcome in it.

I've done s/initial/scratch/ which is better because this function
always uses *scratch* rather than, say, looking at initial-buffer-choice.

> Now that I look at it again, it occurs to me that maybe we should do
> something like:
>
>     (defun scratch-buffer (&optional display)
>       "Create the \*scratch\* buffer.
>     If the buffer doesn't exist, create it first.
>     If DISPLAY (or when used interactively), switch to it."
>       (interactive (list t))
>       (let ((buf (get-buffer "*scratch*")))
>         (unless buf
>           ;; Don't touch the buffer contents or mode unless we know that
>           ;; we just created it.
>           (with-current-buffer (setq buf (get-buffer-create "*scratch*"))
>             (when initial-scratch-message
>               (insert (substitute-command-keys initial-scratch-message))
>               (set-buffer-modified-p nil))
>             (funcall initial-major-mode)))
>         (when display (pop-to-buffer-same-window buf))
>         buf))
>
> i.e. combine the new function with the existing command, so we don't
> need to come up with a new name.

Nice idea, but I'd argue for keeping them separate.  I find it more
natural to distinguish functions called for their return values and
commands.

-- 
Sean Whitton

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

[Sean Whitton]

Hello,

On Mon 09 May 2022 at 02:11pm -04, Stefan Monnier wrote:

> I know you didn't like my `scratch-buffer--create` suggestion because of
> the double dash, but I think at least "scratch" would be very welcome in it.

I've done s/initial/scratch/ which is better because this function
always uses *scratch* rather than, say, looking at initial-buffer-choice.

> Now that I look at it again, it occurs to me that maybe we should do
> something like:
>
>     (defun scratch-buffer (&optional display)
>       "Create the \*scratch\* buffer.
>     If the buffer doesn't exist, create it first.
>     If DISPLAY (or when used interactively), switch to it."
>       (interactive (list t))
>       (let ((buf (get-buffer "*scratch*")))
>         (unless buf
>           ;; Don't touch the buffer contents or mode unless we know that
>           ;; we just created it.
>           (with-current-buffer (setq buf (get-buffer-create "*scratch*"))
>             (when initial-scratch-message
>               (insert (substitute-command-keys initial-scratch-message))
>               (set-buffer-modified-p nil))
>             (funcall initial-major-mode)))
>         (when display (pop-to-buffer-same-window buf))
>         buf))
>
> i.e. combine the new function with the existing command, so we don't
> need to come up with a new name.

Nice idea, but I'd argue for keeping them separate.  I find it more
natural to distinguish functions called for their return values and
commands.

-- 
Sean Whitton