Package: emacs;
Reported by: Markus Rost <rost <at> math.uni-bielefeld.de>
Date: Sat, 3 Oct 2009 15:00:05 UTC
Severity: normal
Done: Glenn Morris <rgm <at> gnu.org>
Bug is archived. No further changes may be made.
Message #60 received at 4623 <at> emacsbugs.donarmstrong.com (full text, mbox):
From: Markus Rost <rost <at> math.uni-bielefeld.de> To: Eli Zaretskii <eliz <at> gnu.org> Cc: monnier <at> iro.umontreal.ca, 4623 <at> debbugs.gnu.org Subject: Re: bug#4623: 23.1.50; rmail changes encoding of characters on save Date: Sat, 5 Dec 2009 21:10:56 +0100 (CET)
> 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://debbugs.gnu.org/cgi/bugreport.cgi?bug=4623>
<URL:http://debbugs.gnu.org/cgi/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))
-
;;; Start of automatically extracted autoloads.
--- 4202,4207 ----
============================================================
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.