Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> +    (message "Warning: Using brain-dead macro `mm-with-unibyte-current-buffer'!")

Looking at the macro, it looks like it tries to ensure that the buffer
is unibyte, executes the form, and then returns the buffer to its
previous byteness state?  (In a way that works on both Emacs and XEmacs
(well, it's a noop in the latter.)

Looking at the callers, it seems like that's what they want, although
I'm not sure.  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

[Stefan Monnier]

> Looking at the macro, it looks like it tries to ensure that the buffer
> is unibyte, executes the form, and then returns the buffer to its
> previous byteness state?

Right, and this makes no sense.  As a general rule the unibyte/multibyte
state of a buffer should only be changed when the buffer is empty.
Anything else is just a gross hack that will sooner or later bite you.

> Looking at the callers, it seems like that's what they want, although
> I'm not sure.

The problem is that "change the buffer to uni/multibyte" doesn't itself
have a clear meaning, so yes: it's not clear what they want *because*
they use this macro.

It'll have to be fixed by hand, one by one, either by understanding
better what the code needs, or if that's too hard, by trial-and-error
(and adding corresponding tests).


        Stefan

Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Looking at the callers, it seems like that's what they want, although
>> I'm not sure.
>
> The problem is that "change the buffer to uni/multibyte" doesn't itself
> have a clear meaning, so yes: it's not clear what they want *because*
> they use this macro.

Doesn't switching to unibyte just mean "expose the raw utf-8-ish
contents as bytes", sort of?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

[Stefan Monnier]

> Doesn't switching to unibyte just mean "expose the raw utf-8-ish
> contents as bytes", sort of?

The keyword here is "sort of".  And of course, the behavior was
different in Emacs-22, yet most/all of this code dates back to before
Emacs-23, so that makes it even more murky.

Conversions between bytes ("unibyte") and characters ("multibyte")
should be done by encoding/decoding.  Using terms like "converting from
unibyte to multibyte" just messes up with your head to make sure you
can't think straight.


        Stefan

Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> The keyword here is "sort of".  And of course, the behavior was
> different in Emacs-22, yet most/all of this code dates back to before
> Emacs-23, so that makes it even more murky.

Indeed.

> Conversions between bytes ("unibyte") and characters ("multibyte")
> should be done by encoding/decoding.  Using terms like "converting from
> unibyte to multibyte" just messes up with your head to make sure you
> can't think straight.

Yes.

Here's the first usage grep shows:

(defun utf7-fragment-encode (start end &optional for-imap)
  "Encode text from START to END in buffer as UTF-7 escape fragment.
Use IMAP modification if FOR-IMAP is non-nil."
  (save-restriction
    (narrow-to-region start end)
    (funcall (utf7-get-u16char-converter 'to-utf-16))
    (mm-with-unibyte-current-buffer
      (base64-encode-region start (point-max)))
    (goto-char start)
    (let ((pm (point-max)))
      (when for-imap
	(while (search-forward "/" nil t)
	  (replace-match ",")))
      (skip-chars-forward "^= \t\n" pm)
      (delete-region (point) pm))))

So we have some non-ASCII text in the buffer, and er, we convert that to
utf-16?  And then we convert the utf-16 represented as utf-8 byte
sequences to base64?  Uhm.

Yeah, OK, `mm-with-unibyte-current-buffer' is probably not helping with
understanding that function any.  :-)  So if somebody would be so kind
to rewrite these functions, that would be nice.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 16 Jan 2015 00:12:39 +0100
> Cc: emacs-devel@gnu.org
> 
> Doesn't switching to unibyte just mean "expose the raw utf-8-ish
> contents as bytes", sort of?

No, it does much more than that, even if you don't use the 'to' value
of the flag.  Look at the sources, and you will see.

Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 16 Jan 2015 03:22:45 +0100
> Cc: emacs-devel@gnu.org
> 
> (defun utf7-fragment-encode (start end &optional for-imap)
>   "Encode text from START to END in buffer as UTF-7 escape fragment.
> Use IMAP modification if FOR-IMAP is non-nil."
>   (save-restriction
>     (narrow-to-region start end)
>     (funcall (utf7-get-u16char-converter 'to-utf-16))
>     (mm-with-unibyte-current-buffer
>       (base64-encode-region start (point-max)))
>     (goto-char start)
>     (let ((pm (point-max)))
>       (when for-imap
> 	(while (search-forward "/" nil t)
> 	  (replace-match ",")))
>       (skip-chars-forward "^= \t\n" pm)
>       (delete-region (point) pm))))
> 
> So we have some non-ASCII text in the buffer, and er, we convert that to
> utf-16?  And then we convert the utf-16 represented as utf-8 byte
> sequences to base64?  Uhm.

The UTF-16 encoded text is represented as series of 8-bit bytes, which
are themselves represented as UTF-8 sequences outside of the Unicode
range of characters.  There's no special problem here, since Emacs is
quite capable of holding raw bytes in a multibyte buffer.

> Yeah, OK, `mm-with-unibyte-current-buffer' is probably not helping with
> understanding that function any.  :-)  So if somebody would be so kind
> to rewrite these functions, that would be nice.

I would suggest using the optional DESTINATION argument of
encode-coding-region, pointing it at a separate scratch buffer that
was made unibyte when it was created.  Thgen base64-encode-region
could be run on that buffer without the mm-with-unibyte-current-buffer
stuff.

Re: master 8f03888: * lisp/gnus/gnus-art.el: Fix up compiler warnings.

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I would suggest using the optional DESTINATION argument of
> encode-coding-region, pointing it at a separate scratch buffer that
> was made unibyte when it was created.  Thgen base64-encode-region
> could be run on that buffer without the mm-with-unibyte-current-buffer
> stuff.

Hm.  Sounds like a job for a new macro.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no