bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Lucas Werkmeister]

Launch graphical Emacs with standard input attached to a pipe, then
attempt to insert /dev/stdin, for example:

     echo test | emacs -Q --insert /dev/stdin
     echo test | emacs -Q --eval '(insert-file-contents "/dev/stdin")'

This now results in an error (and nothing inserted into the buffer):
"Maximum buffer size exceeded".

Previously, this used to work; git bisect identifies cb4579ed6b ("Allow
inserting parts of /dev/urandom with insert-file-contents", bug#18370)
as the first bad commit.

Other (non-stdin) pipes are also affected. Testing with a named pipe 
shows that the error only occurs after the pipe is first written to:

     # first terminal:
     mkfifo /tmp/fifo
     emacs -Q
     M-x insert-file /tmp/fifo
     # second terminal:
     { echo x; sleep 10; echo y; } > /tmp/fifo

Before you run the command in the second terminal, you can observe that 
Emacs is just waiting for input from the pipe; as soon as you run the 
other command, Emacs shows the error before the sleep finishes.


In GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38,
cairo version 1.17.8)
Windowing system distributor 'The X.Org Foundation', version 11.0.12301002
System Description: Arch Linux

Configured using:
  'configure --sysconfdir=/etc --prefix=/usr --libexecdir=/usr/lib
  --with-tree-sitter --localstatedir=/var --with-cairo
  --disable-build-details --with-harfbuzz --with-libsystemd
  --with-modules --with-x-toolkit=gtk3 'CFLAGS=-march=x86-64
  -mtune=generic -O2 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=2
  -Wformat -Werror=format-security -fstack-clash-protection
  -fcf-protection -g
  -ffile-prefix-map=/build/emacs/src=/usr/src/debug/emacs -flto=auto'
  'LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now -flto=auto''

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

Important settings:
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   xterm-mouse-mode: t
   global-auto-revert-mode: t
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   column-number-mode: t
   line-number-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message yank-media puny dired
dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config
gnus-util text-property-search time-date mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils xt-mouse autorevert
filenotify finder-inf rx info package browse-url url url-proxy
url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 220386 8042)
  (symbols 48 13986 0)
  (strings 32 76266 1637)
  (string-bytes 1 1839262)
  (vectors 16 21600)
  (vector-slots 8 323307 13800)
  (floats 8 29 30)
  (intervals 56 250 0)
  (buffers 984 10))

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Eli Zaretskii]

> Date: Tue, 8 Aug 2023 20:20:23 +0200
> From: Lucas Werkmeister <mail@lucaswerkmeister.de>
> 
> Launch graphical Emacs with standard input attached to a pipe, then
> attempt to insert /dev/stdin, for example:
> 
>      echo test | emacs -Q --insert /dev/stdin
>      echo test | emacs -Q --eval '(insert-file-contents "/dev/stdin")'
> 
> This now results in an error (and nothing inserted into the buffer):
> "Maximum buffer size exceeded".

The doc string says:

  When inserting data from a special file (e.g., /dev/urandom), you
  can’t specify VISIT or BEG, and END should be specified to avoid
  inserting unlimited data into the buffer.

> Previously, this used to work; git bisect identifies cb4579ed6b ("Allow
> inserting parts of /dev/urandom with insert-file-contents", bug#18370)
> as the first bad commit.
> 
> Other (non-stdin) pipes are also affected. Testing with a named pipe 
> shows that the error only occurs after the pipe is first written to:
> 
>      # first terminal:
>      mkfifo /tmp/fifo
>      emacs -Q
>      M-x insert-file /tmp/fifo
>      # second terminal:
>      { echo x; sleep 10; echo y; } > /tmp/fifo
> 
> Before you run the command in the second terminal, you can observe that 
> Emacs is just waiting for input from the pipe; as soon as you run the 
> other command, Emacs shows the error before the sleep finishes.

Lars, Paul, any suggestions?

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Eli Zaretskii]

> Cc: 65156@debbugs.gnu.org
> Date: Tue, 08 Aug 2023 21:33:55 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Tue, 8 Aug 2023 20:20:23 +0200
> > From: Lucas Werkmeister <mail@lucaswerkmeister.de>
> > 
> > Launch graphical Emacs with standard input attached to a pipe, then
> > attempt to insert /dev/stdin, for example:
> > 
> >      echo test | emacs -Q --insert /dev/stdin
> >      echo test | emacs -Q --eval '(insert-file-contents "/dev/stdin")'
> > 
> > This now results in an error (and nothing inserted into the buffer):
> > "Maximum buffer size exceeded".
> 
> The doc string says:
> 
>   When inserting data from a special file (e.g., /dev/urandom), you
>   can’t specify VISIT or BEG, and END should be specified to avoid
>   inserting unlimited data into the buffer.
> 
> > Previously, this used to work; git bisect identifies cb4579ed6b ("Allow
> > inserting parts of /dev/urandom with insert-file-contents", bug#18370)
> > as the first bad commit.
> > 
> > Other (non-stdin) pipes are also affected. Testing with a named pipe 
> > shows that the error only occurs after the pipe is first written to:
> > 
> >      # first terminal:
> >      mkfifo /tmp/fifo
> >      emacs -Q
> >      M-x insert-file /tmp/fifo
> >      # second terminal:
> >      { echo x; sleep 10; echo y; } > /tmp/fifo
> > 
> > Before you run the command in the second terminal, you can observe that 
> > Emacs is just waiting for input from the pipe; as soon as you run the 
> > other command, Emacs shows the error before the sleep finishes.
> 
> Lars, Paul, any suggestions?

I installed the patch below on the emacs-29 branch; please see if it
solves your problems with reading from pipes.

Paul, can there be a regular file that is not seekable?  If regular
files are always seekable, the patch can be simplified.

diff --git a/src/fileio.c b/src/fileio.c
index 995e414..55132f1 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4581,7 +4581,7 @@ because (1) it preserves some marker positions (in unchanged portions
       goto handled;
     }
 
-  if (seekable || !NILP (end))
+  if ((seekable && regular) || !NILP (end))
     total = end_offset - beg_offset;
   else
     /* For a special file, all we can do is guess.  */
@@ -4678,7 +4678,7 @@ because (1) it preserves some marker positions (in unchanged portions
 	   For a special file, where TOTAL is just a buffer size,
 	   so don't bother counting in HOW_MUCH.
 	   (INSERTED is where we count the number of characters inserted.)  */
-	if (seekable || !NILP (end))
+	if ((seekable && regular) || !NILP (end))
 	  how_much += this;
 	inserted += this;
       }

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 65156@debbugs.gnu.org
>> Date: Tue, 08 Aug 2023 21:33:55 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>> > Date: Tue, 8 Aug 2023 20:20:23 +0200
>> > From: Lucas Werkmeister <mail@lucaswerkmeister.de>
>> > 
>> > Launch graphical Emacs with standard input attached to a pipe, then
>> > attempt to insert /dev/stdin, for example:
>> > 
>> >      echo test | emacs -Q --insert /dev/stdin
>> >      echo test | emacs -Q --eval '(insert-file-contents "/dev/stdin")'
>> > 
>> > This now results in an error (and nothing inserted into the buffer):
>> > "Maximum buffer size exceeded".
>> 
>> The doc string says:
>> 
>>   When inserting data from a special file (e.g., /dev/urandom), you
>>   can’t specify VISIT or BEG, and END should be specified to avoid
>>   inserting unlimited data into the buffer.
>> 
>> > Previously, this used to work; git bisect identifies cb4579ed6b ("Allow
>> > inserting parts of /dev/urandom with insert-file-contents", bug#18370)
>> > as the first bad commit.
>> > 
>> > Other (non-stdin) pipes are also affected. Testing with a named pipe 
>> > shows that the error only occurs after the pipe is first written to:
>> > 
>> >      # first terminal:
>> >      mkfifo /tmp/fifo
>> >      emacs -Q
>> >      M-x insert-file /tmp/fifo
>> >      # second terminal:
>> >      { echo x; sleep 10; echo y; } > /tmp/fifo
>> > 
>> > Before you run the command in the second terminal, you can observe that 
>> > Emacs is just waiting for input from the pipe; as soon as you run the 
>> > other command, Emacs shows the error before the sleep finishes.
>> 
>> Lars, Paul, any suggestions?
>
> I installed the patch below on the emacs-29 branch; please see if it
> solves your problems with reading from pipes.
>
> Paul, can there be a regular file that is not seekable?  If regular
> files are always seekable, the patch can be simplified.
>
> diff --git a/src/fileio.c b/src/fileio.c
> index 995e414..55132f1 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -4581,7 +4581,7 @@ because (1) it preserves some marker positions (in unchanged portions
>        goto handled;
>      }
>  
> -  if (seekable || !NILP (end))
> +  if ((seekable && regular) || !NILP (end))
>      total = end_offset - beg_offset;
>    else
>      /* For a special file, all we can do is guess.  */
> @@ -4678,7 +4678,7 @@ because (1) it preserves some marker positions (in unchanged portions
>  	   For a special file, where TOTAL is just a buffer size,
>  	   so don't bother counting in HOW_MUCH.
>  	   (INSERTED is where we count the number of characters inserted.)  */
> -	if (seekable || !NILP (end))
> +	if ((seekable && regular) || !NILP (end))
>  	  how_much += this;
>  	inserted += this;
>        }

Everyone, I fixed this on master yesterday, as part of a series of
changes to enable visiting named pipes as files.  The crux of the
problem is this:

      seekable = emacs_fd_lseek (fd, 0, SEEK_CUR) < 0; <----------
      if (!NILP (beg) && !seekable)
	xsignal2 (Qfile_error,

where `seekable' is actually set to 1 if the file is NOT seekable, since
lseek returns 0 upon success and -1 upon failure.

Then, later:

  if (seekable || !NILP (end)) <------------------------
    total = end_offset - beg_offset;
  else
    /* For a special file, all we can do is guess.  */
    total = READ_BUF_SIZE;

total is set to end_offset - beg_offset (both -1 at that point),
resulting in a call to buffer_overflow as the gap is extended beyond
memory.

The change presently in place on emacs-29 should be replaced with a
one-line change that replaces:

  seekable = lseek (fd, 0, SEEK_CUR) < 0;

with

  seekable = lseek (fd, 0, SEEK_CUR) != (off_t) -1;

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Mattias Engdegård]

The current (4767f5eaee) code on emacs-29 fails fileio-tests with:

Test fileio-tests--non-regular-insert condition:
    (ert-test-failed
     ((should-error
       (insert-file-contents "/dev/urandom" nil 5 10))
      :form
      (insert-file-contents "/dev/urandom" nil 5 10)
      :value
      ("/dev/urandom" 5)
      :fail-reason "did not signal an error"))

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

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

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> The current (4767f5eaee) code on emacs-29 fails fileio-tests with:
>
> Test fileio-tests--non-regular-insert condition:
>     (ert-test-failed
>      ((should-error
>        (insert-file-contents "/dev/urandom" nil 5 10))
>       :form
>       (insert-file-contents "/dev/urandom" nil 5 10)
>       :value
>       ("/dev/urandom" 5)
>       :fail-reason "did not signal an error"))

Right, but that's a problem with the test: /dev/urandom is seekable
under the Linux kernel.  I fixed this on master by checking for a
regular file instead of a seekable file when verifying BEG, but that's
too extensive a change for Emacs 29.

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Mattias Engdegård]

9 aug. 2023 kl. 10.15 skrev Po Lu <luangruo@yahoo.com>:

> Right, but that's a problem with the test: /dev/urandom is seekable
> under the Linux kernel.  I fixed this on master by checking for a
> regular file instead of a seekable file when verifying BEG, but that's
> too extensive a change for Emacs 29.

Thank you. We cannot have tests that keep failing, so I disabled it on emacs-29.

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: mail@lucaswerkmeister.de,  eggert@cs.ucla.edu,  larsi@gnus.org,
>   65156@debbugs.gnu.org
> Date: Wed, 09 Aug 2023 10:47:41 +0800
> 
> The change presently in place on emacs-29 should be replaced with a
> one-line change that replaces:
> 
>   seekable = lseek (fd, 0, SEEK_CUR) < 0;
> 
> with
> 
>   seekable = lseek (fd, 0, SEEK_CUR) != (off_t) -1;

Thanks.  In the future, when you encounter such bugs, please look into
their VCS history, and if the bug was introduced during the
development of the last release, fix it on the release branch,
especially if the fix is simple and safe.

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Wed, 9 Aug 2023 10:29:12 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  Lars Ingebrigtsen <larsi@gnus.org>,
>  Paul Eggert <eggert@cs.ucla.edu>,
>  65156@debbugs.gnu.org,
>  mail@lucaswerkmeister.de
> 
> 9 aug. 2023 kl. 10.15 skrev Po Lu <luangruo@yahoo.com>:
> 
> > Right, but that's a problem with the test: /dev/urandom is seekable
> > under the Linux kernel.  I fixed this on master by checking for a
> > regular file instead of a seekable file when verifying BEG, but that's
> > too extensive a change for Emacs 29.
> 
> Thank you. We cannot have tests that keep failing, so I disabled it on emacs-29.

Thanks, but that wasn't TRT in this case.  That test is useful, and
moreover, tests a feature introduced in Emacs 29.  One of the
sub-tests fails, so I just commented it out for now, and re-enabled
the test as a whole.

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Mattias Engdegård]

9 aug. 2023 kl. 14.00 skrev Eli Zaretskii <eliz@gnu.org>:

> One of the
> sub-tests fails, so I just commented it out for now, and re-enabled
> the test as a whole.

Thank you, that is even better. I can confirm that it still passes on macOS.

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Paul Eggert]

On 2023-08-08 12:27, Eli Zaretskii wrote:
> Paul, can there be a regular file that is not seekable?

No.

The current code on master is a bit of a mess. There's nothing wrong 
with using a positive BEG on a seekable and non-regular file, for 
example; the old doc string was wrong and now the code has been changed 
to match the doc string unfortunately. Nor is there anything wrong if 
BEG is 0 on a non-seekable file (though this latter issue is 
longstanding and so I guess nobody cares).

There are surely some gotchas involving the REPLACE arg of 
insert-file-contents too, but it's hard to tell because the doc string 
seems to be corrupted for the case where REPLACE is 'if-regular' and I 
don't know what it's trying to say.

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Eli Zaretskii]

> Date: Wed, 9 Aug 2023 13:57:36 -0700
> Cc: larsi@gnus.org, 65156@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 2023-08-08 12:27, Eli Zaretskii wrote:
> > Paul, can there be a regular file that is not seekable?
> 
> No.

Thanks.  This is no longer an issue, though.

> The current code on master is a bit of a mess. There's nothing wrong 
> with using a positive BEG on a seekable and non-regular file, for 
> example; the old doc string was wrong and now the code has been changed 
> to match the doc string unfortunately. Nor is there anything wrong if 
> BEG is 0 on a non-seekable file (though this latter issue is 
> longstanding and so I guess nobody cares).

These are minor issues that should be easy to clean up, I think?  If
so, now is a good time for doing that on master.

> There are surely some gotchas involving the REPLACE arg of 
> insert-file-contents too, but it's hard to tell because the doc string 
> seems to be corrupted for the case where REPLACE is 'if-regular' and I 
> don't know what it's trying to say.

I guess you are alluding to this part:

  If REPLACE is the symbol ‘if-regular’, then eschew preserving marker
  positions or the undo list if REPLACE is nil if FILENAME is not a
  regular file.  Otherwise, signal an error if REPLACE is non-nil and
  FILENAME is not a regular file.

Which part(s) of this are unclear?

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Paul Eggert]

On 2023-08-09 22:17, Eli Zaretskii wrote:
>    If REPLACE is the symbol ‘if-regular’, then eschew preserving marker
>    positions or the undo list if REPLACE is nil if FILENAME is not a
>    regular file.  Otherwise, signal an error if REPLACE is non-nil and
>    FILENAME is not a regular file.
> 
> Which part(s) of this are unclear?

In "If REPLACE is the symbol 'if-regular', then <X> if REPLACE is nil if 
<Y>. Otherwise, ..." I don't know what the first sentence means. The "if 
REPLACE is nil" seems to contradict the "If REPLACE is the symbol 
'if-regular'" and the relationship between <X> and <Y> is unclear.

Nor do I know which "if" the "Otherwise" is referring to.

Nor is it easy to see how this paragraph connects to the previous one, 
the one that begins "If optional fifth argument REPLACE is non-nil" and 
that goes on to say "When REPLACE is non-nil" as if the second phrase 
were not redundant (so which part of that paragraph talks about what 
happens when REPLACE being nil?).

It's understandable that the doc string is a mess, since the code is 
messier. (Yes, this is the peanut gallery talking....)

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Eli Zaretskii]

> Date: Wed, 9 Aug 2023 23:08:49 -0700
> Cc: mail@lucaswerkmeister.de, larsi@gnus.org, 65156@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 2023-08-09 22:17, Eli Zaretskii wrote:
> >    If REPLACE is the symbol ‘if-regular’, then eschew preserving marker
> >    positions or the undo list if REPLACE is nil if FILENAME is not a
> >    regular file.  Otherwise, signal an error if REPLACE is non-nil and
> >    FILENAME is not a regular file.
> > 
> > Which part(s) of this are unclear?
> 
> In "If REPLACE is the symbol 'if-regular', then <X> if REPLACE is nil if 
> <Y>. Otherwise, ..." I don't know what the first sentence means.

I think it should be changed to say this instead:

  If REPLACE is the symbol ‘if-regular’, then eschew preserving marker
  positions or the undo list when FILENAME is not a regular file.
  Otherwise, signal an error if REPLACE is non-nil and FILENAME is not
  a regular file.

AFAICT, this is what the code does.

> Nor do I know which "if" the "Otherwise" is referring to.

It alludes to the case that REPLACE is not 'if-regular'.

> Nor is it easy to see how this paragraph connects to the previous one, 
> the one that begins "If optional fifth argument REPLACE is non-nil" and 
> that goes on to say "When REPLACE is non-nil" as if the second phrase 
> were not redundant (so which part of that paragraph talks about what 
> happens when REPLACE being nil?).

This describes what happens when REPLACE is neither nil nor
'if-regular', AFAICT.  IOW, the "smart" replacement happens only with
regular files; with non-regular files we either erase the buffer and
insert the stuff from the file (if REPLACE is 'if-regular') or signal
an error.

If you agree that this is what the doc string should say, I will
reword it (but feel free to beat me to that ;-).

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Paul Eggert]

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

On 2023-08-10 01:15, Eli Zaretskii wrote:
> I think it should be changed to say this instead:
> 
>    If REPLACE is the symbol ‘if-regular’, then eschew preserving marker
>    positions or the undo list when FILENAME is not a regular file.
>    Otherwise, signal an error if REPLACE is non-nil and FILENAME is not
>    a regular file.
> 
> AFAICT, this is what the code does.

It does more than that, no? If REPLACE is 'if-regular' and the file is 
not a regular file, the function replaces the entire buffer not the 
accessible region.

How about something like the attached instead?


[-- Attachment #2: insert-doc.patch --]
[-- Type: text/x-patch, Size: 872 bytes --]

diff --git a/src/fileio.c b/src/fileio.c
index 52bbaa61fc2..40c870331b8 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4031,10 +4031,9 @@ because (1) it preserves some marker positions (in unchanged portions
 undo list.  When REPLACE is non-nil, the second return value is the
 number of characters that replace previous buffer contents.
 
-If REPLACE is the symbol `if-regular', then eschew preserving marker
-positions or the undo list if REPLACE is nil if FILENAME is not a
-regular file.  Otherwise, signal an error if REPLACE is non-nil and
-FILENAME is not a regular file.
+If REPLACE is non-nil and FILENAME is not a regular file, act as if
+REPLACE were nil if REPLACE is the symbol `if-regular' and signal an
+error otherwise.
 
 This function does code conversion according to the value of
 `coding-system-for-read' or `file-coding-system-alist', and sets the

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Eli Zaretskii]

> Date: Fri, 11 Aug 2023 10:18:16 -0700
> Cc: mail@lucaswerkmeister.de, larsi@gnus.org, 65156@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> diff --git a/src/fileio.c b/src/fileio.c
> index 52bbaa61fc2..40c870331b8 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -4031,10 +4031,9 @@ because (1) it preserves some marker positions (in unchanged portions
>  undo list.  When REPLACE is non-nil, the second return value is the
>  number of characters that replace previous buffer contents.
>  
> -If REPLACE is the symbol `if-regular', then eschew preserving marker
> -positions or the undo list if REPLACE is nil if FILENAME is not a
> -regular file.  Otherwise, signal an error if REPLACE is non-nil and
> -FILENAME is not a regular file.
> +If REPLACE is non-nil and FILENAME is not a regular file, act as if
> +REPLACE were nil if REPLACE is the symbol `if-regular' and signal an
> +error otherwise.

This is fine, but I think instead of "act as if REPLACE were nil" we
should explicitly say that the buffer is erased and the file's
contents is inserted.

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Paul Eggert]

On 2023-08-11 11:30, Eli Zaretskii wrote:
> This is fine, but I think instead of "act as if REPLACE were nil" we
> should explicitly say that the buffer is erased and the file's
> contents is inserted.

I'm still a bit lost here. It's news to me that REPLACE being nil means 
the entire buffer is erased first.

The bigger picture is that I don't know what insert-file-contents is 
supposed to do in all these complicated circumstances. THat is the 
various motivations behind insert-file-contents's complex argument 
combinations don't fully make sense to me. So I'll step aside and let 
someone more expert fix the doc string, whenever anybody has the time.

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

[Eli Zaretskii]

> Date: Fri, 11 Aug 2023 14:45:48 -0700
> Cc: mail@lucaswerkmeister.de, larsi@gnus.org, 65156@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 2023-08-11 11:30, Eli Zaretskii wrote:
> > This is fine, but I think instead of "act as if REPLACE were nil" we
> > should explicitly say that the buffer is erased and the file's
> > contents is inserted.
> 
> I'm still a bit lost here. It's news to me that REPLACE being nil means 
> the entire buffer is erased first.

That's actually a bug: it should only erase the accessible portion of
the buffer.  I fixed it now.

> The bigger picture is that I don't know what insert-file-contents is 
> supposed to do in all these complicated circumstances. THat is the 
> various motivations behind insert-file-contents's complex argument 
> combinations don't fully make sense to me. So I'll step aside and let 
> someone more expert fix the doc string, whenever anybody has the time.

Until we wait for someone to figure that out, I attempted to make
things better by fixing the doc string's language as best I could, see
the master branch.

WDYT about backporting this (both the doc string and the erasing fix)
to emacs-29?

bug#65156: 29.1; Reading from pipe with --insert or insert-file-contents no longer supported

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

Eli Zaretskii <eliz@gnu.org> writes:

> WDYT about backporting this (both the doc string and the erasing fix)
> to emacs-29?

That's not necessary, since `if-regular' doesn't exist on Emacs 29; it's
used in Emacs 30 to ``visit'' non-regular files as files.

I sincerely apologize for causing this mess.