Encoding problem

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Encoding problem

Stephan Eggermont-3
We have an issue in default Pier with rendering/encoding.
Did I miss setting encoding somewhere?

If we enter in WAEncodingFunctionalTest the following three characters:
->ë
the text input field removes the -.

We noticed because the text area used to edit pier documents
became unusable.

Stephan



_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Sven Van Caekenberghe-2

On 20 May 2014, at 12:47, Stephan Eggermont <[hidden email]> wrote:

> We have an issue in default Pier with rendering/encoding.
> Did I miss setting encoding somewhere?
>
> If we enter in WAEncodingFunctionalTest the following three characters:
> ->ë
> the text input field removes the -.

I can confirm the problem. This is really weird.

Apparently you need a non-ascii character preceded by an xml-unsafe one:

abc<é
aaa&ç

And when the characters are printed one by one wrapped in a span they all show !

> We noticed because the text area used to edit pier documents
> became unusable.
>
> Stephan
>
>
>
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Johan Brichau-2
What version of Seaside are you guys running?

On 20 May 2014, at 13:19, Sven Van Caekenberghe <[hidden email]> wrote:

>
> On 20 May 2014, at 12:47, Stephan Eggermont <[hidden email]> wrote:
>
>> We have an issue in default Pier with rendering/encoding.
>> Did I miss setting encoding somewhere?
>>
>> If we enter in WAEncodingFunctionalTest the following three characters:
>> ->ë
>> the text input field removes the -.
>
> I can confirm the problem. This is really weird.
>
> Apparently you need a non-ascii character preceded by an xml-unsafe one:
>
> abc<é
> aaa&ç
>
> And when the characters are printed one by one wrapped in a span they all show !
>
>> We noticed because the text area used to edit pier documents
>> became unusable.
>>
>> Stephan
>>
>>
>>
>> _______________________________________________
>> seaside mailing list
>> [hidden email]
>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Sven Van Caekenberghe-2
Seaside 3.1.0 (ConfigurationOfSeaside3 stable, I guess) on Pharo 3 (Bootstrap).

On 20 May 2014, at 13:53, Johan Brichau <[hidden email]> wrote:

> What version of Seaside are you guys running?
>
> On 20 May 2014, at 13:19, Sven Van Caekenberghe <[hidden email]> wrote:
>
>>
>> On 20 May 2014, at 12:47, Stephan Eggermont <[hidden email]> wrote:
>>
>>> We have an issue in default Pier with rendering/encoding.
>>> Did I miss setting encoding somewhere?
>>>
>>> If we enter in WAEncodingFunctionalTest the following three characters:
>>> ->ë
>>> the text input field removes the -.
>>
>> I can confirm the problem. This is really weird.
>>
>> Apparently you need a non-ascii character preceded by an xml-unsafe one:
>>
>> abc<é
>> aaa&ç
>>
>> And when the characters are printed one by one wrapped in a span they all show !
>>
>>> We noticed because the text area used to edit pier documents
>>> became unusable.
>>>
>>> Stephan
>>>
>>>
>>>
>>> _______________________________________________
>>> seaside mailing list
>>> [hidden email]
>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>
>> _______________________________________________
>> seaside mailing list
>> [hidden email]
>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Stephan Eggermont-3
In reply to this post by Stephan Eggermont-3
I have the problem with a fresh ci image from
https://ci.inria.fr/pharo-contribution/job/Pier3/
 Pharo 3, release 3.1
 Pharo 3, release 3.0
 Pharo 2, release 3.1

Stephan
_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Paul DeBruicker
In reply to this post by Johan Brichau-2
On Pharo 1.4, with Seaside 3.1, there is no error.

I have not tried with Pier loaded.  Should I?





Johan Brichau-2 wrote
What version of Seaside are you guys running?

On 20 May 2014, at 13:19, Sven Van Caekenberghe <[hidden email]> wrote:

>
> On 20 May 2014, at 12:47, Stephan Eggermont <[hidden email]> wrote:
>
>> We have an issue in default Pier with rendering/encoding.
>> Did I miss setting encoding somewhere?
>>
>> If we enter in WAEncodingFunctionalTest the following three characters:
>> ->ë
>> the text input field removes the -.
>
> I can confirm the problem. This is really weird.
>
> Apparently you need a non-ascii character preceded by an xml-unsafe one:
>
> abc<é
> aaa&ç
>
> And when the characters are printed one by one wrapped in a span they all show !
>
>> We noticed because the text area used to edit pier documents
>> became unusable.
>>
>> Stephan
>>
>>
>>
>> _______________________________________________
>> seaside mailing list
>> [hidden email]
>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Sven Van Caekenberghe-2

On 20 May 2014, at 17:19, Paul DeBruicker <[hidden email]> wrote:

> On Pharo 1.4, with Seaside 3.1, there is no error.
>
> I have not tried with Pier loaded.  Should I?

No, it happens without Pier being loaded.

>
> Johan Brichau-2 wrote
>> What version of Seaside are you guys running?
>>
>> On 20 May 2014, at 13:19, Sven Van Caekenberghe &lt;
>
>> sven@
>
>> &gt; wrote:
>>
>>>
>>> On 20 May 2014, at 12:47, Stephan Eggermont &lt;
>
>> stephan@
>
>> &gt; wrote:
>>>
>>>> We have an issue in default Pier with rendering/encoding.
>>>> Did I miss setting encoding somewhere?
>>>>
>>>> If we enter in WAEncodingFunctionalTest the following three characters:
>>>> ->ë
>>>> the text input field removes the -.
>>>
>>> I can confirm the problem. This is really weird.
>>>
>>> Apparently you need a non-ascii character preceded by an xml-unsafe one:
>>>
>>> abc
>> <é
>>>
>> aaa&ç
>>>
>>> And when the characters are printed one by one wrapped in a span they all
>>> show !
>>>
>>>> We noticed because the text area used to edit pier documents
>>>> became unusable.
>>>>
>>>> Stephan
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> seaside mailing list
>>>>
>
>> seaside@.squeakfoundation
>
>>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>>
>>> _______________________________________________
>>> seaside mailing list
>>>
>
>> seaside@.squeakfoundation
>
>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>
>> _______________________________________________
>> seaside mailing list
>
>> seaside@.squeakfoundation
>
>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>
>
>
>
>
> --
> View this message in context: http://forum.world.st/Encoding-problem-tp4759665p4759711.html
> Sent from the Seaside General mailing list archive at Nabble.com.
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Johan Brichau-2
Thanks for reporting, I created an issue for it: https://code.google.com/p/seaside/issues/detail?id=792

There is something going wrong in the WAPharoXmlEncoder in combination with a GRPharoUtf8EncodedStream:

|encoder stream |
stream := GRPharoUtf8CodecStream on: (WriteStream on: String new) converter: UTF8TextConverter new.
encoder := WAPharoXmlEncoder on: stream.
encoder nextPutAllFast: '->ë'.
stream contents

The above works correctly if the stream is a normal WriteStream

Getting too late now to dive into it...

Johan

On 20 May 2014, at 19:36, Sven Van Caekenberghe <[hidden email]> wrote:

>
> On 20 May 2014, at 17:19, Paul DeBruicker <[hidden email]> wrote:
>
>> On Pharo 1.4, with Seaside 3.1, there is no error.
>>
>> I have not tried with Pier loaded.  Should I?
>
> No, it happens without Pier being loaded.
>
>>
>> Johan Brichau-2 wrote
>>> What version of Seaside are you guys running?
>>>
>>> On 20 May 2014, at 13:19, Sven Van Caekenberghe &lt;
>>
>>> sven@
>>
>>> &gt; wrote:
>>>
>>>>
>>>> On 20 May 2014, at 12:47, Stephan Eggermont &lt;
>>
>>> stephan@
>>
>>> &gt; wrote:
>>>>
>>>>> We have an issue in default Pier with rendering/encoding.
>>>>> Did I miss setting encoding somewhere?
>>>>>
>>>>> If we enter in WAEncodingFunctionalTest the following three characters:
>>>>> ->ë
>>>>> the text input field removes the -.
>>>>
>>>> I can confirm the problem. This is really weird.
>>>>
>>>> Apparently you need a non-ascii character preceded by an xml-unsafe one:
>>>>
>>>> abc
>>> <é
>>>>
>>> aaa&ç
>>>>
>>>> And when the characters are printed one by one wrapped in a span they all
>>>> show !
>>>>
>>>>> We noticed because the text area used to edit pier documents
>>>>> became unusable.
>>>>>
>>>>> Stephan
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> seaside mailing list
>>>>>
>>
>>> seaside@.squeakfoundation
>>
>>>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>>>
>>>> _______________________________________________
>>>> seaside mailing list
>>>>
>>
>>> seaside@.squeakfoundation
>>
>>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>>
>>> _______________________________________________
>>> seaside mailing list
>>
>>> seaside@.squeakfoundation
>>
>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>
>>
>>
>>
>>
>> --
>> View this message in context: http://forum.world.st/Encoding-problem-tp4759665p4759711.html
>> Sent from the Seaside General mailing list archive at Nabble.com.
>> _______________________________________________
>> seaside mailing list
>> [hidden email]
>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Sven Van Caekenberghe-2
The XML encoder is not needed to produce the bug:

| stream |
stream := GRPharoUtf8CodecStream on: String new writeStream converter: UTF8TextConverter new.
stream greaseNext: 1 putAll: '->ë' startingAt: 1.
stream contents

is empty but should be '-'.

The error is in

GRPharoUtf8CodecStream>>#greaseNext:putAllFast:startingAt:

the first escape clause in the whileFalse computes a wrong first argument to #greaseNext:putAllFast:startingAt:

however, it is a bit hard to say what the correct one should be, I will try with

startIndex + anInteger - lastIndex

I hope there are enough tests.

Similar code is in ZnUtf8Encoder>>#next:putAllByteString:startingAt:toStream: but the loop is structured differently - it took me a while to get that one working ;-)

On 20 May 2014, at 22:37, Johan Brichau <[hidden email]> wrote:

> Thanks for reporting, I created an issue for it: https://code.google.com/p/seaside/issues/detail?id=792
>
> There is something going wrong in the WAPharoXmlEncoder in combination with a GRPharoUtf8EncodedStream:
>
> |encoder stream |
> stream := GRPharoUtf8CodecStream on: (WriteStream on: String new) converter: UTF8TextConverter new.
> encoder := WAPharoXmlEncoder on: stream.
> encoder nextPutAllFast: '->ë'.
> stream contents
>
> The above works correctly if the stream is a normal WriteStream
>
> Getting too late now to dive into it...
>
> Johan
>
> On 20 May 2014, at 19:36, Sven Van Caekenberghe <[hidden email]> wrote:
>
>>
>> On 20 May 2014, at 17:19, Paul DeBruicker <[hidden email]> wrote:
>>
>>> On Pharo 1.4, with Seaside 3.1, there is no error.
>>>
>>> I have not tried with Pier loaded.  Should I?
>>
>> No, it happens without Pier being loaded.
>>
>>>
>>> Johan Brichau-2 wrote
>>>> What version of Seaside are you guys running?
>>>>
>>>> On 20 May 2014, at 13:19, Sven Van Caekenberghe &lt;
>>>
>>>> sven@
>>>
>>>> &gt; wrote:
>>>>
>>>>>
>>>>> On 20 May 2014, at 12:47, Stephan Eggermont &lt;
>>>
>>>> stephan@
>>>
>>>> &gt; wrote:
>>>>>
>>>>>> We have an issue in default Pier with rendering/encoding.
>>>>>> Did I miss setting encoding somewhere?
>>>>>>
>>>>>> If we enter in WAEncodingFunctionalTest the following three characters:
>>>>>> ->ë
>>>>>> the text input field removes the -.
>>>>>
>>>>> I can confirm the problem. This is really weird.
>>>>>
>>>>> Apparently you need a non-ascii character preceded by an xml-unsafe one:
>>>>>
>>>>> abc
>>>> <é
>>>>>
>>>> aaa&ç
>>>>>
>>>>> And when the characters are printed one by one wrapped in a span they all
>>>>> show !
>>>>>
>>>>>> We noticed because the text area used to edit pier documents
>>>>>> became unusable.
>>>>>>
>>>>>> Stephan
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> seaside mailing list
>>>>>>
>>>
>>>> seaside@.squeakfoundation
>>>
>>>>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>>>>
>>>>> _______________________________________________
>>>>> seaside mailing list
>>>>>
>>>
>>>> seaside@.squeakfoundation
>>>
>>>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>>>
>>>> _______________________________________________
>>>> seaside mailing list
>>>
>>>> seaside@.squeakfoundation
>>>
>>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>>
>>>
>>>
>>>
>>>
>>> --
>>> View this message in context: http://forum.world.st/Encoding-problem-tp4759665p4759711.html
>>> Sent from the Seaside General mailing list archive at Nabble.com.
>>> _______________________________________________
>>> seaside mailing list
>>> [hidden email]
>>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>>
>> _______________________________________________
>> seaside mailing list
>> [hidden email]
>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Sven Van Caekenberghe-2

On 20 May 2014, at 23:15, Sven Van Caekenberghe <[hidden email]> wrote:

> The error is in
>
> GRPharoUtf8CodecStream>>#greaseNext:putAllFast:startingAt:
>
> the first escape clause in the whileFalse computes a wrong first argument to #greaseNext:putAllFast:startingAt:
>
> however, it is a bit hard to say what the correct one should be, I will try with
>
> startIndex + anInteger - lastIndex
>
> I hope there are enough tests.

I added an extra assert

        self assert: 'ab', umlaut next: 1 startingAt: 1 gives:  'a'.

to GRPharoCodecTest>>#testGreaseNextPutAllStartingAt so that it now reads:

  | umlaut encodedUmlaut |
        umlaut := String with: (Character value: 228).
        encodedUmlaut := String with: (Character value: 195) with: (Character value: 164).
        self assert: 'ab' next: 1 startingAt: 1 gives:  'a'.
        self assert: 'a', umlaut, 'b' next: 1 startingAt: 1 gives:  'a'.
        self assert: 'ab', umlaut next: 1 startingAt: 1 gives:  'a'.
        self assert: 'a', umlaut, 'b' next: 2 startingAt: 1 gives:  'a', encodedUmlaut.
        self assert: 'a', umlaut, 'b' next: 1 startingAt: 2 gives:  encodedUmlaut.
        self assert: 'a', umlaut, 'b' next: 2 startingAt: 2 gives:  encodedUmlaut, 'b'.
        self assert: 'a', umlaut, umlaut next: 2 startingAt: 2 gives:  encodedUmlaut, encodedUmlaut.
        self assert: 'ab', umlaut, 'b', umlaut next: 3 startingAt: 2 gives:  'b', encodedUmlaut, 'b'

it fails before the proposed change, it succeeds after it - and the functional test now passes.

Final code of GRPharoUtf8CodecStream>>#greaseNext:putAllFast:startingAt:

        | lastIndex nextIndex |
        lastIndex := startIndex.
        nextIndex := ByteString findFirstInString: aByteString inSet: Latin1ToUtf8Map startingAt: lastIndex.
        nextIndex = 0 ifTrue: [ ^ stream greaseNext: anInteger putAll: aByteString startingAt: startIndex ].
        [
                nextIndex >= (startIndex + anInteger) ifTrue: [
                        ^ stream greaseNext: startIndex + anInteger - lastIndex putAll: aByteString startingAt: lastIndex ].
                nextIndex > lastIndex ifTrue: [
                        stream greaseNext: nextIndex - lastIndex putAll: aByteString startingAt: lastIndex ].
                stream nextPutAll: (Latin1ToUtf8Encodings at: (aByteString byteAt: nextIndex) + 1).
                lastIndex := nextIndex + 1.
                nextIndex := ByteString findFirstInString: aByteString inSet: Latin1ToUtf8Map startingAt: lastIndex.
                (nextIndex = 0 or: [ nextIndex >= (startIndex + anInteger) ]) ] whileFalse.
        lastIndex >= (startIndex + anInteger) ifFalse: [
                stream greaseNext: startIndex + anInteger - lastIndex putAll: aByteString startingAt: lastIndex ]

But this certainly needs another pair of eyes.

Sven

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Johan Brichau-2
Thanks Sven!

3.1.1 is about to be released, so let's see if we can sneak this fix in.

Johan

On 20 May 2014, at 23:45, Sven Van Caekenberghe <[hidden email]> wrote:

>
> On 20 May 2014, at 23:15, Sven Van Caekenberghe <[hidden email]> wrote:
>
>> The error is in
>>
>> GRPharoUtf8CodecStream>>#greaseNext:putAllFast:startingAt:
>>
>> the first escape clause in the whileFalse computes a wrong first argument to #greaseNext:putAllFast:startingAt:
>>
>> however, it is a bit hard to say what the correct one should be, I will try with
>>
>> startIndex + anInteger - lastIndex
>>
>> I hope there are enough tests.
>
> I added an extra assert
>
> self assert: 'ab', umlaut next: 1 startingAt: 1 gives:  'a'.
>
> to GRPharoCodecTest>>#testGreaseNextPutAllStartingAt so that it now reads:
>
>   | umlaut encodedUmlaut |
> umlaut := String with: (Character value: 228).
> encodedUmlaut := String with: (Character value: 195) with: (Character value: 164).
> self assert: 'ab' next: 1 startingAt: 1 gives:  'a'.
> self assert: 'a', umlaut, 'b' next: 1 startingAt: 1 gives:  'a'.
> self assert: 'ab', umlaut next: 1 startingAt: 1 gives:  'a'.
> self assert: 'a', umlaut, 'b' next: 2 startingAt: 1 gives:  'a', encodedUmlaut.
> self assert: 'a', umlaut, 'b' next: 1 startingAt: 2 gives:  encodedUmlaut.
> self assert: 'a', umlaut, 'b' next: 2 startingAt: 2 gives:  encodedUmlaut, 'b'.
> self assert: 'a', umlaut, umlaut next: 2 startingAt: 2 gives:  encodedUmlaut, encodedUmlaut.
> self assert: 'ab', umlaut, 'b', umlaut next: 3 startingAt: 2 gives:  'b', encodedUmlaut, 'b'
>
> it fails before the proposed change, it succeeds after it - and the functional test now passes.
>
> Final code of GRPharoUtf8CodecStream>>#greaseNext:putAllFast:startingAt:
>
> | lastIndex nextIndex |
> lastIndex := startIndex.
> nextIndex := ByteString findFirstInString: aByteString inSet: Latin1ToUtf8Map startingAt: lastIndex.
> nextIndex = 0 ifTrue: [ ^ stream greaseNext: anInteger putAll: aByteString startingAt: startIndex ].
> [
> nextIndex >= (startIndex + anInteger) ifTrue: [
> ^ stream greaseNext: startIndex + anInteger - lastIndex putAll: aByteString startingAt: lastIndex ].
> nextIndex > lastIndex ifTrue: [
> stream greaseNext: nextIndex - lastIndex putAll: aByteString startingAt: lastIndex ].
> stream nextPutAll: (Latin1ToUtf8Encodings at: (aByteString byteAt: nextIndex) + 1).
> lastIndex := nextIndex + 1.
> nextIndex := ByteString findFirstInString: aByteString inSet: Latin1ToUtf8Map startingAt: lastIndex.
> (nextIndex = 0 or: [ nextIndex >= (startIndex + anInteger) ]) ] whileFalse.
> lastIndex >= (startIndex + anInteger) ifFalse: [
> stream greaseNext: startIndex + anInteger - lastIndex putAll: aByteString startingAt: lastIndex ]
>
> But this certainly needs another pair of eyes.
>
> Sven
>
> _______________________________________________
> seaside mailing list
> [hidden email]
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside

_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Philippe Marschall
In reply to this post by Sven Van Caekenberghe-2
On Tue, May 20, 2014 at 11:45 PM, Sven Van Caekenberghe <[hidden email]> wrote:

>
> On 20 May 2014, at 23:15, Sven Van Caekenberghe <[hidden email]> wrote:
>
>> The error is in
>>
>> GRPharoUtf8CodecStream>>#greaseNext:putAllFast:startingAt:
>>
>> the first escape clause in the whileFalse computes a wrong first argument to #greaseNext:putAllFast:startingAt:
>>
>> however, it is a bit hard to say what the correct one should be, I will try with
>>
>> startIndex + anInteger - lastIndex
>>
>> I hope there are enough tests.
>
> I added an extra assert
>
>         self assert: 'ab', umlaut next: 1 startingAt: 1 gives:  'a'.
>
> to GRPharoCodecTest>>#testGreaseNextPutAllStartingAt so that it now reads:
>
>         | umlaut encodedUmlaut |
>         umlaut := String with: (Character value: 228).
>         encodedUmlaut := String with: (Character value: 195) with: (Character value: 164).
>         self assert: 'ab' next: 1 startingAt: 1 gives:  'a'.
>         self assert: 'a', umlaut, 'b' next: 1 startingAt: 1 gives:  'a'.
>         self assert: 'ab', umlaut next: 1 startingAt: 1 gives:  'a'.
>         self assert: 'a', umlaut, 'b' next: 2 startingAt: 1 gives:  'a', encodedUmlaut.
>         self assert: 'a', umlaut, 'b' next: 1 startingAt: 2 gives:  encodedUmlaut.
>         self assert: 'a', umlaut, 'b' next: 2 startingAt: 2 gives:  encodedUmlaut, 'b'.
>         self assert: 'a', umlaut, umlaut next: 2 startingAt: 2 gives:  encodedUmlaut, encodedUmlaut.
>         self assert: 'ab', umlaut, 'b', umlaut next: 3 startingAt: 2 gives:  'b', encodedUmlaut, 'b'
>
> it fails before the proposed change, it succeeds after it - and the functional test now passes.

The test worked for me in Pharo 1.3 but fails on Pharo 3.0. When I
compared GRPharoUtf8CodecStream >> #greaseNext:putAll:startingAt: in
Pharo 1.3 and Pharo 3.0 they are different.

This is how it looks in Pharo 1.3

greaseNext: anInteger putAll: aCollection startingAt: startIndex
    aCollection isByteString
        ifTrue: [
            | index |
            index := ByteString findFirstInString: aCollection inSet:
Latin1ToUtf8Map startingAt: startIndex.
            (index = 0 or: [ index > (startIndex + anInteger - 1) ])
                ifTrue: [ stream greaseNext: anInteger putAll:
aCollection startingAt: startIndex ]
                ifFalse: [ super greaseNext: anInteger putAll:
aCollection startingAt: startIndex ] ]
        ifFalse: [ super greaseNext: anInteger putAll: aCollection
startingAt: startIndex ]

And this is how it looks in Pharo 3.0

greaseNext: anInteger putAll: aCollection startingAt: startIndex
    aCollection isByteString
        ifTrue: [ self greaseNext: anInteger putAllFast: aCollection
startingAt: startIndex ]
        ifFalse: [ super greaseNext: anInteger putAll: aCollection
startingAt: startIndex ]

I think the version in Pharo 3.0 is better because it can end up
sending #findFirstInString:inSet:startingAt: and #isByteString one
time less.

If I change the implementation in Pharo 3.0 the test succeeds. But
this is only because we don't end up sending
#greaseNext:putAllFast:startingAt: in this case and directly send
#greaseNext:putAllFast:startingAt:. The issue in
#findFirstInString:inSet:startingAt: remains. Your fix looks right (I
first determined what you be correct and then compared it with your
solution and came up with he same thing).

Thanks for all the work.

Sorry Johan I kinda took the issue from you, that was a bit rude. I
only saw it was assigned to you after I committed.

Philippe
_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
Reply | Threaded
Open this post in threaded view
|

Re: Encoding problem

Johan Brichau-2

On 21 May 2014, at 21:39, Philippe Marschall <[hidden email]> wrote:

> Sorry Johan I kinda took the issue from you, that was a bit rude. I
> only saw it was assigned to you after I committed.

Haha, my pleasure ;-)
It was assigned because I created it I guess

Thanks for fixing this!

Johan_______________________________________________
seaside mailing list
[hidden email]
http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside