Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Lars Ingebrigtsen]

larsi@gnus.org (Lars Ingebrigtsen) writes:

>  	(save-excursion
> -	  (if (fboundp 'set-buffer-file-coding-system)
> -	      (set-buffer-file-coding-system 'binary))
> +	  (set-buffer-file-coding-system 'binary)
>  	  (set-buffer buffer)

This seems like "obviously wrong code" (setting the coding system and
then doing `set-buffer'), but I don't use eudc, so ... perhaps it really
wants to set the coding system in the original buffer?

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

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

>>  	(save-excursion
>> -	  (if (fboundp 'set-buffer-file-coding-system)
>> -	      (set-buffer-file-coding-system 'binary))
>> +	  (set-buffer-file-coding-system 'binary)
>>  	  (set-buffer buffer)
>
> This seems like "obviously wrong code" (setting the coding system and
> then doing `set-buffer'), but I don't use eudc, so ... perhaps it really
> wants to set the coding system in the original buffer?

Indeed, and the setting of coding-system can be moved before the
`save-excursion` after which you the byte-compiler will see the
`save-excursion + set-buffer` and recommend to use `with-current-buffer`
instead.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Lars Ingebrigtsen]

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

>> This seems like "obviously wrong code" (setting the coding system and
>> then doing `set-buffer'), but I don't use eudc, so ... perhaps it really
>> wants to set the coding system in the original buffer?
>
> Indeed, and the setting of coding-system can be moved before the
> `save-excursion` after which you the byte-compiler will see the
> `save-excursion + set-buffer` and recommend to use `with-current-buffer`
> instead.

But I think that's not really what's intended...  here's one of the
functions: 

(defun eudc-bob-save-object ()
  "Save the object data of the button at point."
  (interactive)
  (let ((data (eudc-bob-get-overlay-prop 'object-data))
	(buffer (generate-new-buffer "*eudc-tmp*")))
    (save-excursion
      (set-buffer-file-coding-system 'binary)
      (set-buffer buffer)
      (set-buffer-multibyte nil)
      (insert data)
      (save-buffer))
    (kill-buffer buffer)))

I think the intention here is to change the coding system in this new
temporary buffer (before saving), and not whatever buffer the caller of
the function was...

But I don't even know what eudc is, so.  :-)

(And this should probably just be rewritten as a `with-temp-buffer'.)

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

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 13 Aug 2020 10:08:26 +0200
> Cc: emacs-devel@gnu.org
> 
> (defun eudc-bob-save-object ()
>   "Save the object data of the button at point."
>   (interactive)
>   (let ((data (eudc-bob-get-overlay-prop 'object-data))
> 	(buffer (generate-new-buffer "*eudc-tmp*")))
>     (save-excursion
>       (set-buffer-file-coding-system 'binary)
>       (set-buffer buffer)
>       (set-buffer-multibyte nil)
>       (insert data)
>       (save-buffer))
>     (kill-buffer buffer)))
> 
> I think the intention here is to change the coding system in this new
> temporary buffer (before saving), and not whatever buffer the caller of
> the function was...

I think we can simply delete the line that sets
buffer-file-coding-system: in a unibyte buffer it doesn't matter
anyway.  (And it's wrong to use 'binary' for producing files that will
be processed by programs other than Emacs, because 'binary' spills out
the Emacs internal representation of characters.)

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

> I think we can simply delete the line that sets
> buffer-file-coding-system: in a unibyte buffer it doesn't
> matter anyway.

IIRC it can make a difference w.r.t line-endings.

> (And it's wrong to use 'binary' for producing files that will
> be processed by programs other than Emacs, because 'binary' spills out
> the Emacs internal representation of characters.)

It's definitely not wrong for unibyte buffers where there's no "internal
representation of characters" that can spill out ;-)

And AFAIK it's not wrong for multybyte buffers:
- Either the content of the buffer is limited to ascii and eight-bit
  chars in which case the output is exactly right.
- Or there are other chars and there is simply no correct output.
  We should likely signal an error in this case, tho we probably just
  end up exposing the internal utf-8-like encoding instead, indeed.
  Still the error is not necessarily in the use of `binary` in that case
  but in the actual content of the buffer.


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  emacs-devel@gnu.org
> Date: Thu, 13 Aug 2020 09:58:15 -0400
> 
> > I think we can simply delete the line that sets
> > buffer-file-coding-system: in a unibyte buffer it doesn't
> > matter anyway.
> 
> IIRC it can make a difference w.r.t line-endings.

How?  And why does that matter in the case in point?

> > (And it's wrong to use 'binary' for producing files that will
> > be processed by programs other than Emacs, because 'binary' spills out
> > the Emacs internal representation of characters.)
> 
> It's definitely not wrong for unibyte buffers where there's no "internal
> representation of characters" that can spill out ;-)

We cannot know that in general.  Why live dangerously?

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

>> > I think we can simply delete the line that sets
>> > buffer-file-coding-system: in a unibyte buffer it doesn't
>> > matter anyway.
>> IIRC it can make a difference w.r.t line-endings.
> How?

If your global file-coding-system settings use DOS, I guess?

> And why does that matter in the case in point?

No idea, but before removing that code we would have to find out why it
*doesn't* matter.

>> > (And it's wrong to use 'binary' for producing files that will
>> > be processed by programs other than Emacs, because 'binary' spills out
>> > the Emacs internal representation of characters.)
>> It's definitely not wrong for unibyte buffers where there's no "internal
>> representation of characters" that can spill out ;-)
> We cannot know that in general.

Yes, we do: unibyte buffers have no "internal representation of
characters" that can spill out.

> Why live dangerously?

To me `binary` is *the* coding-system to use when you have bytes and
want to write them in a file (and same when you want to read the *bytes*
of a file into a buffer).
Do you know a less "dangerous" alternative?


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Thu, 13 Aug 2020 10:23:57 -0400
> 
> > And why does that matter in the case in point?
> 
> No idea, but before removing that code we would have to find out why it
> *doesn't* matter.

It's quite clear that the code wants to put on disk the exact binary
stream it received.

> >> > (And it's wrong to use 'binary' for producing files that will
> >> > be processed by programs other than Emacs, because 'binary' spills out
> >> > the Emacs internal representation of characters.)
> >> It's definitely not wrong for unibyte buffers where there's no "internal
> >> representation of characters" that can spill out ;-)
> > We cannot know that in general.
> 
> Yes, we do: unibyte buffers have no "internal representation of
> characters" that can spill out.

In unibyte buffers the coding-system doesn't matter.

> To me `binary` is *the* coding-system to use when you have bytes and
> want to write them in a file (and same when you want to read the *bytes*
> of a file into a buffer).

For a unibyte buffer, it doesn't matter.  For a multibyte buffer, it's
dangerous.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

>> No idea, but before removing that code we would have to find out why it
>> *doesn't* matter.
> It's quite clear that the code wants to put on disk the exact binary
> stream it received.

Yes, the intention is quite clear.  What is less clear is whether that
intention would be fulfilled in all possible cases if the code doesn't
set the file-coding-system to `binary`.

>> Yes, we do: unibyte buffers have no "internal representation of
>> characters" that can spill out.
> In unibyte buffers the coding-system doesn't matter.

Reality check:

    % /usr/bin/emacs -Q --batch --eval '(with-temp-file "foo.txt" (set-buffer-multibyte nil) (set-buffer-file-coding-system (quote dos)) (insert "a\nb\n"))' 
    % l foo.txt
    -rw-r--r-- 1 monnier monnier 6 aoû 13 13:41 foo.txt
    %

the coding system *does* matter w.r.t EOL.

>> To me `binary` is *the* coding-system to use when you have bytes and
>> want to write them in a file (and same when you want to read the *bytes*
>> of a file into a buffer).
> For a unibyte buffer, it doesn't matter.

See above.

> For a multibyte buffer, it's dangerous.

So you say, but again: Do you know a less "dangerous" alternative?


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Thu, 13 Aug 2020 13:43:55 -0400
> 
> > In unibyte buffers the coding-system doesn't matter.
> 
> Reality check:
> 
>     % /usr/bin/emacs -Q --batch --eval '(with-temp-file "foo.txt" (set-buffer-multibyte nil) (set-buffer-file-coding-system (quote dos)) (insert "a\nb\n"))' 
>     % l foo.txt
>     -rw-r--r-- 1 monnier monnier 6 aoû 13 13:41 foo.txt
>     %
> 
> the coding system *does* matter w.r.t EOL.

Again, this factoid isn't relevant to the issue at hand.

> > For a multibyte buffer, it's dangerous.
> 
> So you say, but again: Do you know a less "dangerous" alternative?

Yes: encode using UTF-8.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

>> > In unibyte buffers the coding-system doesn't matter.
> Again, this factoid isn't relevant to the issue at hand.

Not sure what is the issue at hand at this point, so I replied to what
you wrote.

>> > For a multibyte buffer, it's dangerous.
>> So you say, but again: Do you know a less "dangerous" alternative?
> Yes: encode using UTF-8.

AFAIK the UTF-8 spec does not specify how to encode "byte chars" (like
our eight-bit chars), so if you (think you) have only bytes in your
multibyte buffer (i.e. only ascii and eight-bit chars), using utf-8 is
rather strange and oddly dangerous since you're relying on some
corner-case detail of our utf-8 coding-system.


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Thu, 13 Aug 2020 15:02:07 -0400
> 
> >> > In unibyte buffers the coding-system doesn't matter.
> > Again, this factoid isn't relevant to the issue at hand.
> 
> Not sure what is the issue at hand at this point

The code in eudc-bob.el, of course.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

>> >> > In unibyte buffers the coding-system doesn't matter.
>> > Again, this factoid isn't relevant to the issue at hand.
>> Not sure what is the issue at hand at this point
> The code in eudc-bob.el, of course.

In that particular case, I don't know whether the coding-system will
make a difference because I don't know if the `data` will contain
newlines and if so whether their encoding is significant.

Until proven otherwise, we should assume that the coding-system
*may* be relevant.


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Thu, 13 Aug 2020 15:54:43 -0400
> 
> >> >> > In unibyte buffers the coding-system doesn't matter.
> >> > Again, this factoid isn't relevant to the issue at hand.
> >> Not sure what is the issue at hand at this point
> > The code in eudc-bob.el, of course.
> 
> In that particular case, I don't know whether the coding-system will
> make a difference because I don't know if the `data` will contain
> newlines and if so whether their encoding is significant.

If newline conversion is the only consideration, we could bind
inhibit-eol-conversion instead, no need for the coding-system stuff.

My point is that code which makes a buffer unibyte and then sets its
buffer-file-coding-system yells "FIXME!", so it is best not to write
it.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> My point is that code which makes a buffer unibyte and then sets its
> buffer-file-coding-system yells "FIXME!", so it is best not to write
> it.

Yeah, setting buffer-file-coding-system here is nonsensical, I think?
buffer-file-coding-system is something you set if you have characters in
the buffer, but a unibyte buffer contains only bytes, so no coding system
conversion will be done when saving the buffer, I'd always assumed?

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

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
> Date: Fri, 14 Aug 2020 11:54:20 +0200
> 
> Yeah, setting buffer-file-coding-system here is nonsensical, I think?
> buffer-file-coding-system is something you set if you have characters in
> the buffer, but a unibyte buffer contains only bytes, so no coding system
> conversion will be done when saving the buffer, I'd always assumed?

Precisely.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> Date: Fri, 14 Aug 2020 13:35:58 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > Yeah, setting buffer-file-coding-system here is nonsensical, I think?
> > buffer-file-coding-system is something you set if you have characters in
> > the buffer, but a unibyte buffer contains only bytes, so no coding system
> > conversion will be done when saving the buffer, I'd always assumed?
> 
> Precisely.

Well, modulo the EOL conversion, as discussed.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> > Yeah, setting buffer-file-coding-system here is nonsensical, I think?
>> > buffer-file-coding-system is something you set if you have characters in
>> > the buffer, but a unibyte buffer contains only bytes, so no coding system
>> > conversion will be done when saving the buffer, I'd always assumed?
>> 
>> Precisely.
>
> Well, modulo the EOL conversion, as discussed.

Oh, so then it is needed...  which is unfortunate.  But that explains
all that (let ((coding-system-for-write 'binary)) ...) code all over
Emacs.  :-)

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

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

>>> > Yeah, setting buffer-file-coding-system here is nonsensical, I think?
>>> > buffer-file-coding-system is something you set if you have characters in
>>> > the buffer, but a unibyte buffer contains only bytes, so no coding system
>>> > conversion will be done when saving the buffer, I'd always assumed?
>>> 
>>> Precisely.
>>
>> Well, modulo the EOL conversion, as discussed.
>
> Oh, so then it is needed...  which is unfortunate.  But that explains
> all that (let ((coding-system-for-write 'binary)) ...) code all over
> Emacs.  :-)

There's a case to be made that coding-systems shouldn't affect unibyte
data at all (i.e. not even EOL).  Whether we could do it without
introducing too much breakage (and whether we should do it) is unclear
to me.


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Sat, 15 Aug 2020 11:53:21 +0200
> 
> > Well, modulo the EOL conversion, as discussed.
> 
> Oh, so then it is needed...

No, we could bind inhibit-eol-conversion instead.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> No, we could bind inhibit-eol-conversion instead.

We could, but that's an even bigger ask for people to remember.  This
stuff is full of subtle things that can go wrong, and having clear rules
to rely on would help a lot.

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

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Sun, 16 Aug 2020 13:17:54 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > No, we could bind inhibit-eol-conversion instead.
> 
> We could, but that's an even bigger ask for people to remember.

How is it different from asking them to remember that coding-system is
relevant to unibyte buffers?

> This stuff is full of subtle things that can go wrong, and having
> clear rules to rely on would help a lot.

So which clear rules would you suggest to have?  Setting
buffer-file-coding-system is not my favorite rule for forcing specific
encoding, whether for multibyte or for unibyte buffers.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> > No, we could bind inhibit-eol-conversion instead.
>> 
>> We could, but that's an even bigger ask for people to remember.
>
> How is it different from asking them to remember that coding-system is
> relevant to unibyte buffers?

It's just another thing -- everybody has to deal with
coding-system-for-* here and there, and then this eol thing is another.
That's too big an ask, I think.

>> This stuff is full of subtle things that can go wrong, and having
>> clear rules to rely on would help a lot.
>
> So which clear rules would you suggest to have?  Setting
> buffer-file-coding-system is not my favorite rule for forcing specific
> encoding, whether for multibyte or for unibyte buffers.

How did we end up with having unibyte buffers go through the eol
conversion in the first place?  Is it a relic from the Mule days?

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

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

> How did we end up with having unibyte buffers go through the eol
> conversion in the first place?  Is it a relic from the Mule days?

I think it's a result of the fact that when Mule was introduced, unibyte
buffers were often used for normal text in the western world (to avoid
bugs in code that wasn't yet prepared for coding-systems and non-8bit
chars) but where there was a desire to still support the EOL-conversion
feature of Mule (which was much better than the previous hacks to
handle DOS text files).


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Lars Ingebrigtsen]

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

> I think it's a result of the fact that when Mule was introduced, unibyte
> buffers were often used for normal text in the western world (to avoid
> bugs in code that wasn't yet prepared for coding-systems and non-8bit
> chars) but where there was a desire to still support the EOL-conversion
> feature of Mule (which was much better than the previous hacks to
> handle DOS text files).

Right...  but these days, we don't use Unibyte buffers for text at all?
So is this still baggage that we have to carry around?

Hm...  I guess if we have a unibyte ASCII buffer, somebody might still
want to do EOL conversions...

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

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Tue, 18 Aug 2020 16:56:32 +0200
> 
> Hm...  I guess if we have a unibyte ASCII buffer, somebody might still
> want to do EOL conversions...

That's the fear, yes.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

> Right...  but these days, we don't use Unibyte buffers for text at all?

That's right.

> So is this still baggage that we have to carry around?

I'm pretty sure it's not necessary.
But I don't have a clear idea of how much breakage it would involve:
someone should try it out and see what breaks in the packages they use to
get a feel for how serious it is.

> Hm...  I guess if we have a unibyte ASCII buffer, somebody might still
> want to do EOL conversions...

I get the impression that the more problematic case will be process
filters: I think it is fairly rare to have ASCII text (really meant as
*text* where we want EOL conversion) in a unibyte buffer, but I think
it's pretty frequent to have ASCII *text* inside unibyte strings.
The difference is that the unibyteness of strings is determined
semi-automatically.

So we could end up with a half-way rule where coding-systems (i.e. EOL
conversion) doesn't apply to text coming from or going to a unibyte
buffer, but does apply to unibyte strings :-(


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Lars Ingebrigtsen]

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

> So we could end up with a half-way rule where coding-systems (i.e. EOL
> conversion) doesn't apply to text coming from or going to a unibyte
> buffer, but does apply to unibyte strings :-(

Yeah, that sounds ... awkward...

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

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 17 Aug 2020 10:32:46 +0200
> 
> How did we end up with having unibyte buffers go through the eol
> conversion in the first place?  Is it a relic from the Mule days?

No, Emacs did EOL conversion even in v19.x (although I think it only
did it on MS-Windows and MS-DOS).  Everything was unibyte back then,
remember?  When MULE was merged, we incorporated EOL conversion into
its machinery and made it available on all platforms.  But unibyte
buffers were still widely used back then, so it was necessary to
provide EOL conversion for them.

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

> I think the intention here is to change the coding system in this new
> temporary buffer (before saving),

Agreed.

> (And this should probably just be rewritten as a `with-temp-buffer'.)

Yup,


        Stefan

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

> (defun eudc-bob-save-object ()
>   "Save the object data of the button at point."
>   (interactive)
>   (let ((data (eudc-bob-get-overlay-prop 'object-data))
> 	(buffer (generate-new-buffer "*eudc-tmp*")))
>     (save-excursion
>       (set-buffer-file-coding-system 'binary)
>       (set-buffer buffer)
>       (set-buffer-multibyte nil)
>       (insert data)
>       (save-buffer))
>     (kill-buffer buffer)))

Does this function work at all?
I mean, won't `save-buffer` just burp because `buffer-file-name` is nil?


        Stefan "who only noticed after attempting to simply the code
                to `(write-region data nil ...)`"

Re: master b8062be 3/5: Remove some compat code from eudc-bob.el

[Stefan Monnier]

> Does this function work at all?
> I mean, won't `save-buffer` just burp because `buffer-file-name` is nil?

Duh, sorry, yes it works, it just prompts the user for a file name.


        Stefan