GNU bug report logs - #55257
29.0.50; New command `scratch-buffer' inconsistent with startup

Previous Next

Package: emacs;

Reported by: David Ponce <da_vid <at> orange.fr>

Date: Wed, 4 May 2022 08:33:02 UTC

Severity: normal

Found in version 29.0.50

Done: Sean Whitton <spwhitton <at> spwhitton.name>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55257 in the body.
You can then email your comments to 55257 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#55257; Package emacs. (Wed, 04 May 2022 08:33:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to David Ponce <da_vid <at> orange.fr>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Wed, 04 May 2022 08:33:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; New command `scratch-buffer' inconsistent with startup
Date: Wed, 4 May 2022 10:31:52 +0200
Hello,

The new command `scratch-buffer' does not re-create the *scratch* buffer
like in startup.el.

The below patch fix this:
--- a/./installs/emacs/lisp/simple.el
+++ b/./emacs.d/simple.el
@@ -10221,7 +10221,8 @@ If the buffer doesn't exist, create it first."
       (pop-to-buffer-same-window "*scratch*")
     (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
     (when initial-scratch-message
-      (insert initial-scratch-message))
+      (insert (substitute-command-keys initial-scratch-message))
+      (set-buffer-modified-p nil))
     (funcall initial-major-mode)))

Thanks!

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.31, cairo version 1.17.4)
 of 2022-05-04
Repository revision: 268713e227e8b665b1874c96ea96d1e7fccaab11
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 35 (KDE Plasma)

Configured using:
 'configure --with-cairo --without-sqlite3
 PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
WEBP X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LC_TIME: fr_FR.utf8
  value of $LANG: fr_FR.UTF-8
  locale-coding-system: utf-8-unix




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#55257; Package emacs. (Wed, 04 May 2022 14:59:02 GMT) Full text and rfc822 format available.

Message #8 received at 55257 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: David Ponce <da_vid <at> orange.fr>
Cc: 55257 <at> debbugs.gnu.org
Subject: Re: bug#55257: 29.0.50; New command `scratch-buffer' inconsistent
 with startup
Date: Wed, 04 May 2022 07:58:44 -0700
Hello,

On Wed 04 May 2022 at 10:31am +02, David Ponce wrote:

> Hello,
>
> The new command `scratch-buffer' does not re-create the *scratch* buffer
> like in startup.el.
>
> The below patch fix this:
> --- a/./installs/emacs/lisp/simple.el
> +++ b/./emacs.d/simple.el
> @@ -10221,7 +10221,8 @@ If the buffer doesn't exist, create it first."
>         (pop-to-buffer-same-window "*scratch*")
>       (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
>       (when initial-scratch-message
> -      (insert initial-scratch-message))
> +      (insert (substitute-command-keys initial-scratch-message))
> +      (set-buffer-modified-p nil))
>       (funcall initial-major-mode)))

Thanks.  Let me try to fix this as part of factoring out the *scratch*
initialisation code, as discussed in an emacs-devel thread.

-- 
Sean Whitton




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Thu, 05 May 2022 22:08:01 GMT) Full text and rfc822 format available.

Message #11 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257-submitter <at> debbugs.gnu.org,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Thu, 05 May 2022 15:07:41 -0700
[Message part 1 (text/plain, inline)]
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
[0001-Factor-out-scratch-initialization.patch (text/x-patch, attachment)]

Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Thu, 05 May 2022 22:14:01 GMT) Full text and rfc822 format available.

Message #14 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257-submitter <at> debbugs.gnu.org,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Thu, 05 May 2022 15:13:24 -0700
[Message part 1 (text/plain, inline)]
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
[0001-Factor-out-scratch-initialization.patch (text/x-patch, attachment)]

Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Fri, 06 May 2022 05:42:01 GMT) Full text and rfc822 format available.

Message #17 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: rpluim <at> gmail.com, 55257-submitter <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Fri, 06 May 2022 08:40:50 +0300
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: Robert Pluim <rpluim <at> gmail.com>, emacs-devel <at> gnu.org,
>  55257-submitter <at> 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.




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Fri, 06 May 2022 08:00:04 GMT) Full text and rfc822 format available.

Message #20 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257-submitter <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Fri, 06 May 2022 10:41:06 +0300
> 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"));




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Fri, 06 May 2022 11:35:02 GMT) Full text and rfc822 format available.

Message #23 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257-submitter <at> debbugs.gnu.org,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Fri, 06 May 2022 07:34:40 -0400
> +  (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





Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Fri, 06 May 2022 19:21:01 GMT) Full text and rfc822 format available.

Message #26 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257-submitter <at> debbugs.gnu.org,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Fri, 06 May 2022 12:20:11 -0700
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




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Fri, 06 May 2022 19:25:01 GMT) Full text and rfc822 format available.

Message #29 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257-submitter <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Fri, 06 May 2022 21:24:02 +0200
Sean Whitton <spwhitton <at> 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




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Fri, 06 May 2022 19:27:02 GMT) Full text and rfc822 format available.

Message #32 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rpluim <at> gmail.com, 55257-submitter <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Fri, 06 May 2022 12:26:46 -0700
Hello,

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

>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: Robert Pluim <rpluim <at> gmail.com>, emacs-devel <at> gnu.org,
>>  55257-submitter <at> 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




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Fri, 06 May 2022 19:29:01 GMT) Full text and rfc822 format available.

Message #35 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Juri Linkov <juri <at> linkov.net>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257-submitter <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Fri, 06 May 2022 12:28:25 -0700
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




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Sat, 07 May 2022 05:31:02 GMT) Full text and rfc822 format available.

Message #38 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: rpluim <at> gmail.com, 55257-submitter <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Sat, 07 May 2022 08:30:06 +0300
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: monnier <at> iro.umontreal.ca, rpluim <at> gmail.com, emacs-devel <at> gnu.org,
>  55257-submitter <at> 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?




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Sat, 07 May 2022 13:53:02 GMT) Full text and rfc822 format available.

Message #41 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rpluim <at> gmail.com, emacs-devel <at> gnu.org, 55257-submitter <at> debbugs.gnu.org,
 Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Sat, 07 May 2022 09:51:54 -0400
>> 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





Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Sat, 07 May 2022 14:14:01 GMT) Full text and rfc822 format available.

Message #44 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: rpluim <at> gmail.com, emacs-devel <at> gnu.org, 55257-submitter <at> debbugs.gnu.org,
 spwhitton <at> spwhitton.name
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Sat, 07 May 2022 17:12:45 +0300
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Sean Whitton <spwhitton <at> spwhitton.name>,  rpluim <at> gmail.com,
>   emacs-devel <at> gnu.org,  55257-submitter <at> 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.




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Sat, 07 May 2022 16:30:03 GMT) Full text and rfc822 format available.

Message #47 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rpluim <at> gmail.com, 55257-submitter <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Sat, 07 May 2022 09:29:50 -0700
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




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Sat, 07 May 2022 16:43:01 GMT) Full text and rfc822 format available.

Message #50 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: rpluim <at> gmail.com, 55257-submitter <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Sat, 07 May 2022 19:41:50 +0300
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: monnier <at> iro.umontreal.ca, rpluim <at> gmail.com, emacs-devel <at> gnu.org,
>  55257-submitter <at> 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.




Message sent on to David Ponce <da_vid <at> orange.fr>:
bug#55257. (Sat, 07 May 2022 17:03:01 GMT) Full text and rfc822 format available.

Message #53 received at 55257-submitter <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rpluim <at> gmail.com, 55257-submitter <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Sat, 07 May 2022 10:02:48 -0700
Hello,

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

>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: monnier <at> iro.umontreal.ca, rpluim <at> gmail.com, emacs-devel <at> gnu.org,
>>  55257-submitter <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#55257; Package emacs. (Sun, 08 May 2022 00:28:02 GMT) Full text and rfc822 format available.

Message #56 received at 55257 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: emacs-devel <at> gnu.org, 55257 <at> debbugs.gnu.org
Cc: Robert Pluim <rpluim <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Sat, 07 May 2022 17:27:38 -0700
[Message part 1 (text/plain, inline)]
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
[v2-0001-Factor-out-scratch-initialization.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#55257; Package emacs. (Mon, 09 May 2022 18:12:02 GMT) Full text and rfc822 format available.

Message #59 received at 55257 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Mon, 09 May 2022 14:11:14 -0400
> 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.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#55257; Package emacs. (Tue, 10 May 2022 01:50:02 GMT) Full text and rfc822 format available.

Message #62 received at 55257 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Robert Pluim <rpluim <at> gmail.com>, 55257 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, emacs-devel <at> gnu.org
Subject: Re: master f2d2fe6fc8: server-execute: Initialize the *scratch* buffer
Date: Mon, 09 May 2022 18:49:02 -0700
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




Reply sent to Sean Whitton <spwhitton <at> spwhitton.name>:
You have taken responsibility. (Tue, 10 May 2022 01:51:01 GMT) Full text and rfc822 format available.

Notification sent to David Ponce <da_vid <at> orange.fr>:
bug acknowledged by developer. (Tue, 10 May 2022 01:51:01 GMT) Full text and rfc822 format available.

Message #67 received at 55257-done <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: 55257-done <at> debbugs.gnu.org
Subject: Re: bug#55257: 29.0.50; New command `scratch-buffer' inconsistent
 with startup
Date: Mon, 09 May 2022 18:50:21 -0700
Hello,

On Wed 04 May 2022 at 07:58am -07, Sean Whitton wrote:

> Hello,
>
> On Wed 04 May 2022 at 10:31am +02, David Ponce wrote:
>
>> Hello,
>>
>> The new command `scratch-buffer' does not re-create the *scratch* buffer
>> like in startup.el.
>>
>> The below patch fix this:
>> --- a/./installs/emacs/lisp/simple.el
>> +++ b/./emacs.d/simple.el
>> @@ -10221,7 +10221,8 @@ If the buffer doesn't exist, create it first."
>>         (pop-to-buffer-same-window "*scratch*")
>>       (pop-to-buffer-same-window (get-buffer-create "*scratch*"))
>>       (when initial-scratch-message
>> -      (insert initial-scratch-message))
>> +      (insert (substitute-command-keys initial-scratch-message))
>> +      (set-buffer-modified-p nil))
>>       (funcall initial-major-mode)))
>
> Thanks.  Let me try to fix this as part of factoring out the *scratch*
> initialisation code, as discussed in an emacs-devel thread.

This is now on master.

-- 
Sean Whitton




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 07 Jun 2022 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 322 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.