bug#13522: 24.2; save-buffer removes edited file under some conditions

[Lars Ingebrigtsen]

Vincent Lefevre <vincent@vinc17.net> writes:

>> Yes, but the file is apparently missing after `C-z'.
>
> Actually, as I've said in another mail, you do not even need to do C-z.
> The file is missing after C-x C-s: if I try to look at the file from
> a different terminal (a real terminal or with GNU Screen), this file
> is missing. This may also be problematic for those who have automatic
> backups to a remote storage area (in particular if files matching *~
> are ignored for these backups).

Yup; it's not ideal.

Glenn applied one solution to this (binding coding-system-for-write),
but that had other problems, apparently.  I've now respun his original
patch, and as far as I can tell, it works pretty well.  However, it
breaks files-tests-bug-18141.

But before I try to debug this, does anybody see anything fundamentally
misguided about this approach?

diff --git a/lisp/files.el b/lisp/files.el
index 2b0dc54db8..7e3bea614e 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5649,7 +5649,7 @@ basic-save-buffer-1
 ;; This returns a value (MODES EXTENDED-ATTRIBUTES BACKUPNAME), like
 ;; backup-buffer.
 (defun basic-save-buffer-2 ()
-  (let (tempsetmodes setmodes)
+  (let (tempsetmodes setmodes writecoding)
     (if (not (file-writable-p buffer-file-name))
        (let ((dir (file-name-directory buffer-file-name)))
          (if (not (file-directory-p dir))
@@ -5665,6 +5665,9 @@ basic-save-buffer-2
                     buffer-file-name)))
                  (setq tempsetmodes t)
                (error "Attempt to save to a file that you aren't allowed to 
write"))))))
+    (setq writecoding
+         (choose-write-coding-system nil nil buffer-file-name nil t
+                                     buffer-file-truename))
     (or buffer-backed-up
        (setq setmodes (backup-buffer)))
     (let* ((dir (file-name-directory buffer-file-name))
@@ -5697,8 +5700,9 @@ basic-save-buffer-2
                  ;; Pass in nil&nil rather than point-min&max
                  ;; cause we're saving the whole buffer.
                  ;; write-region-annotate-functions may use it.
-                 (write-region nil nil tempname nil realname
-                               buffer-file-truename)
+                 (let ((write-region-coding-system writecoding))
+                   (write-region nil nil tempname nil realname
+                                 buffer-file-truename))
                  (when save-silently (message nil)))
              ;; If we failed, restore the buffer's modtime.
              (error (set-visited-file-modtime old-modtime)
@@ -5743,8 +5747,9 @@ basic-save-buffer-2
                 ;; Pass in nil&nil rather than point-min&max to indicate
                 ;; we're saving the buffer rather than just a region.
                 ;; write-region-annotate-functions may make use of it.
-                (write-region nil nil
-                              buffer-file-name nil t buffer-file-truename)
+               (let ((write-region-coding-system writecoding))
+                  (write-region nil nil
+                                buffer-file-name nil t buffer-file-truename))
                 (when save-silently (message nil))
                (setq success t))
            ;; If we get an error writing the new file, and we made
diff --git a/src/fileio.c b/src/fileio.c
index c418036fc6..f04383252a 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -5012,14 +5012,22 @@ build_annotations_unwind (Lisp_Object arg)
 
 /* Decide the coding-system to encode the data with.  */
 
-static Lisp_Object
-choose_write_coding_system (Lisp_Object start, Lisp_Object end, Lisp_Object 
filename,
-                           Lisp_Object append, Lisp_Object visit, Lisp_Object 
lockname,
-                           struct coding_system *coding)
+DEFUN ("choose-write-coding-system", Fchoose_write_coding_system,
+       Schoose_write_coding_system, 3, 6, 0,
+       doc: /* Choose coding system for write.
+Arguments as for `write-region'.  */ )
+  (Lisp_Object start, Lisp_Object end, Lisp_Object filename,
+   Lisp_Object append, Lisp_Object visit, Lisp_Object lockname)
 {
   Lisp_Object val;
   Lisp_Object eol_parent = Qnil;
 
+  if (NILP (start))
+    {
+      XSETFASTINT (start, BEGV);
+      XSETFASTINT (end, ZV);
+    }
+
   if (auto_saving
       && NILP (Fstring_equal (BVAR (current_buffer, filename),
                              BVAR (current_buffer, auto_save_file_name))))
@@ -5119,10 +5127,6 @@ choose_write_coding_system (Lisp_Object start, 
Lisp_Object end, Lisp_Object file
     }
 
   val = coding_inherit_eol_type (val, eol_parent);
-  setup_coding_system (val, coding);
-
-  if (!STRINGP (start) && EQ (Qt, BVAR (current_buffer, selective_display)))
-    coding->mode |= CODING_MODE_SELECTIVE_DISPLAY;
   return val;
 }
 
@@ -5287,9 +5291,15 @@ write_region (Lisp_Object start, Lisp_Object end, 
Lisp_Object filename,
      We used to make this choice before calling build_annotations, but that
      leads to problems when a write-annotate-function takes care of
      unsavable chars (as was the case with X-Symbol).  */
-  Vlast_coding_system_used
-    = choose_write_coding_system (start, end, filename,
-                                 append, visit, lockname, &coding);
+  Vlast_coding_system_used = NILP (Vwrite_region_coding_system) ?
+    Fchoose_write_coding_system (start, end, filename,
+                                append, visit, lockname) :
+    Vwrite_region_coding_system;
+
+  setup_coding_system (Vlast_coding_system_used, &coding);
+
+  if (!STRINGP (start) && !NILP (BVAR (current_buffer, selective_display)))
+    coding.mode |= CODING_MODE_SELECTIVE_DISPLAY;
 
   if (open_and_close_file && !auto_saving)
     {
@@ -6372,6 +6382,7 @@ syms_of_fileio (void)
   DEFSYM (Qset_file_acl, "set-file-acl");
   DEFSYM (Qfile_newer_than_file_p, "file-newer-than-file-p");
   DEFSYM (Qinsert_file_contents, "insert-file-contents");
+  DEFSYM (Qchoose_write_coding_system, "choose-write-coding-system");
   DEFSYM (Qwrite_region, "write-region");
   DEFSYM (Qverify_visited_file_modtime, "verify-visited-file-modtime");
   DEFSYM (Qset_visited_file_modtime, "set-visited-file-modtime");
@@ -6614,6 +6625,11 @@ do (file-exists-p FILENAME) and FILENAME is handled by 
HANDLER, then
   DEFSYM (Qstdout, "stdout");
   DEFSYM (Qstderr, "stderr");
 
+  DEFVAR_LISP ("write-region-coding-system", Vwrite_region_coding_system,
+              doc: /* If non-nil, coding system for `write-region'.
+ You should only ever `let'-bind this around a `write-region' call.  */);
+  Vwrite_region_coding_system = Qnil;
+
   defsubr (&Sfind_file_name_handler);
   defsubr (&Sfile_name_directory);
   defsubr (&Sfile_name_nondirectory);
@@ -6655,6 +6671,7 @@ do (file-exists-p FILENAME) and FILENAME is handled by 
HANDLER, then
   defsubr (&Sdefault_file_modes);
   defsubr (&Sfile_newer_than_file_p);
   defsubr (&Sinsert_file_contents);
+  defsubr (&Schoose_write_coding_system);
   defsubr (&Swrite_region);
   defsubr (&Scar_less_than_car);
   defsubr (&Sverify_visited_file_modtime);


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