bug#4448: 23.1; unrmail fails if buffer has mixed line endings (patch)

bug#4448: 23.1; unrmail fails if buffer has mixed line endings (patch)

[David J. Biesack]

I invoked rmail-input on an RMAIL file and got the following traceback.

I performed find-file-literally on the rmail file and found that
it has mixed line endings - text text of the first messages end in just LF and
others end in CR LF. This causes the 

                  (re-search-forward "^[*][*][*] EOOH [*][*][*]\n")

in unrmail to fail.

below is the traceback.

Debugger entered--Lisp error: (search-failed "^[*][*][*] EOOH [*][*][*]
")
  re-search-forward("^[*][*][*] EOOH [*][*][*]\n")
  unrmail("c:/TEMP/rmail4124CNA" "c:/TEMP/rmail4124PXG")
  byte-code("=3fffc6=3fffc7!=3fffc6=3fffc7!\x18\x19=3fffc8=3fff8e=3fffc9 =3fff88=3fffcaed   #=3fff88=3fffcb \b\"=3fff88=3fffcc=3fffcd!=3fff88=3fffce=3fffcf=3fffce\x1a^[\x1c=3fffd0 =3fff88=3fffd1\b!=3fff88=3fffd2=3fffd3!=3fff88=3fffd4 =3fff88=3fffd5 =3fff88=3fffd6 =3fff88=3fffce\x15=3fffd7 =3fff88db=3fff88=3fffd8 =3fff88+=3fffcc=3fffd9!+=3fff87" [new-file old-file buffer-undo-list coding-system-for-read inhibit-read-only rmail-was-converted make-temp-file "rmail" ((byte-code "=3fffc2\b!=3fff88=3fffc2  !=3fff87" [old-file new-file delete-file] 2)) kill-all-local-variables write-region unrmail message "Replacing BABYL format with mbox format..." t raw-text erase-buffer insert-file-contents set-buffer-file-coding-system raw-text-unix rmail-mode-1 rmail-perm-variables rmail-variables rmail-dont-modify-format rmai
 l-set-message-counters "Replacing BABYL format with mbox format...done"] 4)
  rmail-convert-babyl-to-mbox()
  byte-code("=3fffc2 =3fff88eb=3fff88=3fffc3\x18=3fffc4=3fffc5!=3fff83\x13  rmail-convert-file-maybe()
  byte-code("=3fffc6 =3fff88=3fffc7\b=3fff86      rmail("u:/rmail/test-rmail-to-mbox")
  byte-code("=3fffc1\b!=3fff87" [filename rmail] 2)
  rmail-input("u:/rmail/test-rmail-to-mbox")
  call-interactively(rmail-input nil nil)

Here is a patch which makes it work, but I don't think this is the correct
change; it seems that the correct change would be to account for coding
systems. But this works for me.

*** unrmail.el.orig     Wed Sep 16 09:14:27 2009
--- unrmail.el  Wed Sep 16 09:24:22 2009
***************
*** 167,180 ****
                      (forward-line 1)))
                  (delete-region (point-min) (point))
                  ;; Delete the old reformatted header.
!                 (re-search-forward "^[*][*][*] EOOH [*][*][*]\n")
                  (forward-line -1)
                  (let ((start (point)))
!                   (search-forward "\n\n")
                    (delete-region start (point))))
              ;; Not reformatted.  Delete the special
              ;; lines before the real header.
!             (re-search-forward "^[*][*][*] EOOH [*][*][*]\n")
              (delete-region (point-min) (point)))
  
            ;; Handle rmime formatting.
--- 167,180 ----
                      (forward-line 1)))
                  (delete-region (point-min) (point))
                  ;; Delete the old reformatted header.
!                 (re-search-forward "^[*][*][*] EOOH [*][*][*]\015?\n")
                  (forward-line -1)
                  (let ((start (point)))
!                   (re-search-forward "\n\015?\n")
                    (delete-region start (point))))
              ;; Not reformatted.  Delete the special
              ;; lines before the real header.
!             (re-search-forward "^[*][*][*] EOOH [*][*][*]\015?\n")
              (delete-region (point-min) (point)))
  
            ;; Handle rmime formatting.
***************
*** 188,194 ****
            (save-restriction
              (narrow-to-region
               (point-min)
!              (save-excursion (search-forward "\n\n" nil 'move) (point)))
  
              ;; Fetch or construct what we should use in the `From ' line.
              (setq mail-from (or (let ((from (mail-fetch-field "Mail-From")))
--- 188,194 ----
            (save-restriction
              (narrow-to-region
               (point-min)
!              (save-excursion (re-search-forward "\n\015?\n" nil 'move) (point)))
  
              ;; Fetch or construct what we should use in the `From ' line.
              (setq mail-from (or (let ((from (mail-fetch-field "Mail-From")))
***************
*** 234,240 ****
                (insert ?>)))
            ;; Make sure the message ends with two newlines
            (goto-char (point-max))
!           (unless (looking-back "\n\n")
              (insert "\n"))
            ;; Write it to the output file, suitably encoded.
            (let ((coding-system-for-write coding))
--- 234,240 ----
                (insert ?>)))
            ;; Make sure the message ends with two newlines
            (goto-char (point-max))
!           (unless (looking-back "\n\015?\n")
              (insert "\n"))
            ;; Write it to the output file, suitably encoded.
            (let ((coding-system-for-write coding))


In GNU Emacs 23.1.1 (i386-mingw-nt5.1.2600)
 of 2009-07-30 on SOFT-MJASON
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.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: cp1252
  default-enable-multibyte-characters: t

Major mode: Debugger

Minor modes in effect:
  display-time-mode: t
  shell-dirtrack-mode: t
  delete-selection-mode: t
  pc-selection-mode: t
  show-paren-mode: t
  iswitchb-mode: t
  tooltip-mode: t
  mouse-wheel-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
  abbrev-mode: t

Recent input:
C-x C-o C-g C-g C-x o M-x ^ m <return> C-x C-s <escape> 
C-x k SPC <backspace> C-g C-g C-x k SPC q C-x b R M 
<return> i I n s t a SPC <return> C-x , o i n s t a 
SPC <return> y e s <return> C-x C-f C o d e SPC P SPC 
<return> q C-x b C o d e C-s C-s <return> C-x ~ M-x 
f i n d - f i l e - l i SPC <return> C o d e P SPC 
<return> y C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n 
C-n C-p C-p C-p C-s C-w C-w C-r C-r C-s C-s M-> C-r 
C-r M-< M-x ^ m <return> C-x C-s <escape> C-x k SPC 
C-g C-g C-g C-n C-n C-x k SPC i C o d e P SPC <return> 
C-x C-s h C-x o C-x b I <backspace> R M <return> o 
C o d e P r o SPC <return> y e s <return> C-x C-s C-x 
k SPC C-x C-f <backspace> <return> <up> <up> <up> <up> 
<up> <down> e C-x k SPC <down> <down> <down> <down> 
<down> <down> <down> C-x o i A d d SPC <return> C-x 
k SPC C-x o C-n C-n C-n C-p C-p C-n e C-x k SPC d C-n 
C-n C-n <f2> C-e <backspace> C-d <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
t e s t - r m a i l - t o - m b o x <return> C-x o 
C-x D C-x D i t e s t 0 <backspace> - m <backspace> 
r m SPC SPC SPC SPC <return> C-x h M-w M-x r e p o 
r t 0 <backspace> - e m SPC <return>

Recent messages:
Press RET when finished
Toggling font-lock-mode off; better pass an explicit argument.
Auto-saving...done
(setq debug-on-error nil)
(setq debug-on-error t)
Wrote c:/TEMP/rmail4124CNA
Writing messages to c:/TEMP/rmail4124PXG...
Entering debugger...
Mark set [2 times]
Region saved


-- 
David J. Biesack, SAS
SAS Campus Dr. Cary, NC 27513
www.sas.com    (919) 531-7771

bug#4448: 23.1; unrmail fails if buffer has mixed line endings (patch)

[Eli Zaretskii]

> Date: Wed, 16 Sep 2009 09:28:35 -0400
> From: "David J. Biesack" <David.Biesack@sas.com>
> Cc: 
> 
> 
> I invoked rmail-input on an RMAIL file and got the following traceback.
> 
> I performed find-file-literally on the rmail file and found that
> it has mixed line endings - text text of the first messages end in just LF and
> others end in CR LF. This causes the 
> 
>                   (re-search-forward "^[*][*][*] EOOH [*][*][*]\n")
> 
> in unrmail to fail.

Yes, by design (although the traceback is not by design).

> Here is a patch which makes it work, but I don't think this is the correct
> change; it seems that the correct change would be to account for coding
> systems.

Actually, I think the correct change would be to throw an error with a
meaningful error message.  Babyl files written by Rmail are supposed
to have Unix EOLs.  Of course, entering the debugger is not a graceful
reaction, but I don't think we should support incorrectly formatted
Babyl files.  That way lies madness.

Thanks.

bug#4448: 23.1; unrmail fails if buffer has mixed line endings (patch)

[David J. Biesack]

> Date: Wed, 16 Sep 2009 20:17:38 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > This causes the 
> > 
> >                   (re-search-forward "^[*][*][*] EOOH [*][*][*]\n")
> > 
> > in unrmail to fail.
> 
> Yes, by design (although the traceback is not by design).

I turned on debug-on-error to obtain the traceback
 
> > Here is a patch which makes it work, but I don't think this is the correct
> > change; it seems that the correct change would be to account for coding
> > systems.
> 
> Actually, I think the correct change would be to throw an error with a
> meaningful error message.  Babyl files written by Rmail are supposed
> to have Unix EOLs.

"supposed to", but in my case, this has happened multiple times (I have over 40 rmail files with \n in them) whether because rmail-edit, rmail-output, or the fact that I'm running on Windows and my rmail files are on a unix filesystem, or something else.

If you have to modify the code to detect the situation and generate an more meaningful error message, there should be clear instructions on how to fix it.... which is still not very friendly. In that case, you may as well be more robust to begin with and just make it work, at least for this case. (My opinion; I'm certainly going to leave my patch in my installation.)

> Of course, entering the debugger is not a graceful
> reaction, but I don't think we should support incorrectly formatted
> Babyl files.  That way lies madness.

Well, this again goes back to "how did it get incorrectly formatted?" and if Emacs created the problem, Emacs should correct it. I was happily using Emacs/rmail for years until 23.1 forced me to adopt mbox format.

> Thanks.

thanks

-- 
David J. Biesack, SAS
SAS Campus Dr. Cary, NC 27513
www.sas.com    (919) 531-7771

bug#4448: 23.1; unrmail fails if buffer has mixed line endings (patch)

[Eli Zaretskii]

> Date: Wed, 16 Sep 2009 13:48:41 -0400
> From: "David J. Biesack" <David.Biesack@sas.com>
> CC: 4448@emacsbugs.donarmstrong.com
> 
> If you have to modify the code to detect the situation and generate an more meaningful error message, there should be clear instructions on how to fix it.... which is still not very friendly.

Run it through dos2unix or "flip -u", or even find-file-literally in
Emacs, then remove all ^M characters and save.

bug#4448: 23.1; unrmail fails if buffer has mixed line endings (patch)

[David J. Biesack]

> Date: Wed, 16 Sep 2009 23:33:44 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: <4448@emacsbugs.donarmstrong.com>
> 
> > If you have to modify the code to detect the situation and generate an more meaningful error message, there should be clear instructions on how to fix it.... which is still not very friendly.
> 
> Run it through dos2unix or "flip -u", or even find-file-literally in
> Emacs, then remove all ^M characters and save.

Yes, I know how to convert; I have an elisp function named ^m that does this,
but it's an annoyance and it is unknown how many other users will encounter
this. Again, I assert that if Emacs created rmail files with ^m in them, then 
Emacs should repair them, especially if Emacs forces mbox conversion.

-- 
David J. Biesack, SAS
SAS Campus Dr. Cary, NC 27513
www.sas.com    (919) 531-7771

bug#4448: 23.1; unrmail fails if buffer has mixed line endings (patch)

[Stefan Monnier]

>> > If you have to modify the code to detect the situation and generate
>> > an more meaningful error message, there should be clear
>> > instructions on how to fix it.... which is still not very friendly.
>> Run it through dos2unix or "flip -u", or even find-file-literally in
>> Emacs, then remove all ^M characters and save.
> Yes, I know how to convert; I have an elisp function named ^m that
> does this, but it's an annoyance and it is unknown how many other
> users will encounter this.  Again, I assert that if Emacs created rmail
> files with ^m in them, then  Emacs should repair them, especially if
> Emacs forces mbox conversion.

Agreed.
Of course, maybe the ^Ms were not added by Emacs but by your networked
file system.  Still, if the BABYL file worked fine with Emacs-22,
I think we should try and make it work fine with Emacs-23 as well.


        Stefan

bug#4448: 23.1; unrmail fails if buffer has mixed line endings (patch)

[David J. Biesack]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> CC: <4448@emacsbugs.donarmstrong.com>, Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 17 Sep 2009 10:06:05 -0400
> 
> Agreed.
> Of course, maybe the ^Ms were not added by Emacs but by your networked
> file system.  

I highly doubt that - these files have mixed line endings:
the first n messages have no ^m; then remaining messages
have ^m on all lines, and this changed only on message boundaries.
(I've not inspected closely to see if any file has more
than one line ending switch.)

-- 
David J. Biesack, SAS
SAS Campus Dr. Cary, NC 27513
www.sas.com    (919) 531-7771