bug#66825: last-coding-system-used in basic-save-buffer

[Juri Linkov]

>> Indeed, the problem is in basic-save-buffer on the following line:
>>
>>     (setq buffer-file-coding-system last-coding-system-used)
>>
>> It's hard to guess why this code relies on the value that
>> can be changed by other functions during saving the buffer.
>
> Because this is the protocol: functions that determine the encoding of
> buffer text dynamically set this variable, so it could later be used
> to reflect the detection in buffer-file-coding-system.

Still it seems there is some flaw in the design when some
functions change 'last-coding-system-used' intentionally,
but some can change it unintentionally that causes this bug.

>> A possible workaround would be to protect the value of
>> last-coding-system-used in 'project-mode-line-format':
>
> That's not a workaround, that's what we should do in such cases.
> Alternatively, the "important" setting of last-coding-system-used
> should be done later, after the inner functions already returned.

I don't understand this alternative.  The mode line updating
that uses 'project-mode-line-format' that unintentionally
changes 'last-coding-system-used' is called from this line
in 'basic-save-buffer-2':

  (write-region nil nil
                buffer-file-name nil t buffer-file-truename)

because this call in 'write_region' updates the mode line:

    message_with_string ((NUMBERP (append)
                          ? "Updated %s"
                          : ! NILP (append)
                          ? "Added to %s"
                          : "Wrote %s"),
                         visit_file, 1);

>> However, I noticed that occasionally this bug occurs even
>> when this function is not used.  So the proper fix needed
>> in 'basic-save-buffer', but I don't know if it's intended
>> that some function should change 'last-coding-system-used'
>> during saving the buffer.
>
> Yes, it's intended: saving the buffer can change
> buffer-file-coding-system if it detects characters which cannot be
> encoded by the original buffer-file-coding-system.

I see this detection happens in the same 'write_region':

  /* Decide the coding-system to encode the data with.
     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);

This means the value 'last-coding-system-used' should be preserved
afterwards.  Therefore I don't see how this could be fixed in
'basic-save-buffer' and 'write-region'.

Ok, then let's fix this in 'project-mode-line-format':

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index bb44cfefa54..da89036160a 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -2074,14 +2121,17 @@ project-mode-line-format
 (defun project-mode-line-format ()
   "Compose the project mode-line."
   (when-let ((project (project-current)))
-    (concat
-     " "
-     (propertize
-      (project-name project)
-      'face project-mode-line-face
-      'mouse-face 'mode-line-highlight
-      'help-echo "mouse-1: Project menu"
-      'local-map project-mode-line-map))))
+    ;; Preserve the original value of 'last-coding-system-used'
+    ;; that can break 'basic-save-buffer' (bug#66825)
+    (let ((last-coding-system-used nil))
+      (concat
+       " "
+       (propertize
+        (project-name project)
+        'face project-mode-line-face
+        'mouse-face 'mode-line-highlight
+        'help-echo "mouse-1: Project menu"
+        'local-map project-mode-line-map)))))
 
 (provide 'project)
 ;;; project.el ends here