Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)). PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ... Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails. Guille, can you explain the change. The commit log just says "make tests run". Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position. Thanks, Alistair |
> On 15 Mar 2018, at 12:28, Alistair Grant <[hidden email]> wrote: > > Hi Guille & Pavel, > > On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: > so that it doesn't reset the dataStream position back to the start of > the stream. This breaks Pavel's PlayingCard, part of FreeCell (you > can see that I only load important stuff :-)). > > PlayingCard class>>initialize > "PlayingCard initialize" > | forms f | > "Read the stored forms from mime-encoded data in imageData." > f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). > forms := OrderedCollection new. > f next = 2 ifFalse: [self error: 'corrupted imageData']. > [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. > ... > > > Previously, f would be at the start of the stream (which makes sense > given this usage), but now it is at the end of the stream, so returns > nil and "f next = 2" fails. > > Guille, can you explain the change. The commit log just says "make tests run". > > Note that a lot of users of this won't fail as they just call > #contents of the stream after #mimeDecodeToBytes:, which works > regardless of the current position. So doing f reset at the caller site would fix it then ? Note that there is the newer ZnBase64Encoder that can be used as follows: f := (ZnBase64Encoder new decode: self imageData) readStream. Sven |
Hi Sven,
On 15 March 2018 at 12:47, Sven Van Caekenberghe <[hidden email]> wrote: > > >> On 15 Mar 2018, at 12:28, Alistair Grant <[hidden email]> wrote: >> >> Hi Guille & Pavel, >> >> On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: >> so that it doesn't reset the dataStream position back to the start of >> the stream. This breaks Pavel's PlayingCard, part of FreeCell (you >> can see that I only load important stuff :-)). >> >> PlayingCard class>>initialize >> "PlayingCard initialize" >> | forms f | >> "Read the stored forms from mime-encoded data in imageData." >> f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). >> forms := OrderedCollection new. >> f next = 2 ifFalse: [self error: 'corrupted imageData']. >> [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. >> ... >> >> >> Previously, f would be at the start of the stream (which makes sense >> given this usage), but now it is at the end of the stream, so returns >> nil and "f next = 2" fails. >> >> Guille, can you explain the change. The commit log just says "make tests run". >> >> Note that a lot of users of this won't fail as they just call >> #contents of the stream after #mimeDecodeToBytes:, which works >> regardless of the current position. > > So doing > > f reset > > at the caller site would fix it then ? Yes, but I was thinking more about whether it was intended to put this responsibility on the caller (changing behaviour). > Note that there is the newer ZnBase64Encoder that can be used as follows: > > f := (ZnBase64Encoder new decode: self imageData) readStream. That works too. Thanks, Alistair |
In reply to this post by Sven Van Caekenberghe-2
BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods:
Remove String>>#base64Encoded. Change ByteArray>>#base64Encoded "Encode the receiver using Base64, see ZnBase64Encoder" ^ ZnBase64Encoder new encode: self String>>#base64Decoded "Decode the receiver using Base64, see ZnBase64Encoder" ^ ZnBase64Encoder new decode: self The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64: So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept. > On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <[hidden email]> wrote: > > > >> On 15 Mar 2018, at 12:28, Alistair Grant <[hidden email]> wrote: >> >> Hi Guille & Pavel, >> >> On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: >> so that it doesn't reset the dataStream position back to the start of >> the stream. This breaks Pavel's PlayingCard, part of FreeCell (you >> can see that I only load important stuff :-)). >> >> PlayingCard class>>initialize >> "PlayingCard initialize" >> | forms f | >> "Read the stored forms from mime-encoded data in imageData." >> f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). >> forms := OrderedCollection new. >> f next = 2 ifFalse: [self error: 'corrupted imageData']. >> [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. >> ... >> >> >> Previously, f would be at the start of the stream (which makes sense >> given this usage), but now it is at the end of the stream, so returns >> nil and "f next = 2" fails. >> >> Guille, can you explain the change. The commit log just says "make tests run". >> >> Note that a lot of users of this won't fail as they just call >> #contents of the stream after #mimeDecodeToBytes:, which works >> regardless of the current position. > > So doing > > f reset > > at the caller site would fix it then ? > > Note that there is the newer ZnBase64Encoder that can be used as follows: > > f := (ZnBase64Encoder new decode: self imageData) readStream. > > Sven |
Also:
'apicture.png' asFileReference inspect. Will raise a "image format not recognised" because: - AbstractFileReference>>binaryReadStreamDo: ultimately uses a ZnBufferedReadStream - ImageReadWriter>>on: sends #reset to the stream and ZnBufferedReadStream doesn't implement #reset. My guess is that ZnBufferedReadStream should implement #reset (it has #position:). But I'm not familiar with the current thinking on stream rationalisation. I'll raise some fogbugz issues a bit later. Image downloaded today: Pharo 7.0 Build information: Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d (64 Bit) Thanks, Alistair On 15 March 2018 at 13:08, Sven Van Caekenberghe <[hidden email]> wrote: > BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods: > > Remove > > String>>#base64Encoded. > > Change > > ByteArray>>#base64Encoded > "Encode the receiver using Base64, see ZnBase64Encoder" > > ^ ZnBase64Encoder new encode: self > > String>>#base64Decoded > "Decode the receiver using Base64, see ZnBase64Encoder" > > ^ ZnBase64Encoder new decode: self > > The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64: > > So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept. > >> On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <[hidden email]> wrote: >> >> >> >>> On 15 Mar 2018, at 12:28, Alistair Grant <[hidden email]> wrote: >>> >>> Hi Guille & Pavel, >>> >>> On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: >>> so that it doesn't reset the dataStream position back to the start of >>> the stream. This breaks Pavel's PlayingCard, part of FreeCell (you >>> can see that I only load important stuff :-)). >>> >>> PlayingCard class>>initialize >>> "PlayingCard initialize" >>> | forms f | >>> "Read the stored forms from mime-encoded data in imageData." >>> f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). >>> forms := OrderedCollection new. >>> f next = 2 ifFalse: [self error: 'corrupted imageData']. >>> [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. >>> ... >>> >>> >>> Previously, f would be at the start of the stream (which makes sense >>> given this usage), but now it is at the end of the stream, so returns >>> nil and "f next = 2" fails. >>> >>> Guille, can you explain the change. The commit log just says "make tests run". >>> >>> Note that a lot of users of this won't fail as they just call >>> #contents of the stream after #mimeDecodeToBytes:, which works >>> regardless of the current position. >> >> So doing >> >> f reset >> >> at the caller site would fix it then ? >> >> Note that there is the newer ZnBase64Encoder that can be used as follows: >> >> f := (ZnBase64Encoder new decode: self imageData) readStream. >> >> Sven > > |
Yeah, right, good catch.
But what is this nonsense: PNGReadWriter>>on: aStream (stream := aStream) reset. stream binary "Note that 'reset' makes a file be text. Must do this after." Of course you can only read an image from a binary stream, there should be nothing else there but stream := aStream > On 15 Mar 2018, at 13:31, Alistair Grant <[hidden email]> wrote: > > Also: > > 'apicture.png' asFileReference inspect. > > Will raise a "image format not recognised" because: > > - AbstractFileReference>>binaryReadStreamDo: ultimately uses a > ZnBufferedReadStream > - ImageReadWriter>>on: sends #reset to the stream > > and ZnBufferedReadStream doesn't implement #reset. > > My guess is that ZnBufferedReadStream should implement #reset (it has > #position:). But I'm not familiar with the current thinking on stream > rationalisation. > > I'll raise some fogbugz issues a bit later. > > Image downloaded today: > > Pharo 7.0 > Build information: > Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d > (64 Bit) > > > > Thanks, > Alistair > > > On 15 March 2018 at 13:08, Sven Van Caekenberghe <[hidden email]> wrote: >> BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods: >> >> Remove >> >> String>>#base64Encoded. >> >> Change >> >> ByteArray>>#base64Encoded >> "Encode the receiver using Base64, see ZnBase64Encoder" >> >> ^ ZnBase64Encoder new encode: self >> >> String>>#base64Decoded >> "Decode the receiver using Base64, see ZnBase64Encoder" >> >> ^ ZnBase64Encoder new decode: self >> >> The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64: >> >> So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept. >> >>> On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <[hidden email]> wrote: >>> >>> >>> >>>> On 15 Mar 2018, at 12:28, Alistair Grant <[hidden email]> wrote: >>>> >>>> Hi Guille & Pavel, >>>> >>>> On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: >>>> so that it doesn't reset the dataStream position back to the start of >>>> the stream. This breaks Pavel's PlayingCard, part of FreeCell (you >>>> can see that I only load important stuff :-)). >>>> >>>> PlayingCard class>>initialize >>>> "PlayingCard initialize" >>>> | forms f | >>>> "Read the stored forms from mime-encoded data in imageData." >>>> f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). >>>> forms := OrderedCollection new. >>>> f next = 2 ifFalse: [self error: 'corrupted imageData']. >>>> [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. >>>> ... >>>> >>>> >>>> Previously, f would be at the start of the stream (which makes sense >>>> given this usage), but now it is at the end of the stream, so returns >>>> nil and "f next = 2" fails. >>>> >>>> Guille, can you explain the change. The commit log just says "make tests run". >>>> >>>> Note that a lot of users of this won't fail as they just call >>>> #contents of the stream after #mimeDecodeToBytes:, which works >>>> regardless of the current position. >>> >>> So doing >>> >>> f reset >>> >>> at the caller site would fix it then ? >>> >>> Note that there is the newer ZnBase64Encoder that can be used as follows: >>> >>> f := (ZnBase64Encoder new decode: self imageData) readStream. >>> >>> Sven >> >> > |
And in PNGReadWriter>>nextImage we should remove stream reset and stream binary.
This probably goes for the other image readers as well. > On 15 Mar 2018, at 13:38, Sven Van Caekenberghe <[hidden email]> wrote: > > Yeah, right, good catch. > > But what is this nonsense: > > PNGReadWriter>>on: aStream > (stream := aStream) reset. > stream binary > "Note that 'reset' makes a file be text. Must do this after." > > Of course you can only read an image from a binary stream, there should be nothing else there but > > stream := aStream > >> On 15 Mar 2018, at 13:31, Alistair Grant <[hidden email]> wrote: >> >> Also: >> >> 'apicture.png' asFileReference inspect. >> >> Will raise a "image format not recognised" because: >> >> - AbstractFileReference>>binaryReadStreamDo: ultimately uses a >> ZnBufferedReadStream >> - ImageReadWriter>>on: sends #reset to the stream >> >> and ZnBufferedReadStream doesn't implement #reset. >> >> My guess is that ZnBufferedReadStream should implement #reset (it has >> #position:). But I'm not familiar with the current thinking on stream >> rationalisation. >> >> I'll raise some fogbugz issues a bit later. >> >> Image downloaded today: >> >> Pharo 7.0 >> Build information: >> Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d >> (64 Bit) >> >> >> >> Thanks, >> Alistair >> >> >> On 15 March 2018 at 13:08, Sven Van Caekenberghe <[hidden email]> wrote: >>> BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods: >>> >>> Remove >>> >>> String>>#base64Encoded. >>> >>> Change >>> >>> ByteArray>>#base64Encoded >>> "Encode the receiver using Base64, see ZnBase64Encoder" >>> >>> ^ ZnBase64Encoder new encode: self >>> >>> String>>#base64Decoded >>> "Decode the receiver using Base64, see ZnBase64Encoder" >>> >>> ^ ZnBase64Encoder new decode: self >>> >>> The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64: >>> >>> So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept. >>> >>>> On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <[hidden email]> wrote: >>>> >>>> >>>> >>>>> On 15 Mar 2018, at 12:28, Alistair Grant <[hidden email]> wrote: >>>>> >>>>> Hi Guille & Pavel, >>>>> >>>>> On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: >>>>> so that it doesn't reset the dataStream position back to the start of >>>>> the stream. This breaks Pavel's PlayingCard, part of FreeCell (you >>>>> can see that I only load important stuff :-)). >>>>> >>>>> PlayingCard class>>initialize >>>>> "PlayingCard initialize" >>>>> | forms f | >>>>> "Read the stored forms from mime-encoded data in imageData." >>>>> f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). >>>>> forms := OrderedCollection new. >>>>> f next = 2 ifFalse: [self error: 'corrupted imageData']. >>>>> [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. >>>>> ... >>>>> >>>>> >>>>> Previously, f would be at the start of the stream (which makes sense >>>>> given this usage), but now it is at the end of the stream, so returns >>>>> nil and "f next = 2" fails. >>>>> >>>>> Guille, can you explain the change. The commit log just says "make tests run". >>>>> >>>>> Note that a lot of users of this won't fail as they just call >>>>> #contents of the stream after #mimeDecodeToBytes:, which works >>>>> regardless of the current position. >>>> >>>> So doing >>>> >>>> f reset >>>> >>>> at the caller site would fix it then ? >>>> >>>> Note that there is the newer ZnBase64Encoder that can be used as follows: >>>> >>>> f := (ZnBase64Encoder new decode: self imageData) readStream. >>>> >>>> Sven >>> >>> >> > |
In reply to this post by Sven Van Caekenberghe-2
Hi sven
can you open a bug entry? Sven I have a APi question why we could not have decodeBase64: in something else than ZnUtils. I tend to stay away from class having Util in their names :) I still have the scars on my body of my battle against Utilities. Stef On Thu, Mar 15, 2018 at 1:08 PM, Sven Van Caekenberghe <[hidden email]> wrote: > BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods: > > Remove > > String>>#base64Encoded. > > Change > > ByteArray>>#base64Encoded > "Encode the receiver using Base64, see ZnBase64Encoder" > > ^ ZnBase64Encoder new encode: self > > String>>#base64Decoded > "Decode the receiver using Base64, see ZnBase64Encoder" > > ^ ZnBase64Encoder new decode: self > > The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64: > > So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept. > >> On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <[hidden email]> wrote: >> >> >> >>> On 15 Mar 2018, at 12:28, Alistair Grant <[hidden email]> wrote: >>> >>> Hi Guille & Pavel, >>> >>> On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: >>> so that it doesn't reset the dataStream position back to the start of >>> the stream. This breaks Pavel's PlayingCard, part of FreeCell (you >>> can see that I only load important stuff :-)). >>> >>> PlayingCard class>>initialize >>> "PlayingCard initialize" >>> | forms f | >>> "Read the stored forms from mime-encoded data in imageData." >>> f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). >>> forms := OrderedCollection new. >>> f next = 2 ifFalse: [self error: 'corrupted imageData']. >>> [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. >>> ... >>> >>> >>> Previously, f would be at the start of the stream (which makes sense >>> given this usage), but now it is at the end of the stream, so returns >>> nil and "f next = 2" fails. >>> >>> Guille, can you explain the change. The commit log just says "make tests run". >>> >>> Note that a lot of users of this won't fail as they just call >>> #contents of the stream after #mimeDecodeToBytes:, which works >>> regardless of the current position. >> >> So doing >> >> f reset >> >> at the caller site would fix it then ? >> >> Note that there is the newer ZnBase64Encoder that can be used as follows: >> >> f := (ZnBase64Encoder new decode: self imageData) readStream. >> >> Sven > > |
In reply to this post by Sven Van Caekenberghe-2
Yes this is that you cannot have a png embedded in stream. Just at the
beginning. What funny concept. On Thu, Mar 15, 2018 at 2:14 PM, Sven Van Caekenberghe <[hidden email]> wrote: > And in PNGReadWriter>>nextImage we should remove stream reset and stream binary. > > This probably goes for the other image readers as well. > >> On 15 Mar 2018, at 13:38, Sven Van Caekenberghe <[hidden email]> wrote: >> >> Yeah, right, good catch. >> >> But what is this nonsense: >> >> PNGReadWriter>>on: aStream >> (stream := aStream) reset. >> stream binary >> "Note that 'reset' makes a file be text. Must do this after." >> >> Of course you can only read an image from a binary stream, there should be nothing else there but >> >> stream := aStream >> >>> On 15 Mar 2018, at 13:31, Alistair Grant <[hidden email]> wrote: >>> >>> Also: >>> >>> 'apicture.png' asFileReference inspect. >>> >>> Will raise a "image format not recognised" because: >>> >>> - AbstractFileReference>>binaryReadStreamDo: ultimately uses a >>> ZnBufferedReadStream >>> - ImageReadWriter>>on: sends #reset to the stream >>> >>> and ZnBufferedReadStream doesn't implement #reset. >>> >>> My guess is that ZnBufferedReadStream should implement #reset (it has >>> #position:). But I'm not familiar with the current thinking on stream >>> rationalisation. >>> >>> I'll raise some fogbugz issues a bit later. >>> >>> Image downloaded today: >>> >>> Pharo 7.0 >>> Build information: >>> Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d >>> (64 Bit) >>> >>> >>> >>> Thanks, >>> Alistair >>> >>> >>> On 15 March 2018 at 13:08, Sven Van Caekenberghe <[hidden email]> wrote: >>>> BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods: >>>> >>>> Remove >>>> >>>> String>>#base64Encoded. >>>> >>>> Change >>>> >>>> ByteArray>>#base64Encoded >>>> "Encode the receiver using Base64, see ZnBase64Encoder" >>>> >>>> ^ ZnBase64Encoder new encode: self >>>> >>>> String>>#base64Decoded >>>> "Decode the receiver using Base64, see ZnBase64Encoder" >>>> >>>> ^ ZnBase64Encoder new decode: self >>>> >>>> The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64: >>>> >>>> So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept. >>>> >>>>> On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <[hidden email]> wrote: >>>>> >>>>> >>>>> >>>>>> On 15 Mar 2018, at 12:28, Alistair Grant <[hidden email]> wrote: >>>>>> >>>>>> Hi Guille & Pavel, >>>>>> >>>>>> On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: >>>>>> so that it doesn't reset the dataStream position back to the start of >>>>>> the stream. This breaks Pavel's PlayingCard, part of FreeCell (you >>>>>> can see that I only load important stuff :-)). >>>>>> >>>>>> PlayingCard class>>initialize >>>>>> "PlayingCard initialize" >>>>>> | forms f | >>>>>> "Read the stored forms from mime-encoded data in imageData." >>>>>> f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). >>>>>> forms := OrderedCollection new. >>>>>> f next = 2 ifFalse: [self error: 'corrupted imageData']. >>>>>> [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. >>>>>> ... >>>>>> >>>>>> >>>>>> Previously, f would be at the start of the stream (which makes sense >>>>>> given this usage), but now it is at the end of the stream, so returns >>>>>> nil and "f next = 2" fails. >>>>>> >>>>>> Guille, can you explain the change. The commit log just says "make tests run". >>>>>> >>>>>> Note that a lot of users of this won't fail as they just call >>>>>> #contents of the stream after #mimeDecodeToBytes:, which works >>>>>> regardless of the current position. >>>>> >>>>> So doing >>>>> >>>>> f reset >>>>> >>>>> at the caller site would fix it then ? >>>>> >>>>> Note that there is the newer ZnBase64Encoder that can be used as follows: >>>>> >>>>> f := (ZnBase64Encoder new decode: self imageData) readStream. >>>>> >>>>> Sven >>>> >>>> >>> >> > > |
Free forum by Nabble | Edit this page |