Subject: [PATCH] (Finsert_file_contents): Refine commit d07af40d8826
From: Stefan Monnier <[email-masked]>
To: [email-masked]
Date: Sun, 22 Jun 2025 22:28:31 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Tags: patch

As requested in bug#78777, here's a separate bug report for the
following problem:

- Start with:

      % src/emacs -Q BUGS &
      % echo foo >>BUGS

- In the Emacs session type:

      a

  Notice how Emacs correctly prompts about "changed on disk, really edit".
  Hit `C-g` so we don't actually edit the buffer.

- In the Emacs session do:

      M-: (insert-file-contents "README" nil nil nil t) RET

  Notice how Emacs just blindly changed the contents of the buffer
  without prompting about "changed on disk, really edit".

I think this last step is an error, we should be prompted just as we are
with any other buffer modification that diverges from the file's contents.

The patch below does just that,

        Stefan

From 54c128dd5fa29293f10f6b8a667078285eddca0a Mon Sep 17 00:00:00 2001
From: Stefan Monnier <[email-masked]>
Date: Sun, 22 Jun 2025 22:25:03 -0400
Subject: [PATCH 2/2] (Finsert_file_contents): Refine commit d07af40d8826

* src/fileio.c (Finsert_file_contents): Inhibit ask-supersession
only if we're VISITing.

* test/src/fileio-tests.el (ert--tests-dir): New var.
(fileio-tests--insert-file-contents-supersession): New test.
---
 src/fileio.c             | 31 +++++++++++++++++++------------
 test/src/fileio-tests.el | 26 ++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index 7e1ac3fc383..39d577de7fe 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4550,14 +4550,19 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
 	  beg_offset += same_at_start - BEGV_BYTE;
 	  end_offset -= ZV_BYTE - same_at_end;
 
-          /* This binding is to avoid ask-user-about-supersession-threat
-	     being called in insert_from_buffer or del_range_bytes (via
-	     prepare_to_modify_buffer).
-             AFAICT we could avoid ask-user-about-supersession-threat by setting
-             current_buffer->modtime earlier, but we could still end up calling
-             ask-user-about-supersession-threat if the file is modified while
-             we read it, so we bind buffer-file-name instead.  */
-          specbind (Qbuffer_file_name, Qnil);
+          if (!NILP (visit))
+            /* This binding is to avoid ask-user-about-supersession-threat
+	       being called in insert_from_buffer or del_range_bytes (via
+	       prepare_to_modify_buffer).
+	       Such a prompt makes no sense if we're VISITing the file,
+	       since the insertion makes the buffer *more* like the file
+	       rather than the reverse.
+               AFAICT we could avoid ask-user-about-supersession-threat by
+               setting current_buffer->modtime earlier, but we could still
+               end up calling ask-user-about-supersession-threat if the file
+               is modified while we read it, so we bind buffer-file-name
+               instead.  */
+            specbind (Qbuffer_file_name, Qnil);
 	  del_range_byte (same_at_start, same_at_end);
 	  /* Insert from the file at the proper position.  */
 	  temp = BYTE_TO_CHAR (same_at_start);
@@ -4666,8 +4671,9 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
 	  /* Truncate the buffer to the size of the file.  */
 	  if (same_at_start != same_at_end)
 	    {
-              /* See previous specbind for the reason behind this.  */
-              specbind (Qbuffer_file_name, Qnil);
+              if (!NILP (visit))
+		/* See previous specbind for the reason behind this.  */
+		specbind (Qbuffer_file_name, Qnil);
 	      del_range_byte (same_at_start, same_at_end);
 	    }
 	  inserted = 0;
@@ -4716,8 +4722,9 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
 	 we are taking from the decoded string.  */
       inserted -= (ZV_BYTE - same_at_end) + (same_at_start - BEGV_BYTE);
 
-      /* See previous specbind for the reason behind this.  */
-      specbind (Qbuffer_file_name, Qnil);
+      if (!NILP (visit))
+        /* See previous specbind for the reason behind this.  */
+        specbind (Qbuffer_file_name, Qnil);
       if (same_at_end != same_at_start)
 	{
 	  del_range_byte (same_at_start, same_at_end);
diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index 13cc5de29e8..b6302c35fee 100644
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -235,5 +235,31 @@ fileio-tests-w32-time-stamp-granularity
                   "2025/02/01 23:15:59.123456700")))
       (delete-file tfile))))
 
+(defconst ert--tests-dir
+  (file-name-directory (macroexp-file-name)))
+
+(ert-deftest fileio-tests--insert-file-contents-supersession ()
+  (ert-with-temp-file file
+    (write-region "foo" nil file)
+    (let* ((asked nil)
+           (buf (find-file-noselect file))
+           (auast (lambda (&rest _) (setq asked t))))
+      (unwind-protect
+          (with-current-buffer buf
+            ;; Pretend someone else edited the file.
+            (write-region "bar" nil file 'append)
+            ;; Use `advice-add' rather than `cl-letf' because the function
+            ;; may not be defined yet.
+            (advice-add 'ask-user-about-supersession-threat :override auast)
+            ;; Modify the local buffer via `insert-file-contents'.
+            (insert-file-contents
+             (expand-file-name "lread-resources/somelib.el"
+                               ert--tests-dir)
+             nil nil nil 'replace))
+        (advice-remove 'ask-user-about-supersession-threat auast)
+        (kill-buffer buf))
+      ;; We should have prompted about the supersession threat.
+      (should asked))))
+
 
 ;;; fileio-tests.el ends here
-- 
2.39.5