bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Eli Zaretskii]

Please write in English if possible, because the Emacs maintainers
usually do not have translators to read other languages for them.

Your bug report will be posted to the emacs-pretest-bug@gnu.org mailing list.

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:

If one invokes rmail-edit, and edits the headers of the message, the
changes to the headers are not copied back to the original message
held in rmail-buffer.  The effect is that your edits of the headers
are lost after you type "C-c C-c" to exit rmail-edit.

Note that fixing this properly should not just copy the modified
headers, but also take the necessary actions for special headers like
Content-Type and Content-Transfer-Encoding.

If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
If you would like to further debug the crash, please read the file
d:/gnu/emacs/etc/DEBUG for instructions.


In GNU Emacs 23.0.90.1 (i386-mingw-nt5.1.2600)
 of 2009-02-07 on HOME-C4E4A596F7
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4)'

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: ENU
  value of $XMODIFIERS: nil
  locale-coding-system: cp1255
  default-enable-multibyte-characters: t

Major mode: RMAIL

Minor modes in effect:
  tooltip-mode: t
  tool-bar-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
- b u <tab> <tab> C-g M-: ( s e t - b u f f e r - m 
u M-/ SPC n i l ) <return> C-x o C-SPC <right> <right> 
<right> <right> <right> <right> <right> <right> <right> 
<right> <right> M-: ( e n c o d e - c o d i n g - r 
e g i o n SPC ( m a r k ) S-SPC ( p o i n t ) S-SPC 
( g e t - b u f f e r SPC " * f o o * " ) ) <return> 
C-] M-: <up> <M-right> <C-right> <C-right> <C-right> 
<C-right> <right> SPC ' u n d e c i d e d C-e <return> 
C-x o <right> <right> <right> <right> <right> <right> 
<right> <right> <right> <right> C-x o M-: <up> <M-right> 
<C-right> <C-right> <C-right> <C-right> <C-right> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> r a w - t e x t 
- u n i x C-e <return> C-x o C-x o SPC C-y <left> <left> 
<left> <left> <left> <backspace> C-u C-\ l a t i n 
- 1 - p r e <tab> <return> : u <backspace> <backspace> 
" u C-a C-SPC C-e M-w C-x o <down> <down> <return> 
<return> C-x o M-: <up> <return> C-x o C-e <return> 
<return> C-x o M-: <up> <up> <return> <help-echo> <help-echo> 
<help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
C-x 1 C-u M-x r m a i l <return> ~ / d a <tab> I N 
B <tab> n <tab> <return> h C-x o <help-echo> <help-echo> 
<down-mouse-1> <help-echo> <drag-mouse-1> M-1 j e <down> 
<down> <down> <down> <down> <down> <left> , SPC f o 
o <down> <down> <down> <down> <down> C-c C-c M-x r 
e p o r t - e m <tab> <return>

Recent messages:
Quit
10 (#o12, #xa) [2 times]
Mark set
Loading quail/latin-pre...done
Mark set
22 (#o26, #x16) [2 times]
Loading vc-cvs...done
Counting messages...done
Computing summary lines...done
Editing: Type C-c C-c to return to Rmail, C-c C-] to abort

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Richard M Stallman]

    If one invokes rmail-edit, and edits the headers of the message, the
    changes to the headers are not copied back to the original message
    held in rmail-buffer.  The effect is that your edits of the headers
    are lost after you type "C-c C-c" to exit rmail-edit.

I never got around to writing the code to copy header edits into the
original message.  Perhaps the easiest way to do it is to put the "old
version" into a buffer and compare the edited headers with the old
headers.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

Richard M Stallman wrote:

>     If one invokes rmail-edit, and edits the headers of the message, the
>     changes to the headers are not copied back to the original message
>     held in rmail-buffer.  The effect is that your edits of the headers
>     are lost after you type "C-c C-c" to exit rmail-edit.
>
> I never got around to writing the code to copy header edits into the
> original message.  Perhaps the easiest way to do it is to put the "old
> version" into a buffer and compare the edited headers with the old
> headers.

I think that editing a message should present the message with headers
expanded. In which case I don't see why changing the header would be
any different to changing the body.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: 2228@emacsbugs.donarmstrong.com,  Eli Zaretskii <eliz@gnu.org>
> Date: Sun, 08 Feb 2009 15:43:04 -0500
> 
> Richard M Stallman wrote:
> 
> >     If one invokes rmail-edit, and edits the headers of the message, the
> >     changes to the headers are not copied back to the original message
> >     held in rmail-buffer.  The effect is that your edits of the headers
> >     are lost after you type "C-c C-c" to exit rmail-edit.
> >
> > I never got around to writing the code to copy header edits into the
> > original message.  Perhaps the easiest way to do it is to put the "old
> > version" into a buffer and compare the edited headers with the old
> > headers.
> 
> I think that editing a message should present the message with headers
> expanded. In which case I don't see why changing the header would be
> any different to changing the body.

Sounds like a good plan.  Would you like to give it a shot?

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

Eli Zaretskii wrote:

> Sounds like a good plan.  Would you like to give it a shot?

Will do.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Richard M Stallman]

    I think that editing a message should present the message with headers
    expanded.

I think that editing the pruned headers is an important feature.  The
full headers are too long and inconvenient.

As it is now, you get the choice.  If you display the full headers and
then edit, then you get to edit the full headers.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

Richard M Stallman wrote:

> I think that editing the pruned headers is an important feature.  The
> full headers are too long and inconvenient.
>
> As it is now, you get the choice.  If you display the full headers and
> then edit, then you get to edit the full headers.

With the minor drawback that it doesn't actually work, hence this bug report.

IMO fixing that method is too much work for too little gain.

What I proposed is what both VM and Gnus do AFAIK.


rmailedit as a whole seems overly complex to me. If I were doing it,
I'd copy the expanded message text to a new buffer for editing, rather
than re-using the rmail buffer. Aborting an edit would just kill the
buffer. Saving an edit would compare the buffer text with the region
between msgbeg and msgend in rmail-view-buffer, and swap it in if it
differed, then call rmail-show-message.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Eli Zaretskii]

> From: Richard M Stallman <rms@gnu.org>
> CC: 2228@emacsbugs.donarmstrong.com, eliz@gnu.org
> Date: Mon, 09 Feb 2009 19:11:23 -0500
> 
>     I think that editing a message should present the message with headers
>     expanded.
> 
> I think that editing the pruned headers is an important feature.  The
> full headers are too long and inconvenient.

OTOH, the pre-mbox Rmail was always expanding the headers in
rmail-edit.  So I think we will be OK having header editing working
first, even if full headers are deemed slightly inconvenient by some
users.  (FWIW, I don't mind the expanded headers at all, and I do use
rmail-edit quite a lot.)  It is more important, IMO, to have Rmail
working at least as well as it was before the mbox merge than to
improve it.  We can always improve it later.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: 2228@emacsbugs.donarmstrong.com,  eliz@gnu.org
> Date: Mon, 09 Feb 2009 22:38:30 -0500
> 
> rmailedit as a whole seems overly complex to me. If I were doing it,
> I'd copy the expanded message text to a new buffer for editing, rather
> than re-using the rmail buffer.

??? The buffer where the current Rmail shows the message _is_ a
separate buffer.  It holds only the single message that is shown, not
the whole message collection.  Why do you need yet another buffer?

> Aborting an edit would just kill the buffer.

You can replace that with erasing it and then re-displaying the
current message with rmail-show-message.

> Saving an edit would compare the buffer text with the region
> between msgbeg and msgend in rmail-view-buffer, and swap it in if it
> differed, then call rmail-show-message.

You could do that with the current arrangement as well, can't you?

But please be very conservative about throwing away the existing code
in rmailedit: some of it could actually implement valuable features.
We have been bit by that in the very recent past: a too naive approach
to reimplementing features anew which lost some of them in the
process, because there's no requirements spec for what Rmail does.
The current trail of Rmail-related bugs is the evidence of how
dangerous that can be.

Thanks.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Richard M Stallman]

    > Sounds like a good plan.  Would you like to give it a shot?

    Will do.

Please don't change it to always edit the full headers!

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Eli Zaretskii]

> From: Richard M Stallman <rms@gnu.org>
> CC: eliz@gnu.org, 2228@emacsbugs.donarmstrong.com
> Date: Tue, 10 Feb 2009 15:02:26 -0500
> 
>     > Sounds like a good plan.  Would you like to give it a shot?
> 
>     Will do.
> 
> Please don't change it to always edit the full headers!

Why not? that's what the Babyl Rmail did, so it's obviously not that
bad.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Stefan Monnier]

> OTOH, the pre-mbox Rmail was always expanding the headers in
> rmail-edit.  So I think we will be OK having header editing working
> first, even if full headers are deemed slightly inconvenient by some
> users.  (FWIW, I don't mind the expanded headers at all, and I do use
> rmail-edit quite a lot.)  It is more important, IMO, to have Rmail
> working at least as well as it was before the mbox merge than to
> improve it.  We can always improve it later.

Agreed.  Especially about the last part: the goal for 23.1 is to get
back to the features set ot Emacs-22's Rmail, no less and no more.
Improvement will come later.


        Stefan

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Richard M Stallman]

    > As it is now, you get the choice.  If you display the full headers and
    > then edit, then you get to edit the full headers.

    With the minor drawback that it doesn't actually work, hence this bug report.

It works fine for the usual job, editing the message body.
Making it work for editing headers probably takes 200 lines of code.

    rmailedit as a whole seems overly complex to me. If I were doing it,
    I'd copy the expanded message text to a new buffer for editing, rather
    than re-using the rmail buffer. Aborting an edit would just kill the
    buffer. Saving an edit would compare the buffer text with the region
    between msgbeg and msgend in rmail-view-buffer, and swap it in if it
    differed, then call rmail-show-message.

I would not object to switching to that approach, but still it shouldn't
normally show all the unpruned headers.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Richard M Stallman]

    Why not? that's what the Babyl Rmail did, so it's obviously not that
    bad.

Yes it was -- that's why I changed it.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Eli Zaretskii]

> From: Richard M Stallman <rms@gnu.org>
> CC: rgm@gnu.org, 2228@emacsbugs.donarmstrong.com
> Date: Wed, 11 Feb 2009 01:22:51 -0500
> 
>     Why not? that's what the Babyl Rmail did, so it's obviously not that
>     bad.
> 
> Yes it was -- that's why I changed it.

IMO, we should have our priorities right: first make Rmail no worse
than it was before, then improve it.  We can even have a release in
between.

If we keep insisting on perfection, we will never release Emacs 23.1.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

Richard M Stallman wrote:

> It works fine for the usual job, editing the message body.

However, it has a clear bug, in that it silently discards any changes
made to the headers.

> Making it work for editing headers probably takes 200 lines of code.

(That's more code than exists in rmailedit.el in total at present.)

Me installing the 10 line fix I have for the above problem will not
make it any harder for someone to write those 200 lines in the future.
I will happily open the wishlist bug for that.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Stefan Monnier]

> Me installing the 10 line fix I have for the above problem will not
> make it any harder for someone to write those 200 lines in the future.

Can you show us that fix?


        Stefan

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

Stefan Monnier wrote:

>> Me installing the 10 line fix I have for the above problem will not
>> make it any harder for someone to write those 200 lines in the future.
>
> Can you show us that fix?


*** rmailedit.el	10 Feb 2009 03:33:48 -0000	1.50
--- rmailedit.el	12 Feb 2009 02:36:41 -0000
***************
*** 78,88 ****
  
  ;;;###autoload
  (defun rmail-edit-current-message ()
!   "Edit the contents of this message."
    (interactive)
    (if (zerop rmail-total-messages)
        (error "No messages in this buffer"))
!   (set (make-local-variable 'rmail-old-pruned) (rmail-msg-is-pruned))
    (rmail-edit-mode)
    (set (make-local-variable 'rmail-old-text)
         (save-restriction
--- 78,92 ----
  
  ;;;###autoload
  (defun rmail-edit-current-message ()
!   "Edit the contents of this message.
! Expands the header, moves point to the start of the body."
    (interactive)
    (if (zerop rmail-total-messages)
        (error "No messages in this buffer"))
!   (if (set (make-local-variable 'rmail-old-pruned) (rmail-msg-is-pruned))
!       (rmail-toggle-header 0))		; expand headers
!   (goto-char (point-min))
!   (search-forward "\n\n" nil t)		; move to start of body
    (rmail-edit-mode)
    (set (make-local-variable 'rmail-old-text)
         (save-restriction
***************
*** 113,119 ****
    ;; Disguise any "From " lines so they don't start a new message.
    (save-excursion
      (goto-char (point-min))
!     (or rmail-old-pruned (forward-line 1))
      (while (re-search-forward "^>*From " nil t)
        (beginning-of-line)
        (insert ">")
--- 117,123 ----
    ;; Disguise any "From " lines so they don't start a new message.
    (save-excursion
      (goto-char (point-min))
!     (forward-line 1)		       ; leave the initial ^From alone
      (while (re-search-forward "^>*From " nil t)
        (beginning-of-line)
        (insert ">")
***************
*** 126,135 ****
  	(insert "\n"))
      (unless (looking-back "\n\n")
        (insert "\n")))
    (let ((old rmail-old-text)
  	(pruned rmail-old-pruned)
  	character-coding is-text-message coding-system
! 	headers-end limit)
      ;; Go back to Rmail mode, but carefully.
      (force-mode-line-update)
      (let ((rmail-buffer-swapped nil)) ; Prevent change-major-mode-hook
--- 130,140 ----
  	(insert "\n"))
      (unless (looking-back "\n\n")
        (insert "\n")))
+   ;; FIXME should also ensure \n\n still ends the headers.
    (let ((old rmail-old-text)
  	(pruned rmail-old-pruned)
  	character-coding is-text-message coding-system
! 	start limit)
      ;; Go back to Rmail mode, but carefully.
      (force-mode-line-update)
      (let ((rmail-buffer-swapped nil)) ; Prevent change-major-mode-hook
***************
*** 143,152 ****
      ;; If text has really changed, mark message as edited.
      (unless (and (= (length old) (- (point-max) (point-min)))
  		 (string= old (buffer-substring (point-min) (point-max))))
!       (setq old nil)
!       (goto-char (point-min))
!       (search-forward "\n\n")
!       (setq headers-end (point))
        (rmail-swap-buffers-maybe)
        (narrow-to-region (rmail-msgbeg rmail-current-message)
  			(rmail-msgend rmail-current-message))
--- 148,155 ----
      ;; If text has really changed, mark message as edited.
      (unless (and (= (length old) (- (point-max) (point-min)))
  		 (string= old (buffer-substring (point-min) (point-max))))
!       (setq old nil
! 	    start (goto-char (point-min)))
        (rmail-swap-buffers-maybe)
        (narrow-to-region (rmail-msgbeg rmail-current-message)
  			(rmail-msgend rmail-current-message))
***************
*** 165,176 ****
        (if character-coding
  	  (setq character-coding (downcase character-coding)))
  
!       (goto-char limit)
        (let ((inhibit-read-only t))
  	(let ((data-buffer (current-buffer))
  	      (end (copy-marker (point) t)))
  	  (with-current-buffer rmail-view-buffer
! 	    (encode-coding-region headers-end (point-max) coding-system
  				  data-buffer))
  	  (delete-region end (point-max)))
  
--- 168,179 ----
        (if character-coding
  	  (setq character-coding (downcase character-coding)))
  
!       (goto-char (point-min))
        (let ((inhibit-read-only t))
  	(let ((data-buffer (current-buffer))
  	      (end (copy-marker (point) t)))
  	  (with-current-buffer rmail-view-buffer
! 	    (encode-coding-region start (point-max) coding-system
  				  data-buffer))
  	  (delete-region end (point-max)))
  

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

Glenn Morris wrote:

> ! 	    (encode-coding-region start (point-max) coding-system
>   				  data-buffer))

It's possible this piece of the change is bogus, and that the encoding
should only be applied to the body, not the headers as well; but you
get the idea.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Richard M Stallman]

    Me installing the 10 line fix I have for the above problem will not
    make it any harder for someone to write those 200 lines in the future.

In a literal sense, no, but it will remove the impetus for someone to
write them.

I can write them myself, and I will do so if necessary.  But just now
I am struggling to find time for things.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

Richard M Stallman wrote:

> In a literal sense, no, but it will remove the impetus for someone to
> write them.

I wondered if this was a hostage deal.

> I can write them myself, and I will do so if necessary.

As I already said, IMO it is too much work for too little gain.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

Glenn Morris wrote:

> As I already said, IMO it is too much work for too little gain.

I should also have said, I am too busy fixing actual bugs in rmail (of
which there are a large number) right now to worry about feature
requests. I would have thought this only sensible during pretesting.

Anyway, I've spent too much time on this.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Richard M Stallman]

    > In a literal sense, no, but it will remove the impetus for someone to
    > write them.

    I wondered if this was a hostage deal.

But who is the hostage?  I feel that others are refusing
so that I will have to do the work.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Glenn Morris]

tags 2228 moreinfo
stop

So can this bug be closed now?

Processed: Re: bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Emacs bug Tracking System]

Processing commands for control@emacsbugs.donarmstrong.com:

> tags 2228 moreinfo
bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers
There were no tags set.
Tags added: moreinfo

> stop
Stopping processing here.

Please contact me if you need assistance.

Don Armstrong
(administrator, Emacs bugs database)

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: 2228@emacsbugs.donarmstrong.com
> Date: Tue, 17 Feb 2009 23:39:59 -0500
> 
> tags 2228 moreinfo
> stop
> 
> So can this bug be closed now?

Sorry, I didn't yet have time to retest.  If you can wait a few more
days, I will get to that soon.  Or you can close the bug, and I will
reopen if needed.

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Xavier Maillard]

   So can this bug be closed now?

I think so. At least it works for me here.

	Xavier
-- 
http://www.gnu.org
http://www.april.org
http://www.lolica.org

bug#2228: 23.0.90; rmail-cease-edit ignores changes to the message headers

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: 2228@emacsbugs.donarmstrong.com
> Date: Tue, 17 Feb 2009 23:39:59 -0500
> 
> tags 2228 moreinfo
> stop
> 
> So can this bug be closed now?

Yes, thanks.