bug#4623: 23.1.50; rmail changes encoding of characters on save

[Markus Rost <rost@math.uni-bielefeld.de>]

> Btw, I find rmail-swap-buffers incomplete in its handling of encoding
> and the modified flag.  It looks like it works by sheer luck, unless
> I'm missing something.  I suggest the following more thorough version.

I feel like running in circles.  Eli's rmail-swap-buffers function
(except for the modp-that) is in my previous post

<URL:http://lists.gnu.org/archive/html/bug-gnu-emacs/2009-10/msg00182.html>

which itself mentions Eli's Oct 2008 message:

<URL:http://lists.gnu.org/archive/html/emacs-devel/2008-10/msg00468.html>

See also the other messages in

<URL:http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4623>
<URL:http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4655>

> Maybe we need to set up an after-save-hook to restore the original
> encoding after saving the message collection?

Agreed.  I don't see any other way without changing files.el and
fileio.c.  The real problem is the buffer change in
write-region-annotate-functions and this part of basic-save-buffer

		(setq setmodes (basic-save-buffer-1)))
	    ;; Now we have saved the current buffer.  Let's make sure
	    ;; that buffer-file-coding-system is fixed to what
	    ;; actually used for saving by binding it locally.
	    (if save-buffer-coding-system
		(setq save-buffer-coding-system last-coding-system-used)
	      (setq buffer-file-coding-system last-coding-system-used))

which just assumes that last-coding-system-used was used in the
current buffer.  This part of basic-save-buffer practically forbids a
buffer change in write-region-annotate-functions.

Anyway, I think that the logical way for saving in Rmail is to swap
buffers back and forth and not change buffers.

What about the patch below?

And:  Could someone change the name of the variable rmail-view-buffer?
A good name is perhaps 'rmail-hold-buffer'.

Also: The doc string of rmail-buffer-swapped is not helpful.  What about

(defvar rmail-buffer-swapped nil
  "If nil, `rmail-buffer' contains the actual mbox message collection")

or

(defvar rmail-buffer-swapped nil
  "If non-nil, the actual mbox message collection is in `rmail-hold-buffer'.")


=== Buffer *vc-diff* =======================================
*** rmail.el	05 Dec 2009 20:13:54 +0100	1.562
--- rmail.el	05 Dec 2009 20:38:06 +0100	
***************
*** 1279,1284 ****
--- 1279,1286 ----
    (rmail-perm-variables)
    (rmail-variables))
  
+ (defvar rmail-swapped-before-save nil)
+ 
  (defun rmail-mode-1 ()
    (setq major-mode 'rmail-mode)
    (setq mode-name "RMAIL")
***************
*** 1293,1303 ****
    (set-syntax-table text-mode-syntax-table)
    (setq local-abbrev-table text-mode-abbrev-table)
    ;; Functions to support buffer swapping:
!   (add-hook 'write-region-annotate-functions
! 	    'rmail-write-region-annotate nil t)
    (add-hook 'kill-buffer-hook 'rmail-mode-kill-buffer-hook nil t)
    (add-hook 'change-major-mode-hook 'rmail-change-major-mode-hook nil t))
  
  (defun rmail-generate-viewer-buffer ()
    "Return a reusable buffer suitable for viewing messages.
  Create the buffer if necessary."
--- 1295,1322 ----
    (set-syntax-table text-mode-syntax-table)
    (setq local-abbrev-table text-mode-abbrev-table)
    ;; Functions to support buffer swapping:
!   (make-local-variable 'rmail-swapped-before-save)
!   (add-hook 'before-save-hook 'rmail-swap-before-save nil t)
!   (add-hook 'after-save-hook 'rmail-swap-after-save t t)
    (add-hook 'kill-buffer-hook 'rmail-mode-kill-buffer-hook nil t)
    (add-hook 'change-major-mode-hook 'rmail-change-major-mode-hook nil t))
  
+ (defun rmail-swap-before-save ()
+   (setq rmail-swapped-before-save nil)
+   (when (rmail-buffers-swapped-p)
+     (rmail-swap-buffers)
+     ;; This is probably not necssary, but let's be on the safe side.
+     (set (make-local-variable 'coding-system-for-write) 'no-conversion)
+     (setq rmail-buffer-swapped nil)
+     (setq rmail-swapped-before-save t)
+     (widen)))
+ 
+ (defun rmail-swap-after-save ()
+   (when rmail-swapped-before-save
+     (rmail-swap-buffers)
+     (setq rmail-buffer-swapped t)
+     (setq rmail-swapped-before-save nil)))
+ 
  (defun rmail-generate-viewer-buffer ()
    "Return a reusable buffer suitable for viewing messages.
  Create the buffer if necessary."
***************
*** 1313,1328 ****
  
  (defun rmail-swap-buffers ()
    "Swap text between current buffer and `rmail-view-buffer'.
! This function preserves the current buffer's modified flag, and also
! sets the current buffer's `buffer-file-coding-system' to that of
! `rmail-view-buffer'."
!   (let ((modp (buffer-modified-p))
! 	(coding
  	 (with-current-buffer rmail-view-buffer
  	   buffer-file-coding-system)))
      (buffer-swap-text rmail-view-buffer)
!     (setq buffer-file-coding-system coding)
!     (restore-buffer-modified-p modp)))
  
  (defun rmail-buffers-swapped-p ()
    "Return non-nil if the message collection is in `rmail-view-buffer'."
--- 1332,1353 ----
  
  (defun rmail-swap-buffers ()
    "Swap text between current buffer and `rmail-view-buffer'.
!   This function preserves the buffers' modified flags, and also
!   swaps the current buffer's `buffer-file-coding-system' with that
!   of `rmail-view-buffer'."
!   (let ((modp-this (buffer-modified-p))
! 	(modp-that
! 	 (with-current-buffer rmail-view-buffer (buffer-modified-p)))
! 	(coding-this buffer-file-coding-system)
! 	(coding-that
  	 (with-current-buffer rmail-view-buffer
  	   buffer-file-coding-system)))
      (buffer-swap-text rmail-view-buffer)
!     (setq buffer-file-coding-system coding-that)
!     (with-current-buffer rmail-view-buffer
!       (setq buffer-file-coding-system coding-this)
!       (restore-buffer-modified-p modp-that))
!     (restore-buffer-modified-p modp-this)))
  
  (defun rmail-buffers-swapped-p ()
    "Return non-nil if the message collection is in `rmail-view-buffer'."
***************
*** 1470,1477 ****
    (interactive)
    (set-buffer rmail-buffer)
    (rmail-expunge)
-   ;; No need to swap buffers: rmail-write-region-annotate takes care of it.
-   ;; (rmail-swap-buffers-maybe)
    (save-buffer)
    (if (rmail-summary-exists)
        (rmail-select-summary (set-buffer-modified-p nil))))
--- 1495,1500 ----
***************
*** 4179,4194 ****
  (add-to-list 'desktop-buffer-mode-handlers
  	     '(rmail-mode . rmail-restore-desktop-buffer))
  
- ;; Used in `write-region-annotate-functions' to write rmail files.
- (defun rmail-write-region-annotate (start end)
-   (when (and (null start) (rmail-buffers-swapped-p))
-     (set-buffer rmail-view-buffer)
-     ;; Prevent viewing different messages from messing up the coding. (Bug#4623)
-     ;; FIXME is there a better solution?
-     (set (make-local-variable 'coding-system-for-write) 'no-conversion)
-     (widen)
-     nil))
- 
  \f
  ;;; Start of automatically extracted autoloads.
  \f
--- 4202,4207 ----
============================================================