bug#4655: 23.1.50; buffer-swap-text oddity

[Markus Rost]

A follow up to my previous message:

If you look at the end of rmail-cease-edit:

    (save-excursion
      (rmail-show-message)
      (rmail-toggle-header (if pruned 1 0))))
  (run-hooks 'rmail-mode-hook))

the question arises in which buffer rmail-mode-hook should be run?
One may check with

(add-hook 'rmail-mode-hook
          (lambda () (message "foo: %s" (current-buffer))))

that currently it runs in rmail-view-buffer, which is perhaps not what
one expects.  Changing to

    (rmail-show-message)
    (save-excursion
      (rmail-toggle-header (if pruned 1 0))))
  (run-hooks 'rmail-mode-hook))

is probably OK.  That would be in line with Stefan's point:

>  5- most uses of save-excursion are wrong,

=======

Here are more issues related to the usage of buffer-swap-text:

I tried to fix

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

(I think this bug should be fixed before the next release:  The mbox
files should not be corrupted.)

with the patch below.  See also the thread of

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

It seems to work, except that after a save ('s' in Rmail) the
rmail-buffer gets a wrong value of buffer-file-coding-system (to be
seen in the mode line).  Only temporarily, switching back and forth to
another message in Rmail restores the correct value of
buffer-file-coding-system.

After further debugging, it turned out that the problem sits in

;; 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)
    (widen)
    nil))

Namely, when rmail-expunge-and-save calls save-buffer, then, with
rmail-write-region-annotate in write-region-annotate-functions,
save-buffer and basic-save-buffer* write the rmail-view-buffer to the
file (which is correct) but then set the buffer-file-coding-system to
last-coding-system-used in the rmail-buffer (which is not correct).

It seems that generally write-region-annotate-functions should not
change the buffer to be written.  (What if arg START of write-region
is a buffer position?)


===File ~/.emacs.d/rmail/rmail23/bug/rmail-swap.diff========
*** rmail.el    06 Oct 2009 13:20:10 +0200      1.554
--- rmail.el    08 Oct 2009 14:07:01 +0200      
***************
*** 1306,1320 ****
  
  (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 ()
--- 1306,1322 ----
  
  (defun rmail-swap-buffers ()
    "Swap text between current buffer and `rmail-view-buffer'.
! This function preserves the current buffer's modified flag and
! also swaps the value of `buffer-file-coding-system'."
    (let ((modp (buffer-modified-p))
+       (coding0 buffer-file-coding-system)
        (coding
         (with-current-buffer rmail-view-buffer
           buffer-file-coding-system)))
      (buffer-swap-text rmail-view-buffer)
      (setq buffer-file-coding-system coding)
+     (with-current-buffer rmail-view-buffer
+       (setq buffer-file-coding-system coding0))
      (restore-buffer-modified-p modp)))
  
  (defun rmail-buffers-swapped-p ()
============================================================