buffer-swap-text and multibyteness

buffer-swap-text and multibyteness

[Eli Zaretskii]

set-buffer-multibyte jumps through the hoops to DTRT.

By contrast, in buffer-swap-text we do just this:

  swapfield (enable_multibyte_characters, Lisp_Object);

Could this cause trouble, e.g. if one of the buffers whose text is
swapped contains eight-bit characters?

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

> set-buffer-multibyte jumps through the hoops to DTRT.
> By contrast, in buffer-swap-text we do just this:

>   swapfield (enable_multibyte_characters, Lisp_Object);

> Could this cause trouble, e.g. if one of the buffers whose text is
> swapped contains eight-bit characters?

Why would it?  The text is swapped along with the
enable_multibyte_characters field, so they should stay consistent.
At least that was my reasoning when writing this code.
Basically, enable_multibyte_characters should be stored in the
buffer_text structure rather than in the buffer itself.


        Stefan

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Kenichi Handa <handa@m17n.org>,  emacs-devel@gnu.org
> Date: Sat, 31 Jan 2009 12:12:55 -0500
> 
> > set-buffer-multibyte jumps through the hoops to DTRT.
> > By contrast, in buffer-swap-text we do just this:
> 
> >   swapfield (enable_multibyte_characters, Lisp_Object);
> 
> > Could this cause trouble, e.g. if one of the buffers whose text is
> > swapped contains eight-bit characters?
> 
> Why would it?  The text is swapped along with the
> enable_multibyte_characters field, so they should stay consistent.

I don't know, perhaps I was just confused.  It's hard not to be, with
all that mind-boggling swapping that goes on in Rmail between a
unibyte buffer and a multibyte one.  A horrible design, if you ask me.
It's little surprise to me that I found so many bugs in Rmail due to
this (and more are still there): one needs constantly to track the
current state of the ``swappiness'', or else all bets are off.
(Quick: if you are currently in rmail-view-buffer, and
rmail-buffers-swapped-p returns non-nil, is the text unibyte or
multibyte?)

We should have simply used 2 separate buffers.

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> > set-buffer-multibyte jumps through the hoops to DTRT.
>> > By contrast, in buffer-swap-text we do just this:
>> 
>> >   swapfield (enable_multibyte_characters, Lisp_Object);
>> 
>> > Could this cause trouble, e.g. if one of the buffers whose text is
>> > swapped contains eight-bit characters?
>> 
>> Why would it?  The text is swapped along with the
>> enable_multibyte_characters field, so they should stay consistent.

> I don't know, perhaps I was just confused.  It's hard not to be, with
> all that mind-boggling swapping that goes on in Rmail between a
> unibyte buffer and a multibyte one.  A horrible design, if you ask me.
> It's little surprise to me that I found so many bugs in Rmail due to
> this (and more are still there): one needs constantly to track the
> current state of the ``swappiness'', or else all bets are off.
> (Quick: if you are currently in rmail-view-buffer, and
> rmail-buffers-swapped-p returns non-nil, is the text unibyte or
> multibyte?)

I haven't looked at the code recently, but it does sound like an
undesirable state of affairs, indeed.  In tar-mode (and the way I imagine
buffer-swap-text to be used), swapping the buffer is a rare operation
that's basically only done when the the major mode changes.


        Stefan

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: handa@m17n.org,  emacs-devel@gnu.org
> Date: Sat, 31 Jan 2009 14:43:37 -0500
> 
> In tar-mode (and the way I imagine buffer-swap-text to be used),
> swapping the buffer is a rare operation that's basically only done
> when the the major mode changes.

In Rmail, we do it each time a new message is being displayed.

Just imagine what it takes to implement rmail-edit with this
mind-swapping design (which is probably the main reason why it
currently seems to work only with plain-ASCII message bodies).

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> In tar-mode (and the way I imagine buffer-swap-text to be used),
>> swapping the buffer is a rare operation that's basically only done
>> when the the major mode changes.

> In Rmail, we do it each time a new message is being displayed.

Why???


        Stefan

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> X-Spam-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,
> 	FORGED_RCVD_HELO,SPF_SOFTFAIL autolearn=no version=3.1.0
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Sat, 31 Jan 2009 16:28:59 -0500
> 
> >> In tar-mode (and the way I imagine buffer-swap-text to be used),
> >> swapping the buffer is a rare operation that's basically only done
> >> when the the major mode changes.
> 
> > In Rmail, we do it each time a new message is being displayed.
> 
> Why???

Because each message needs to be decoded before it is shown.  The
decoded text goes to the rmail-view-buffer, and we then call
rmail-swap-buffers to show the decoded text in the rmail-buffer.  See
rmail-show-message for the gory details.

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> >> In tar-mode (and the way I imagine buffer-swap-text to be used),
>> >> swapping the buffer is a rare operation that's basically only done
>> >> when the the major mode changes.
>> 
>> > In Rmail, we do it each time a new message is being displayed.
>> 
>> Why???

> Because each message needs to be decoded before it is shown.  The
> decoded text goes to the rmail-view-buffer, and we then call
> rmail-swap-buffers to show the decoded text in the rmail-buffer.  See
> rmail-show-message for the gory details.

It looks problematic indeed.  I'm having trouble understanding how
it works.  Part of it might be due to naming: IIUC rmail-view-buffer
doesn't actually hold the "view" buffer, but instead it holds the
mbox bytes.


        Stefan

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Sat, 31 Jan 2009 20:43:57 -0500
> 
> It looks problematic indeed.  I'm having trouble understanding how
> it works.  Part of it might be due to naming: IIUC rmail-view-buffer
> doesn't actually hold the "view" buffer, but instead it holds the
> mbox bytes.

Yes, most of the time rmail-view-buffer holds the original message
collection, because that's what rmail-swap-buffers causes when we
display a message.

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> It looks problematic indeed.  I'm having trouble understanding how
>> it works.  Part of it might be due to naming: IIUC rmail-view-buffer
>> doesn't actually hold the "view" buffer, but instead it holds the
>> mbox bytes.

> Yes, most of the time rmail-view-buffer holds the original message
> collection, because that's what rmail-swap-buffers causes when we
> display a message.

Having looked more closely at the code I see that the main problem is
that, in order to keep Rmail's original design, all the buffer-local
vars are local to the `rmail-buffer'.  But now that we use 2 different
buffers, some vars should be local to the `rmail-buffer' and others
local to the other buffer (currently called `rmail-view-buffer').
Since we haven't made such a change, we end up having to swap buffer
texts all the time so that all the work (be it on the view or the data)
is always done within the rmail-buffer.

I think we should rename rmail-view-buffer to rmail-data-buffer, and
move vars like `rmail-message-vector' from rmail-buffer to
rmail-data-buffer.  Then we can stop swapping the two buffers all
the time.

But such changes spell "lots of breakage".


        Stefan

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>     Just imagine what it takes to implement rmail-edit with this
>     mind-swapping design (which is probably the main reason why it
>     currently seems to work only with plain-ASCII message bodies).

> I implemented support for editing non-ASCII messages.  If it fails,
> please send a bug report with a test case.

I don't have a test case, but would propose a completely untested patch
(see below).  The reason is:

- narrow should take place before extracting the headers.
- insert-buffer-substring from a multibyte buffer to a unibyte buffer
  (as here), has tricky semantics.  Better do the encoding at that
  point, so we're going from multibyte non-encoded text to unibyte
  encoded text, both of which are well understood.
- text encoding should take place before the QP/base64 encoding.


        Stefan


Index: lisp/mail/rmailedit.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mail/rmailedit.el,v
retrieving revision 1.47
diff -u -r1.47 rmailedit.el
--- lisp/mail/rmailedit.el      1 Feb 2009 03:26:20 -0000       1.47
+++ lisp/mail/rmailedit.el      2 Feb 2009 01:23:45 -0000
@@ -143,7 +143,10 @@
       (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))
 
       (setq character-coding (mail-fetch-field "content-transfer-encoding")
            is-text-message (rmail-is-text-p)
@@ -151,28 +154,25 @@
       (if character-coding
          (setq character-coding (downcase character-coding)))
 
-      (narrow-to-region (rmail-msgbeg rmail-current-message)
-                       (rmail-msgend rmail-current-message))
       (goto-char (point-min))
       (search-forward "\n\n")
-      (let ((inhibit-read-only t)
-           (headers-end-1 (point)))
-       (insert-buffer-substring rmail-view-buffer headers-end)
-       (delete-region (point) (point-max))
+      (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)))
 
        ;; Re-encode the message body in whatever
        ;; way it was decoded.
        (cond
         ((string= character-coding "quoted-printable")
-         (mail-quote-printable-region headers-end-1 (point-max)))
+         (mail-quote-printable-region (point) (point-max)))
         ((and (string= character-coding "base64") is-text-message)
-         (base64-encode-region headers-end-1 (point-max)))
+         (base64-encode-region (point) (point-max)))
         ((eq character-coding 'uuencode)
-         (error "Not supported yet."))
-        (t
-         (if (or (not coding-system) (not (coding-system-p coding-system)))
-             (setq coding-system 'undecided))
-         (encode-coding-region headers-end-1 (point-max) coding-system)))
+         (error "Not supported yet.")))
        ))
 
     (rmail-set-attribute rmail-edited-attr-index t)

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <E1LTl7l-0005Dm-8W@fencepost.gnu.org>, Richard M Stallman <rms@gnu.org> writes:

>     Just imagine what it takes to implement rmail-edit with this
>     mind-swapping design (which is probably the main reason why it
>     currently seems to work only with plain-ASCII message bodies).

> I implemented support for editing non-ASCII messages.  If it fails,
> please send a bug report with a test case.

It always fails.  For instance, this mail contains signle
A-grave here:

À

Please edit this message after typing e
(rmail-edit-current-message) and type C-c C-c.  The above
line is changed to \300 (byte #xC0).

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

Sorry, the previous mail went out with the wrong encoding.
I resent it with UTF-8.

In article <E1LTl7l-0005Dm-8W@fencepost.gnu.org>, Richard M Stallman <rms@gnu.org> writes:
>     Just imagine what it takes to implement rmail-edit with this
>     mind-swapping design (which is probably the main reason why it
>     currently seems to work only with plain-ASCII message bodies).

> I implemented support for editing non-ASCII messages.  If it fails,
> please send a bug report with a test case.

It always fails.  For instance, this mail contains signle
A-grave here:

À

Please edit this message after typing e
(rmail-edit-current-message) and type C-c C-c.  The above
line is changed to \300 (byte #xC0).

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

> It always fails.  For instance, this mail contains signle
> A-grave here:

> À

> Please edit this message after typing e
> (rmail-edit-current-message) and type C-c C-c.  The above
> line is changed to \300 (byte #xC0).

Have you tried the patch I sent (and which I re-appended below)?


        Stefan


- narrow should take place before extracting the headers.
- insert-buffer-substring from a multibyte buffer to a unibyte buffer
  (as here), has tricky semantics.  Better do the encoding at that
  point, so we're going from multibyte non-encoded text to unibyte
  encoded text, both of which are well understood.
- text encoding should take place before the QP/base64 encoding.


Index: lisp/mail/rmailedit.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mail/rmailedit.el,v
retrieving revision 1.47
diff -u -r1.47 rmailedit.el
--- lisp/mail/rmailedit.el      1 Feb 2009 03:26:20 -0000       1.47
+++ lisp/mail/rmailedit.el      2 Feb 2009 01:23:45 -0000
@@ -143,7 +143,10 @@
       (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))
 
       (setq character-coding (mail-fetch-field "content-transfer-encoding")
            is-text-message (rmail-is-text-p)
@@ -151,28 +154,25 @@
       (if character-coding
          (setq character-coding (downcase character-coding)))
 
-      (narrow-to-region (rmail-msgbeg rmail-current-message)
-                       (rmail-msgend rmail-current-message))
       (goto-char (point-min))
       (search-forward "\n\n")
-      (let ((inhibit-read-only t)
-           (headers-end-1 (point)))
-       (insert-buffer-substring rmail-view-buffer headers-end)
-       (delete-region (point) (point-max))
+      (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)))
 
        ;; Re-encode the message body in whatever
        ;; way it was decoded.
        (cond
         ((string= character-coding "quoted-printable")
-         (mail-quote-printable-region headers-end-1 (point-max)))
+         (mail-quote-printable-region (point) (point-max)))
         ((and (string= character-coding "base64") is-text-message)
-         (base64-encode-region headers-end-1 (point-max)))
+         (base64-encode-region (point) (point-max)))
         ((eq character-coding 'uuencode)
-         (error "Not supported yet."))
-        (t
-         (if (or (not coding-system) (not (coding-system-p coding-system)))
-             (setq coding-system 'undecided))
-         (encode-coding-region headers-end-1 (point-max) coding-system)))
+         (error "Not supported yet.")))
        ))
 
     (rmail-set-attribute rmail-edited-attr-index t)

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: rms@gnu.org,  eliz@gnu.org,  emacs-devel@gnu.org
> Date: Wed, 04 Feb 2009 23:26:12 -0500
> 
> > It always fails.  For instance, this mail contains signle
> > A-grave here:
> 
> > À
> 
> > Please edit this message after typing e
> > (rmail-edit-current-message) and type C-c C-c.  The above
> > line is changed to \300 (byte #xC0).
> 
> Have you tried the patch I sent (and which I re-appended below)?

Your patch fixes rmail-edit with non-ASCII characters, thanks.  I
installed it.

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <jwvocxh1pg1.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > À

> > Please edit this message after typing e
> > (rmail-edit-current-message) and type C-c C-c.  The above
> > line is changed to \300 (byte #xC0).

> Have you tried the patch I sent (and which I re-appended below)?

I've just tried the latest CVS code that has you patch.  It
seems it works well in a usual case.   But, for instance,
one edits a mail of:
   Content-type: text/plain; charset=iso-8859-1
and inserts a non-latin-1 character, we must use the
different encoding (utf-8 or the one selected by
select-message-coding-system), and modify content-type:
header.

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> > Please edit this message after typing e
>> > (rmail-edit-current-message) and type C-c C-c.  The above
>> > line is changed to \300 (byte #xC0).

>> Have you tried the patch I sent (and which I re-appended below)?

> I've just tried the latest CVS code that has you patch.  It
> seems it works well in a usual case.   But, for instance,
> one edits a mail of:
>    Content-type: text/plain; charset=iso-8859-1
> and inserts a non-latin-1 character, we must use the
> different encoding (utf-8 or the one selected by
> select-message-coding-system), and modify content-type:
> header.

Yes, the code doesn't try to handle these problems.  In my poiunt of
view, the way "edit" should work is that it should basically be put into
`message-mode' and then "saving" would simply encode the whole message
(as if preparing to send it).  To do that, we could/should use the code
we already have for Gnus.


        Stefan

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: eliz@gnu.org,  rms@gnu.org,  emacs-devel@gnu.org
> Date: Mon, 09 Feb 2009 09:11:22 -0500
> 
> In my poiunt of view, the way "edit" should work is that it should
> basically be put into `message-mode' and then "saving" would simply
> encode the whole message (as if preparing to send it).

I'm not sure this isn't an overkill.

> To do that, we could/should use the code we already have for Gnus.

Similar code exists in sendmail.el and smtp-mail.el.

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> In my poiunt of view, the way "edit" should work is that it should
>> basically be put into `message-mode' and then "saving" would simply
>> encode the whole message (as if preparing to send it).
> I'm not sure this isn't an overkill.

Not sure why you'd think so.

>> To do that, we could/should use the code we already have for Gnus.
> Similar code exists in sendmail.el and smtp-mail.el.

Great.  We probably don't need to another yet-another implementation,
then, do we?


        Stefan

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: handa@m17n.org,  rms@gnu.org,  emacs-devel@gnu.org
> Date: Mon, 09 Feb 2009 21:03:41 -0500
> 
> >> In my poiunt of view, the way "edit" should work is that it should
> >> basically be put into `message-mode' and then "saving" would simply
> >> encode the whole message (as if preparing to send it).
> > I'm not sure this isn't an overkill.
> 
> Not sure why you'd think so.

When did you last look at the code you were describing?  It does a lot
more then just encode the message and add/rewrite the Content-Type
headers.

> >> To do that, we could/should use the code we already have for Gnus.
> > Similar code exists in sendmail.el and smtp-mail.el.
> 
> Great.  We probably don't need to another yet-another implementation,
> then, do we?

We don't, but IIRC the code each of these places has is tightly
coupled with other package-specific processing.  If we want a single
general-purpose one, we need first to refactor it.  Again, please take
a look at each of these packages.

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> >> In my poiunt of view, the way "edit" should work is that it should
>> >> basically be put into `message-mode' and then "saving" would simply
>> >> encode the whole message (as if preparing to send it).
>> > I'm not sure this isn't an overkill.
>> 
>> Not sure why you'd think so.

> When did you last look at the code you were describing?  It does a lot
> more then just encode the message and add/rewrite the Content-Type
> headers.

But... why do you think this "a lot more" is not needed/useful in the
case of rmailedit?

> We don't, but IIRC the code each of these places has is tightly
> coupled with other package-specific processing.  If we want a single
> general-purpose one, we need first to refactor it.  Again, please take
> a look at each of these packages.

Yes, I'm aware of this problem.  Some refactoring will be needed.
But such a refactoring would be a good thing.  Whereas adding another
implementation would just not benefit anyone else.


        Stefan

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: handa@m17n.org, rms@gnu.org, emacs-devel@gnu.org
> Date: Tue, 10 Feb 2009 16:57:26 -0500
> 
> >> >> In my poiunt of view, the way "edit" should work is that it should
> >> >> basically be put into `message-mode' and then "saving" would simply
> >> >> encode the whole message (as if preparing to send it).
> >> > I'm not sure this isn't an overkill.
> >> 
> >> Not sure why you'd think so.
> 
> > When did you last look at the code you were describing?  It does a lot
> > more then just encode the message and add/rewrite the Content-Type
> > headers.
> 
> But... why do you think this "a lot more" is not needed/useful in the
> case of rmailedit?

Because rmailedit never sends any message, so it does not need most of
that stuff.

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> >> >> In my poiunt of view, the way "edit" should work is that it should
>> >> >> basically be put into `message-mode' and then "saving" would simply
>> >> >> encode the whole message (as if preparing to send it).
>> >> > I'm not sure this isn't an overkill.
>> >> 
>> >> Not sure why you'd think so.
>> 
>> > When did you last look at the code you were describing?  It does a lot
>> > more then just encode the message and add/rewrite the Content-Type
>> > headers.
>> 
>> But... why do you think this "a lot more" is not needed/useful in the
>> case of rmailedit?

> Because rmailedit never sends any message, so it does not need most of
> that stuff.

I still don't know what you're referring to.  What does it do extra that
rmailedit.el wouldn't need?


        Stefan

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <jwvhc33gih3.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> To do that, we could/should use the code we already have for Gnus.
> > Similar code exists in sendmail.el and smtp-mail.el.

> Great.  We probably don't need to another yet-another implementation,
> then, do we?

I've just found rmailmm.el.  It seems that it implements
MIME handling for rmail.  Does it still work with the
current rmail?

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <umycvs6a0.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> > From: Kenichi Handa <handa@m17n.org>
> > CC: eliz@gnu.org, rms@gnu.org, emacs-devel@gnu.org
> > Date: Mon, 09 Feb 2009 20:31:00 +0900
> > 
> > But, for instance, one edits a mail of:
> >    Content-type: text/plain; charset=iso-8859-1
> > and inserts a non-latin-1 character, we must use the
> > different encoding (utf-8 or the one selected by
> > select-message-coding-system), and modify content-type:
> > header.

> Did you try rmail-redecode-body after editing?

No, but it won't help because when I type C-c C-c after
editing, the unencodable characters disappear from the
buffer " *message-viewer RMAIL*".

> Anyway, I don't think the Babyl Rmail handled such a use case
> correctly, either.

No, it used to work because rmail-cease-edit didn't have to
encode the message.

> Why would one wish to insert a character into a
> message that cannot be encoded in the message's charset?  What would
> be the reason for such strange editing?

I got a Japanese mail with attachment file containing many
non-Japanese characters encoded by emacs-mule.  I editted
the mail, decoded the attached file by hand by the
combination of base64-decode-region and
decode-coding-region, and typed C-c C-c.

> All the use cases I can think of should be covered by
> rmail-redecode-body, which is The Right Way of changing
> the encoding of a message.

It works only if the unencodable characters is not lost in
the rmail file buffer.

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Kenichi Handa <handa@m17n.org>
> CC: emacs-devel@gnu.org, monnier@iro.umontreal.ca, rms@gnu.org
> Date: Tue, 10 Feb 2009 17:17:12 +0900
> 
> > Did you try rmail-redecode-body after editing?
> 
> No, but it won't help because when I type C-c C-c after
> editing, the unencodable characters disappear from the
> buffer " *message-viewer RMAIL*".

Do you have an idea why they disappeared?

> I got a Japanese mail with attachment file containing many
> non-Japanese characters encoded by emacs-mule.  I editted
> the mail, decoded the attached file by hand by the
> combination of base64-decode-region and
> decode-coding-region, and typed C-c C-c.

I do this a lot, but I don't use decode-coding-region.  I use
base64-decode-region, then type "C-c C-c" followed by "M-x
rmail-redecode-body RET".  This did work for me after the last series
of changes in Rmail.  If it doesn't work for you with the current CVS,
can you somehow forward to me the original message, before your
editing, so I could try reproducing what you did?

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <u8woesljv.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> > > Did you try rmail-redecode-body after editing?
> > 
> > No, but it won't help because when I type C-c C-c after
> > editing, the unencodable characters disappear from the
> > buffer " *message-viewer RMAIL*".

> Do you have an idea why they disappeared?

It's because the encoding routine produces a default
character for such an unencodable character.  Raising an
error signal for such a case in the deep place of encoding
routine is in my todo list and not yet implemented.

> > I got a Japanese mail with attachment file containing many
> > non-Japanese characters encoded by emacs-mule.  I editted
> > the mail, decoded the attached file by hand by the
> > combination of base64-decode-region and
> > decode-coding-region, and typed C-c C-c.

> I do this a lot, but I don't use decode-coding-region.  I use
> base64-decode-region, then type "C-c C-c" followed by "M-x
> rmail-redecode-body RET".  This did work for me after the last series
> of changes in Rmail.  If it doesn't work for you with the current CVS,
> can you somehow forward to me the original message, before your
> editing, so I could try reproducing what you did?

rmail-redecode-body works only when the whole message body
is encoded by a single coding system, which is not the case
in my scenario above.

Please try your method with this mail.

Here, I enter Japanese "Hello":
   こんにちは

And, here I enter base64-encoded Latin-1 text:

ICAgICBBeCCgIKEgoiCjIKQgpSCmIKcgqCCpIKogqyCsIK0griCvCiAgICAgQnggsCCxILIgsyC0
ILUgtiC3ILgguSC6ILsgvCC9IL4gvwogICAgIEN4IMAgwSDCIMMgxCDFIMYgxyDIIMkgyiDLIMwg
zSDOIM8KICAgICBEeCDQINEg0iDTINQg1SDWINcg2CDZINog2yDcIN0g3iDfCiAgICAgRXgg4CDh
IOIg4yDkIOUg5iDnIOgg6SDqIOsg7CDtIO4g7wogICAgIEZ4IPAg8SDyIPMg9CD1IPYg9yD4IPkg
+iD7IPwg/SD+IP8K

And, I send this message itself using charset=iso-2022-jp.

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Kenichi Handa <handa@m17n.org>
> CC: monnier@iro.umontreal.ca, rms@gnu.org, emacs-devel@gnu.org
> Date: Tue, 10 Feb 2009 20:52:00 +0900
> 
> Please try your method with this mail.
> 
> Here, I enter Japanese "Hello":
>    こんにちは
> 
> And, here I enter base64-encoded Latin-1 text:
> 
> ICAgICBBeCCgIKEgoiCjIKQgpSCmIKcgqCCpIKogqyCsIK0griCvCiAgICAgQnggsCCxILIgsyC0
> ILUgtiC3ILgguSC6ILsgvCC9IL4gvwogICAgIEN4IMAgwSDCIMMgxCDFIMYgxyDIIMkgyiDLIMwg
> zSDOIM8KICAgICBEeCDQINEg0iDTINQg1SDWINcg2CDZINog2yDcIN0g3iDfCiAgICAgRXgg4CDh
> IOIg4yDkIOUg5iDnIOgg6SDqIOsg7CDtIO4g7wogICAgIEZ4IPAg8SDyIPMg9CD1IPYg9yD4IPkg
> +iD7IPwg/SD+IP8K
> 
> And, I send this message itself using charset=iso-2022-jp.

You are right, rmail-cease-edit was too naive in selecting the correct
encoding.

I think I fixed your use-case now, please try.  After base64 decoding
the Latin-1 text and manually decoding the text with
decode-coding-region, if buffer-file-coding-system cannot safely
encode the message, Emacs will ask you to select a safe encoding when
you type C-c C-c.  You can also set the correct encoding manually
before you type C-c C-c.  The charset= header is rewritten as needed,
as part of putting the modified message back into the message
collection buffer.

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <uljrzu8ms.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> You are right, rmail-cease-edit was too naive in selecting the correct
> encoding.

> I think I fixed your use-case now, please try.  After base64 decoding
> the Latin-1 text and manually decoding the text with
> decode-coding-region, if buffer-file-coding-system cannot safely
> encode the message, Emacs will ask you to select a safe encoding when
> you type C-c C-c.  You can also set the correct encoding manually
> before you type C-c C-c.  The charset= header is rewritten as needed,
> as part of putting the modified message back into the message
> collection buffer.

Thank you, I confirmed that this problem is fixed.

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Kenichi Handa <handa@m17n.org>
> CC: emacs-devel@gnu.org, monnier@iro.umontreal.ca, rms@gnu.org
> Date: Tue, 24 Feb 2009 17:09:45 +0900
> 
> In article <uljrzu8ms.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
> 
> > You are right, rmail-cease-edit was too naive in selecting the correct
> > encoding.
> 
> > I think I fixed your use-case now, please try.  After base64 decoding
> > the Latin-1 text and manually decoding the text with
> > decode-coding-region, if buffer-file-coding-system cannot safely
> > encode the message, Emacs will ask you to select a safe encoding when
> > you type C-c C-c.  You can also set the correct encoding manually
> > before you type C-c C-c.  The charset= header is rewritten as needed,
> > as part of putting the modified message back into the message
> > collection buffer.
> 
> Thank you, I confirmed that this problem is fixed.

Thanks for testing it.

Btw, while I worked on this, I discovered a strange problem with a
coding-system I never heard about before: iso-2022-jp-2.  (It was one
of the encodings select-safe-coding-system suggested for encoding the
text with both Japanese and Latin-1 characters.)  Whenever I used
iso-2022-jp-2 to encode the edited message back into the mbox buffer,
Emacs crashed inside GC.  Is something wrong with that coding-system?

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <ueixnr7rv.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> Btw, while I worked on this, I discovered a strange problem with a
> coding-system I never heard about before: iso-2022-jp-2.  (It was one
> of the encodings select-safe-coding-system suggested for encoding the
> text with both Japanese and Latin-1 characters.)  Whenever I used
> iso-2022-jp-2 to encode the edited message back into the mbox buffer,
> Emacs crashed inside GC.  Is something wrong with that coding-system?

Perhaps, but I can't reproduce it with this simple test:

(decode-coding-string (encode-coding-string "abc漢字ÀÁ" 'iso-2022-jp-2)
		      'iso-2022-jp-2)

Please give me a test case.

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Kenichi Handa <handa@m17n.org>
> CC: monnier@iro.umontreal.ca, rms@gnu.org, emacs-devel@gnu.org
> Date: Wed, 25 Feb 2009 10:15:15 +0900
> 
> In article <ueixnr7rv.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Btw, while I worked on this, I discovered a strange problem with a
> > coding-system I never heard about before: iso-2022-jp-2.  (It was one
> > of the encodings select-safe-coding-system suggested for encoding the
> > text with both Japanese and Latin-1 characters.)  Whenever I used
> > iso-2022-jp-2 to encode the edited message back into the mbox buffer,
> > Emacs crashed inside GC.  Is something wrong with that coding-system?
> 
> Perhaps, but I can't reproduce it with this simple test:
> 
> (decode-coding-string (encode-coding-string "abc漢字ÀÁ" 'iso-2022-jp-2)
> 		      'iso-2022-jp-2)
> 
> Please give me a test case.

It was the same test case you sent to me earlier in this thread.

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <uab8bqhof.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> > From: Kenichi Handa <handa@m17n.org>
> > CC: monnier@iro.umontreal.ca, rms@gnu.org, emacs-devel@gnu.org
> > Date: Wed, 25 Feb 2009 10:15:15 +0900
> > 
> > In article <ueixnr7rv.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > Btw, while I worked on this, I discovered a strange problem with a
> > > coding-system I never heard about before: iso-2022-jp-2.  (It was one
> > > of the encodings select-safe-coding-system suggested for encoding the
> > > text with both Japanese and Latin-1 characters.)  Whenever I used
> > > iso-2022-jp-2 to encode the edited message back into the mbox buffer,
> > > Emacs crashed inside GC.  Is something wrong with that coding-system?
> > 
> > Perhaps, but I can't reproduce it with this simple test:
> > 
> > (decode-coding-string (encode-coding-string "abc漢字ÀÁ" 'iso-2022-jp-2)
> > 		      'iso-2022-jp-2)
> > 
> > Please give me a test case.

> It was the same test case you sent to me earlier in this thread.

I tried with that mail, but Emacs encoded and decoded the
contents without crashing even if I selected iso-2022-jp-2.

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Eli Zaretskii]

> From: Richard M Stallman <rms@gnu.org>
> CC: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> Date: Sun, 01 Feb 2009 17:49:09 -0500
> 
>     Just imagine what it takes to implement rmail-edit with this
>     mind-swapping design (which is probably the main reason why it
>     currently seems to work only with plain-ASCII message bodies).
> 
> I implemented support for editing non-ASCII messages.  If it fails,
> please send a bug report with a test case.

I did (bugs 2015 and 2017).  I was in the middle of working on them
last weekend when I ran out of time; I intend to continue next
weekend, which is why I didn't submit more test cases.  Even after all
the fixes I did in both rmail and unrmail, some non-ASCII messages are
not displayed correctly after conversion from BABYL and/or
rmail-edit.  Strangely, it seems to depend on the encoding of the
original message somehow.  I suspect it has something to do with the
fact that one of the two buffers being swapped is a unibyte buffer,
while the other is multibyte, and that we set
buffer-file-coding-system of the buffers according to the message that
is being displayed.

Re: buffer-swap-text and multibyteness

[Richard M Stallman]

    I haven't looked at the code recently, but it does sound like an
    undesirable state of affairs, indeed.  In tar-mode (and the way I imagine
    buffer-swap-text to be used), swapping the buffer is a rare operation
    that's basically only done when the the major mode changes.

Rmail swaps the buffers twice every time you switch messages.
A number of other commands also temporarily unswap.

Before I worked on it, several other commands and functions used to unswap,
but I altered them to operate on the proper buffer without altering the
arrangements.  Perhaps similar changes can be applied elsewhere.

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <ubptne1rl.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> I don't know, perhaps I was just confused.  It's hard not to be, with
> all that mind-boggling swapping that goes on in Rmail between a
> unibyte buffer and a multibyte one.  A horrible design, if you ask me.
> It's little surprise to me that I found so many bugs in Rmail due to
> this (and more are still there): one needs constantly to track the
> current state of the ``swappiness'', or else all bets are off.
> (Quick: if you are currently in rmail-view-buffer, and
> rmail-buffers-swapped-p returns non-nil, is the text unibyte or
> multibyte?)

> We should have simply used 2 separate buffers.

I think rmail-buffer and rmail-view-buffer should be set
consistently to the unibyte buffer containing RMAIL file and
to the multibyte buffer showing one message to a user.

And, most of the interactive functions should just do
something like this:

  (set-buffer rmail-buffer)
  ...
  (switch-to-buffer rmail-view-buffer)

Is the conbination of set-buffer and switch-to-buffer
heavier than two buffer-swap-text?

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

> I think rmail-buffer and rmail-view-buffer should be set
> consistently to the unibyte buffer containing RMAIL file and
> to the multibyte buffer showing one message to a user.

> And, most of the interactive functions should just do
> something like this:

>   (set-buffer rmail-buffer)
>   ...
>   (switch-to-buffer rmail-view-buffer)

> Is the conbination of set-buffer and switch-to-buffer
> heavier than two buffer-swap-text?

Yes. we agree (except that switch-to-buffer is a function I would
prefer to see disapppear, so better use something else like
pop-to-buffer or just set-buffer).

Also, I'd prefer to use `rmail-buffer' for the multibyte buffer that's
displayed, and rmail-data-buffer for the raw unibyte mbox.


        Stefan

Re: buffer-swap-text and multibyteness

[Kenichi Handa]

In article <jwvpri14l9n.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > I think rmail-buffer and rmail-view-buffer should be set
> > consistently to the unibyte buffer containing RMAIL file and
> > to the multibyte buffer showing one message to a user.

> > And, most of the interactive functions should just do
> > something like this:

> >   (set-buffer rmail-buffer)
> >   ...
> >   (switch-to-buffer rmail-view-buffer)

> > Is the conbination of set-buffer and switch-to-buffer
> > heavier than two buffer-swap-text?

> Yes. we agree (except that switch-to-buffer is a function I would
> prefer to see disapppear, so better use something else like
> pop-to-buffer or just set-buffer).

Is above "Yes" also to this question?

> > Is the conbination of set-buffer and switch-to-buffer
> > heavier than two buffer-swap-text?

By the way,

> Also, I'd prefer to use `rmail-buffer' for the multibyte buffer that's
> displayed, and rmail-data-buffer for the raw unibyte mbox.

I don't have strong preference for those variable names as
far as their setting doesn't change by swapping.

---
Kenichi Handa
handa@m17n.org

Re: buffer-swap-text and multibyteness

[Stefan Monnier]

>> Yes. we agree (except that switch-to-buffer is a function I would
>> prefer to see disapppear, so better use something else like
>> pop-to-buffer or just set-buffer).

> Is above "Yes" also to this question?

>> > Is the conbination of set-buffer and switch-to-buffer
>> > heavier than two buffer-swap-text?

buffer-swap-text does not meddle with window<->buffer association, so
the equivalent is just `set-buffer'.  I.e. we'd just use

  (with-current-buffer rmail-data-buffer
    ...)

which is a lot lighter weight than a pair of buffer-swap-text.


        Stefan