NeoCSVReader and an empty field at the very end of a file

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

NeoCSVReader and an empty field at the very end of a file

jtuchel
Hi there,

I am on VA Smalltalk and therefor using an older version of NeoCSV
(SvenVanCaekenberghe.14). I found a bug in this old version that is
somewhat special.

It seems NeoCSV cannot handle the situation where the very last field is
just empty AND if there is no trailing CRLF at the end of the file.
Somethinng like this:

SecondLastColumnValue;;<EOF>

In that case, readField fails because it tries to do a readQuotedField
or readUnquotedField, both of which try to read beyond EOF.

So I changed readField to this:

readField

     ^self atEnd "In case the very last field of a file is empty, like
'45;56;;'"
         ifTrue: ['']
         ifFalse: [self peekQuote ifTrue: [self readQuotedField]
ifFalse: [self readUnquotedField]]

and all seems fine so far.

Side note: My original file has a trailing CrLf but if I upload it via a
browser to a Seaside Server, the Browser cuts off the trailing CrLf (I
can see this in the Browser's Network debugging tools - both in IE and
FF) - so it seems NeoCSV has to be ready for this situation.

Joachim


--
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          mailto:[hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

jtuchel
Hi,

I've tried porting SvenVanCaekenberghe.20 and see the same problems in
this version. IN addition to the fix already mentioned, I also had to
change readSeparator:

readSeparator

     ^self atEnd ifFalse: [self peekFor: separator]

As far as I can tell by now, this fixes the problem at hand. Any ideas
if this is a safe fix?

Joachim


Am 01.07.15 um 12:35 schrieb [hidden email]:

> Hi there,
>
> I am on VA Smalltalk and therefor using an older version of NeoCSV
> (SvenVanCaekenberghe.14). I found a bug in this old version that is
> somewhat special.
>
> It seems NeoCSV cannot handle the situation where the very last field
> is just empty AND if there is no trailing CRLF at the end of the file.
> Somethinng like this:
>
> SecondLastColumnValue;;<EOF>
>
> In that case, readField fails because it tries to do a readQuotedField
> or readUnquotedField, both of which try to read beyond EOF.
>
> So I changed readField to this:
>
> readField
>
>     ^self atEnd "In case the very last field of a file is empty, like
> '45;56;;'"
>         ifTrue: ['']
>         ifFalse: [self peekQuote ifTrue: [self readQuotedField]
> ifFalse: [self readUnquotedField]]
>
> and all seems fine so far.
>
> Side note: My original file has a trailing CrLf but if I upload it via
> a browser to a Seaside Server, the Browser cuts off the trailing CrLf
> (I can see this in the Browser's Network debugging tools - both in IE
> and FF) - so it seems NeoCSV has to be ready for this situation.
>
> Joachim
>
>


--
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          mailto:[hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Sven Van Caekenberghe-2
Hi Joachim,

First, thanks for the feedback.

Second, since you are on a different platform, that might be a factor.

Did you test your problem on Pharo itself ?

Because there are already unit tests specifically for the case you describe:

#testEmptyLastFieldUnquoted
#testEmptyLastFieldQuoted

These obviously pass for Pharo, do they pass for you ?

Maybe your problem case is slightly different though ?

Sven

> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>
> Hi,
>
> I've tried porting SvenVanCaekenberghe.20 and see the same problems in this version. IN addition to the fix already mentioned, I also had to change readSeparator:
>
> readSeparator
>
>    ^self atEnd ifFalse: [self peekFor: separator]
>
> As far as I can tell by now, this fixes the problem at hand. Any ideas if this is a safe fix?
>
> Joachim
>
>
> Am 01.07.15 um 12:35 schrieb [hidden email]:
>> Hi there,
>>
>> I am on VA Smalltalk and therefor using an older version of NeoCSV (SvenVanCaekenberghe.14). I found a bug in this old version that is somewhat special.
>>
>> It seems NeoCSV cannot handle the situation where the very last field is just empty AND if there is no trailing CRLF at the end of the file. Somethinng like this:
>>
>> SecondLastColumnValue;;<EOF>
>>
>> In that case, readField fails because it tries to do a readQuotedField or readUnquotedField, both of which try to read beyond EOF.
>>
>> So I changed readField to this:
>>
>> readField
>>
>>    ^self atEnd "In case the very last field of a file is empty, like '45;56;;'"
>>        ifTrue: ['']
>>        ifFalse: [self peekQuote ifTrue: [self readQuotedField] ifFalse: [self readUnquotedField]]
>>
>> and all seems fine so far.
>>
>> Side note: My original file has a trailing CrLf but if I upload it via a browser to a Seaside Server, the Browser cuts off the trailing CrLf (I can see this in the Browser's Network debugging tools - both in IE and FF) - so it seems NeoCSV has to be ready for this situation.
>>
>> Joachim
>>
>>
>
>
> --
> -----------------------------------------------------------------------
> Objektfabrik Joachim Tuchel          mailto:[hidden email]
> Fliederweg 1                         http://www.objektfabrik.de
> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>
>


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

jtuchel
Hi Sven,

I didn't test on Pharo. But I remember seeing differences in the way
Pharo and VAST react to reads beyond the end of a Stream.
Not sure, but this could have been in NeoCSV context two or three years
ago.

So it is very likely I was bitten by platform differences.

Thanks for answering.

Joachim

Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:

> Hi Joachim,
>
> First, thanks for the feedback.
>
> Second, since you are on a different platform, that might be a factor.
>
> Did you test your problem on Pharo itself ?
>
> Because there are already unit tests specifically for the case you describe:
>
> #testEmptyLastFieldUnquoted
> #testEmptyLastFieldQuoted
>
> These obviously pass for Pharo, do they pass for you ?
>
> Maybe your problem case is slightly different though ?
>
> Sven
>
>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>
>> Hi,
>>
>> I've tried porting SvenVanCaekenberghe.20 and see the same problems in this version. IN addition to the fix already mentioned, I also had to change readSeparator:
>>
>> readSeparator
>>
>>     ^self atEnd ifFalse: [self peekFor: separator]
>>
>> As far as I can tell by now, this fixes the problem at hand. Any ideas if this is a safe fix?
>>
>> Joachim
>>
>>
>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>> Hi there,
>>>
>>> I am on VA Smalltalk and therefor using an older version of NeoCSV (SvenVanCaekenberghe.14). I found a bug in this old version that is somewhat special.
>>>
>>> It seems NeoCSV cannot handle the situation where the very last field is just empty AND if there is no trailing CRLF at the end of the file. Somethinng like this:
>>>
>>> SecondLastColumnValue;;<EOF>
>>>
>>> In that case, readField fails because it tries to do a readQuotedField or readUnquotedField, both of which try to read beyond EOF.
>>>
>>> So I changed readField to this:
>>>
>>> readField
>>>
>>>     ^self atEnd "In case the very last field of a file is empty, like '45;56;;'"
>>>         ifTrue: ['']
>>>         ifFalse: [self peekQuote ifTrue: [self readQuotedField] ifFalse: [self readUnquotedField]]
>>>
>>> and all seems fine so far.
>>>
>>> Side note: My original file has a trailing CrLf but if I upload it via a browser to a Seaside Server, the Browser cuts off the trailing CrLf (I can see this in the Browser's Network debugging tools - both in IE and FF) - so it seems NeoCSV has to be ready for this situation.
>>>
>>> Joachim
>>>
>>>
>>
>> --
>> -----------------------------------------------------------------------
>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>> Fliederweg 1                         http://www.objektfabrik.de
>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>
>>
>
>


--
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          mailto:[hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Sven Van Caekenberghe-2
Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when #atEnd.

It is the way it is (I am personally for stricter semantics), but that fact is certainly used in code all allround the place.

> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>
> Hi Sven,
>
> I didn't test on Pharo. But I remember seeing differences in the way Pharo and VAST react to reads beyond the end of a Stream.
> Not sure, but this could have been in NeoCSV context two or three years ago.
>
> So it is very likely I was bitten by platform differences.
>
> Thanks for answering.
>
> Joachim
>
> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>> Hi Joachim,
>>
>> First, thanks for the feedback.
>>
>> Second, since you are on a different platform, that might be a factor.
>>
>> Did you test your problem on Pharo itself ?
>>
>> Because there are already unit tests specifically for the case you describe:
>>
>> #testEmptyLastFieldUnquoted
>> #testEmptyLastFieldQuoted
>>
>> These obviously pass for Pharo, do they pass for you ?
>>
>> Maybe your problem case is slightly different though ?
>>
>> Sven
>>
>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>
>>> Hi,
>>>
>>> I've tried porting SvenVanCaekenberghe.20 and see the same problems in this version. IN addition to the fix already mentioned, I also had to change readSeparator:
>>>
>>> readSeparator
>>>
>>>    ^self atEnd ifFalse: [self peekFor: separator]
>>>
>>> As far as I can tell by now, this fixes the problem at hand. Any ideas if this is a safe fix?
>>>
>>> Joachim
>>>
>>>
>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>> Hi there,
>>>>
>>>> I am on VA Smalltalk and therefor using an older version of NeoCSV (SvenVanCaekenberghe.14). I found a bug in this old version that is somewhat special.
>>>>
>>>> It seems NeoCSV cannot handle the situation where the very last field is just empty AND if there is no trailing CRLF at the end of the file. Somethinng like this:
>>>>
>>>> SecondLastColumnValue;;<EOF>
>>>>
>>>> In that case, readField fails because it tries to do a readQuotedField or readUnquotedField, both of which try to read beyond EOF.
>>>>
>>>> So I changed readField to this:
>>>>
>>>> readField
>>>>
>>>>    ^self atEnd "In case the very last field of a file is empty, like '45;56;;'"
>>>>        ifTrue: ['']
>>>>        ifFalse: [self peekQuote ifTrue: [self readQuotedField] ifFalse: [self readUnquotedField]]
>>>>
>>>> and all seems fine so far.
>>>>
>>>> Side note: My original file has a trailing CrLf but if I upload it via a browser to a Seaside Server, the Browser cuts off the trailing CrLf (I can see this in the Browser's Network debugging tools - both in IE and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>
>>>> Joachim
>>>>
>>>>
>>>
>>> --
>>> -----------------------------------------------------------------------
>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>> Fliederweg 1                         http://www.objektfabrik.de
>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>
>>>
>>
>>
>
>
> --
> -----------------------------------------------------------------------
> Objektfabrik Joachim Tuchel          mailto:[hidden email]
> Fliederweg 1                         http://www.objektfabrik.de
> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>
>


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

jtuchel
I am not complaining. It just makes porting harder.

What made the thing especially strange was that I only had that problem
with uploaded files because the Browser removes the trailing CrLf (or
better, doesn't add another one in the multipart form data). So reading
from a file all works like a charm, but not when you process the same,
uploaded file on the server side...

Joachim

Am 01.07.15 um 15:12 schrieb Sven Van Caekenberghe:

> Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when #atEnd.
>
> It is the way it is (I am personally for stricter semantics), but that fact is certainly used in code all allround the place.
>
>> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>>
>> Hi Sven,
>>
>> I didn't test on Pharo. But I remember seeing differences in the way Pharo and VAST react to reads beyond the end of a Stream.
>> Not sure, but this could have been in NeoCSV context two or three years ago.
>>
>> So it is very likely I was bitten by platform differences.
>>
>> Thanks for answering.
>>
>> Joachim
>>
>> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>>> Hi Joachim,
>>>
>>> First, thanks for the feedback.
>>>
>>> Second, since you are on a different platform, that might be a factor.
>>>
>>> Did you test your problem on Pharo itself ?
>>>
>>> Because there are already unit tests specifically for the case you describe:
>>>
>>> #testEmptyLastFieldUnquoted
>>> #testEmptyLastFieldQuoted
>>>
>>> These obviously pass for Pharo, do they pass for you ?
>>>
>>> Maybe your problem case is slightly different though ?
>>>
>>> Sven
>>>
>>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>>
>>>> Hi,
>>>>
>>>> I've tried porting SvenVanCaekenberghe.20 and see the same problems in this version. IN addition to the fix already mentioned, I also had to change readSeparator:
>>>>
>>>> readSeparator
>>>>
>>>>     ^self atEnd ifFalse: [self peekFor: separator]
>>>>
>>>> As far as I can tell by now, this fixes the problem at hand. Any ideas if this is a safe fix?
>>>>
>>>> Joachim
>>>>
>>>>
>>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>>> Hi there,
>>>>>
>>>>> I am on VA Smalltalk and therefor using an older version of NeoCSV (SvenVanCaekenberghe.14). I found a bug in this old version that is somewhat special.
>>>>>
>>>>> It seems NeoCSV cannot handle the situation where the very last field is just empty AND if there is no trailing CRLF at the end of the file. Somethinng like this:
>>>>>
>>>>> SecondLastColumnValue;;<EOF>
>>>>>
>>>>> In that case, readField fails because it tries to do a readQuotedField or readUnquotedField, both of which try to read beyond EOF.
>>>>>
>>>>> So I changed readField to this:
>>>>>
>>>>> readField
>>>>>
>>>>>     ^self atEnd "In case the very last field of a file is empty, like '45;56;;'"
>>>>>         ifTrue: ['']
>>>>>         ifFalse: [self peekQuote ifTrue: [self readQuotedField] ifFalse: [self readUnquotedField]]
>>>>>
>>>>> and all seems fine so far.
>>>>>
>>>>> Side note: My original file has a trailing CrLf but if I upload it via a browser to a Seaside Server, the Browser cuts off the trailing CrLf (I can see this in the Browser's Network debugging tools - both in IE and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>>
>>>>> Joachim
>>>>>
>>>>>
>>>> --
>>>> -----------------------------------------------------------------------
>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>
>>>>
>>>
>>
>> --
>> -----------------------------------------------------------------------
>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>> Fliederweg 1                         http://www.objektfabrik.de
>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>
>>
>
>


--
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          mailto:[hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Hannes Hirzel
Joachim,

which results do you get on VA Smalltalk for the tests?

Do they all pass?

--Hannes

On 7/1/15, [hidden email] <[hidden email]> wrote:

> I am not complaining. It just makes porting harder.
>
> What made the thing especially strange was that I only had that problem
> with uploaded files because the Browser removes the trailing CrLf (or
> better, doesn't add another one in the multipart form data). So reading
> from a file all works like a charm, but not when you process the same,
> uploaded file on the server side...
>
> Joachim
>
> Am 01.07.15 um 15:12 schrieb Sven Van Caekenberghe:
>> Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when #atEnd.
>>
>> It is the way it is (I am personally for stricter semantics), but that
>> fact is certainly used in code all allround the place.
>>
>>> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>>>
>>> Hi Sven,
>>>
>>> I didn't test on Pharo. But I remember seeing differences in the way
>>> Pharo and VAST react to reads beyond the end of a Stream.
>>> Not sure, but this could have been in NeoCSV context two or three years
>>> ago.
>>>
>>> So it is very likely I was bitten by platform differences.
>>>
>>> Thanks for answering.
>>>
>>> Joachim
>>>
>>> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>>>> Hi Joachim,
>>>>
>>>> First, thanks for the feedback.
>>>>
>>>> Second, since you are on a different platform, that might be a factor.
>>>>
>>>> Did you test your problem on Pharo itself ?
>>>>
>>>> Because there are already unit tests specifically for the case you
>>>> describe:
>>>>
>>>> #testEmptyLastFieldUnquoted
>>>> #testEmptyLastFieldQuoted
>>>>
>>>> These obviously pass for Pharo, do they pass for you ?
>>>>
>>>> Maybe your problem case is slightly different though ?
>>>>
>>>> Sven
>>>>
>>>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I've tried porting SvenVanCaekenberghe.20 and see the same problems in
>>>>> this version. IN addition to the fix already mentioned, I also had to
>>>>> change readSeparator:
>>>>>
>>>>> readSeparator
>>>>>
>>>>>     ^self atEnd ifFalse: [self peekFor: separator]
>>>>>
>>>>> As far as I can tell by now, this fixes the problem at hand. Any ideas
>>>>> if this is a safe fix?
>>>>>
>>>>> Joachim
>>>>>
>>>>>
>>>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>>>> Hi there,
>>>>>>
>>>>>> I am on VA Smalltalk and therefor using an older version of NeoCSV
>>>>>> (SvenVanCaekenberghe.14). I found a bug in this old version that is
>>>>>> somewhat special.
>>>>>>
>>>>>> It seems NeoCSV cannot handle the situation where the very last field
>>>>>> is just empty AND if there is no trailing CRLF at the end of the file.
>>>>>> Somethinng like this:
>>>>>>
>>>>>> SecondLastColumnValue;;<EOF>
>>>>>>
>>>>>> In that case, readField fails because it tries to do a readQuotedField
>>>>>> or readUnquotedField, both of which try to read beyond EOF.
>>>>>>
>>>>>> So I changed readField to this:
>>>>>>
>>>>>> readField
>>>>>>
>>>>>>     ^self atEnd "In case the very last field of a file is empty, like
>>>>>> '45;56;;'"
>>>>>>         ifTrue: ['']
>>>>>>         ifFalse: [self peekQuote ifTrue: [self readQuotedField]
>>>>>> ifFalse: [self readUnquotedField]]
>>>>>>
>>>>>> and all seems fine so far.
>>>>>>
>>>>>> Side note: My original file has a trailing CrLf but if I upload it via
>>>>>> a browser to a Seaside Server, the Browser cuts off the trailing CrLf
>>>>>> (I can see this in the Browser's Network debugging tools - both in IE
>>>>>> and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>>>
>>>>>> Joachim
>>>>>>
>>>>>>
>>>>> --
>>>>> -----------------------------------------------------------------------
>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>> D-71640 Ludwigsburg
>>>>> http://joachimtuchel.wordpress.com
>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>
>>>>>
>>>>
>>>
>>> --
>>> -----------------------------------------------------------------------
>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>> Fliederweg 1                         http://www.objektfabrik.de
>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>
>>>
>>
>>
>
>
> --
> -----------------------------------------------------------------------
> Objektfabrik Joachim Tuchel          mailto:[hidden email]
> Fliederweg 1                         http://www.objektfabrik.de
> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

jtuchel
I gave up on porting tests. VAST doesn't support creating Arrays with
curly braces, and they are used all over the place in Pharo code. I wuld
use them if I had them, so this is not an accuse or anything. It's just
a huge load of work to keep tests up to date when porting newer
versions, so I just ignore tests when porting. A sad truth.

Joachim

Am 01.07.15 um 16:14 schrieb H. Hirzel:

> Joachim,
>
> which results do you get on VA Smalltalk for the tests?
>
> Do they all pass?
>
> --Hannes
>
> On 7/1/15, [hidden email] <[hidden email]> wrote:
>> I am not complaining. It just makes porting harder.
>>
>> What made the thing especially strange was that I only had that problem
>> with uploaded files because the Browser removes the trailing CrLf (or
>> better, doesn't add another one in the multipart form data). So reading
>> from a file all works like a charm, but not when you process the same,
>> uploaded file on the server side...
>>
>> Joachim
>>
>> Am 01.07.15 um 15:12 schrieb Sven Van Caekenberghe:
>>> Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when #atEnd.
>>>
>>> It is the way it is (I am personally for stricter semantics), but that
>>> fact is certainly used in code all allround the place.
>>>
>>>> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>>>>
>>>> Hi Sven,
>>>>
>>>> I didn't test on Pharo. But I remember seeing differences in the way
>>>> Pharo and VAST react to reads beyond the end of a Stream.
>>>> Not sure, but this could have been in NeoCSV context two or three years
>>>> ago.
>>>>
>>>> So it is very likely I was bitten by platform differences.
>>>>
>>>> Thanks for answering.
>>>>
>>>> Joachim
>>>>
>>>> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>>>>> Hi Joachim,
>>>>>
>>>>> First, thanks for the feedback.
>>>>>
>>>>> Second, since you are on a different platform, that might be a factor.
>>>>>
>>>>> Did you test your problem on Pharo itself ?
>>>>>
>>>>> Because there are already unit tests specifically for the case you
>>>>> describe:
>>>>>
>>>>> #testEmptyLastFieldUnquoted
>>>>> #testEmptyLastFieldQuoted
>>>>>
>>>>> These obviously pass for Pharo, do they pass for you ?
>>>>>
>>>>> Maybe your problem case is slightly different though ?
>>>>>
>>>>> Sven
>>>>>
>>>>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I've tried porting SvenVanCaekenberghe.20 and see the same problems in
>>>>>> this version. IN addition to the fix already mentioned, I also had to
>>>>>> change readSeparator:
>>>>>>
>>>>>> readSeparator
>>>>>>
>>>>>>      ^self atEnd ifFalse: [self peekFor: separator]
>>>>>>
>>>>>> As far as I can tell by now, this fixes the problem at hand. Any ideas
>>>>>> if this is a safe fix?
>>>>>>
>>>>>> Joachim
>>>>>>
>>>>>>
>>>>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>>>>> Hi there,
>>>>>>>
>>>>>>> I am on VA Smalltalk and therefor using an older version of NeoCSV
>>>>>>> (SvenVanCaekenberghe.14). I found a bug in this old version that is
>>>>>>> somewhat special.
>>>>>>>
>>>>>>> It seems NeoCSV cannot handle the situation where the very last field
>>>>>>> is just empty AND if there is no trailing CRLF at the end of the file.
>>>>>>> Somethinng like this:
>>>>>>>
>>>>>>> SecondLastColumnValue;;<EOF>
>>>>>>>
>>>>>>> In that case, readField fails because it tries to do a readQuotedField
>>>>>>> or readUnquotedField, both of which try to read beyond EOF.
>>>>>>>
>>>>>>> So I changed readField to this:
>>>>>>>
>>>>>>> readField
>>>>>>>
>>>>>>>      ^self atEnd "In case the very last field of a file is empty, like
>>>>>>> '45;56;;'"
>>>>>>>          ifTrue: ['']
>>>>>>>          ifFalse: [self peekQuote ifTrue: [self readQuotedField]
>>>>>>> ifFalse: [self readUnquotedField]]
>>>>>>>
>>>>>>> and all seems fine so far.
>>>>>>>
>>>>>>> Side note: My original file has a trailing CrLf but if I upload it via
>>>>>>> a browser to a Seaside Server, the Browser cuts off the trailing CrLf
>>>>>>> (I can see this in the Browser's Network debugging tools - both in IE
>>>>>>> and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>>>>
>>>>>>> Joachim
>>>>>>>
>>>>>>>
>>>>>> --
>>>>>> -----------------------------------------------------------------------
>>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>>> D-71640 Ludwigsburg
>>>>>> http://joachimtuchel.wordpress.com
>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>
>>>>>>
>>>> --
>>>> -----------------------------------------------------------------------
>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>
>>>>
>>>
>>
>> --
>> -----------------------------------------------------------------------
>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>> Fliederweg 1                         http://www.objektfabrik.de
>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>
>>
>>
>


--
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          mailto:[hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Thierry Goubier
Le 01/07/2015 21:07, [hidden email] a écrit :
> I gave up on porting tests. VAST doesn't support creating Arrays with
> curly braces, and they are used all over the place in Pharo code. I wuld
> use them if I had them, so this is not an accuse or anything. It's just
> a huge load of work to keep tests up to date when porting newer
> versions, so I just ignore tests when porting. A sad truth.

Maybe we should write a refactoring to change those curly braces into an
equivalent Array based syntax?

I'll have some significant porting from Pharo to do one of these days,
so maybe I'll setup something for that.

Thierry

>
> Joachim
>
> Am 01.07.15 um 16:14 schrieb H. Hirzel:
>> Joachim,
>>
>> which results do you get on VA Smalltalk for the tests?
>>
>> Do they all pass?
>>
>> --Hannes
>>
>> On 7/1/15, [hidden email] <[hidden email]> wrote:
>>> I am not complaining. It just makes porting harder.
>>>
>>> What made the thing especially strange was that I only had that problem
>>> with uploaded files because the Browser removes the trailing CrLf (or
>>> better, doesn't add another one in the multipart form data). So reading
>>> from a file all works like a charm, but not when you process the same,
>>> uploaded file on the server side...
>>>
>>> Joachim
>>>
>>> Am 01.07.15 um 15:12 schrieb Sven Van Caekenberghe:
>>>> Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when
>>>> #atEnd.
>>>>
>>>> It is the way it is (I am personally for stricter semantics), but that
>>>> fact is certainly used in code all allround the place.
>>>>
>>>>> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>>>>>
>>>>> Hi Sven,
>>>>>
>>>>> I didn't test on Pharo. But I remember seeing differences in the way
>>>>> Pharo and VAST react to reads beyond the end of a Stream.
>>>>> Not sure, but this could have been in NeoCSV context two or three
>>>>> years
>>>>> ago.
>>>>>
>>>>> So it is very likely I was bitten by platform differences.
>>>>>
>>>>> Thanks for answering.
>>>>>
>>>>> Joachim
>>>>>
>>>>> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>>>>>> Hi Joachim,
>>>>>>
>>>>>> First, thanks for the feedback.
>>>>>>
>>>>>> Second, since you are on a different platform, that might be a
>>>>>> factor.
>>>>>>
>>>>>> Did you test your problem on Pharo itself ?
>>>>>>
>>>>>> Because there are already unit tests specifically for the case you
>>>>>> describe:
>>>>>>
>>>>>> #testEmptyLastFieldUnquoted
>>>>>> #testEmptyLastFieldQuoted
>>>>>>
>>>>>> These obviously pass for Pharo, do they pass for you ?
>>>>>>
>>>>>> Maybe your problem case is slightly different though ?
>>>>>>
>>>>>> Sven
>>>>>>
>>>>>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I've tried porting SvenVanCaekenberghe.20 and see the same
>>>>>>> problems in
>>>>>>> this version. IN addition to the fix already mentioned, I also
>>>>>>> had to
>>>>>>> change readSeparator:
>>>>>>>
>>>>>>> readSeparator
>>>>>>>
>>>>>>>      ^self atEnd ifFalse: [self peekFor: separator]
>>>>>>>
>>>>>>> As far as I can tell by now, this fixes the problem at hand. Any
>>>>>>> ideas
>>>>>>> if this is a safe fix?
>>>>>>>
>>>>>>> Joachim
>>>>>>>
>>>>>>>
>>>>>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>>>>>> Hi there,
>>>>>>>>
>>>>>>>> I am on VA Smalltalk and therefor using an older version of NeoCSV
>>>>>>>> (SvenVanCaekenberghe.14). I found a bug in this old version that is
>>>>>>>> somewhat special.
>>>>>>>>
>>>>>>>> It seems NeoCSV cannot handle the situation where the very last
>>>>>>>> field
>>>>>>>> is just empty AND if there is no trailing CRLF at the end of the
>>>>>>>> file.
>>>>>>>> Somethinng like this:
>>>>>>>>
>>>>>>>> SecondLastColumnValue;;<EOF>
>>>>>>>>
>>>>>>>> In that case, readField fails because it tries to do a
>>>>>>>> readQuotedField
>>>>>>>> or readUnquotedField, both of which try to read beyond EOF.
>>>>>>>>
>>>>>>>> So I changed readField to this:
>>>>>>>>
>>>>>>>> readField
>>>>>>>>
>>>>>>>>      ^self atEnd "In case the very last field of a file is
>>>>>>>> empty, like
>>>>>>>> '45;56;;'"
>>>>>>>>          ifTrue: ['']
>>>>>>>>          ifFalse: [self peekQuote ifTrue: [self readQuotedField]
>>>>>>>> ifFalse: [self readUnquotedField]]
>>>>>>>>
>>>>>>>> and all seems fine so far.
>>>>>>>>
>>>>>>>> Side note: My original file has a trailing CrLf but if I upload
>>>>>>>> it via
>>>>>>>> a browser to a Seaside Server, the Browser cuts off the trailing
>>>>>>>> CrLf
>>>>>>>> (I can see this in the Browser's Network debugging tools - both
>>>>>>>> in IE
>>>>>>>> and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>>>>>
>>>>>>>> Joachim
>>>>>>>>
>>>>>>>>
>>>>>>> --
>>>>>>> -----------------------------------------------------------------------
>>>>>>>
>>>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>>>> D-71640 Ludwigsburg
>>>>>>> http://joachimtuchel.wordpress.com
>>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>>
>>>>>>>
>>>>> --
>>>>> -----------------------------------------------------------------------
>>>>>
>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>> D-71640 Ludwigsburg
>>>>> http://joachimtuchel.wordpress.com
>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>
>>>>>
>>>>
>>>
>>> --
>>> -----------------------------------------------------------------------
>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>> Fliederweg 1                         http://www.objektfabrik.de
>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>
>>>
>>>
>>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

jtuchel
You mean RewriteRules? I was planning to do that, for this exact reason.
But you know how things go when there is a long list of priorities.
I know it saves a lot of time and makes porting so much more safe, but
it needs an up-front investment in time...

Joachim

Am 01.07.15 um 21:10 schrieb Thierry Goubier:

> Le 01/07/2015 21:07, [hidden email] a écrit :
>> I gave up on porting tests. VAST doesn't support creating Arrays with
>> curly braces, and they are used all over the place in Pharo code. I wuld
>> use them if I had them, so this is not an accuse or anything. It's just
>> a huge load of work to keep tests up to date when porting newer
>> versions, so I just ignore tests when porting. A sad truth.
>
> Maybe we should write a refactoring to change those curly braces into
> an equivalent Array based syntax?
>
> I'll have some significant porting from Pharo to do one of these days,
> so maybe I'll setup something for that.
>
> Thierry
>
>>
>> Joachim
>>
>> Am 01.07.15 um 16:14 schrieb H. Hirzel:
>>> Joachim,
>>>
>>> which results do you get on VA Smalltalk for the tests?
>>>
>>> Do they all pass?
>>>
>>> --Hannes
>>>
>>> On 7/1/15, [hidden email] <[hidden email]> wrote:
>>>> I am not complaining. It just makes porting harder.
>>>>
>>>> What made the thing especially strange was that I only had that
>>>> problem
>>>> with uploaded files because the Browser removes the trailing CrLf (or
>>>> better, doesn't add another one in the multipart form data). So
>>>> reading
>>>> from a file all works like a charm, but not when you process the same,
>>>> uploaded file on the server side...
>>>>
>>>> Joachim
>>>>
>>>> Am 01.07.15 um 15:12 schrieb Sven Van Caekenberghe:
>>>>> Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when
>>>>> #atEnd.
>>>>>
>>>>> It is the way it is (I am personally for stricter semantics), but
>>>>> that
>>>>> fact is certainly used in code all allround the place.
>>>>>
>>>>>> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>>>>>>
>>>>>> Hi Sven,
>>>>>>
>>>>>> I didn't test on Pharo. But I remember seeing differences in the way
>>>>>> Pharo and VAST react to reads beyond the end of a Stream.
>>>>>> Not sure, but this could have been in NeoCSV context two or three
>>>>>> years
>>>>>> ago.
>>>>>>
>>>>>> So it is very likely I was bitten by platform differences.
>>>>>>
>>>>>> Thanks for answering.
>>>>>>
>>>>>> Joachim
>>>>>>
>>>>>> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>>>>>>> Hi Joachim,
>>>>>>>
>>>>>>> First, thanks for the feedback.
>>>>>>>
>>>>>>> Second, since you are on a different platform, that might be a
>>>>>>> factor.
>>>>>>>
>>>>>>> Did you test your problem on Pharo itself ?
>>>>>>>
>>>>>>> Because there are already unit tests specifically for the case you
>>>>>>> describe:
>>>>>>>
>>>>>>> #testEmptyLastFieldUnquoted
>>>>>>> #testEmptyLastFieldQuoted
>>>>>>>
>>>>>>> These obviously pass for Pharo, do they pass for you ?
>>>>>>>
>>>>>>> Maybe your problem case is slightly different though ?
>>>>>>>
>>>>>>> Sven
>>>>>>>
>>>>>>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I've tried porting SvenVanCaekenberghe.20 and see the same
>>>>>>>> problems in
>>>>>>>> this version. IN addition to the fix already mentioned, I also
>>>>>>>> had to
>>>>>>>> change readSeparator:
>>>>>>>>
>>>>>>>> readSeparator
>>>>>>>>
>>>>>>>>      ^self atEnd ifFalse: [self peekFor: separator]
>>>>>>>>
>>>>>>>> As far as I can tell by now, this fixes the problem at hand. Any
>>>>>>>> ideas
>>>>>>>> if this is a safe fix?
>>>>>>>>
>>>>>>>> Joachim
>>>>>>>>
>>>>>>>>
>>>>>>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>>>>>>> Hi there,
>>>>>>>>>
>>>>>>>>> I am on VA Smalltalk and therefor using an older version of
>>>>>>>>> NeoCSV
>>>>>>>>> (SvenVanCaekenberghe.14). I found a bug in this old version
>>>>>>>>> that is
>>>>>>>>> somewhat special.
>>>>>>>>>
>>>>>>>>> It seems NeoCSV cannot handle the situation where the very last
>>>>>>>>> field
>>>>>>>>> is just empty AND if there is no trailing CRLF at the end of the
>>>>>>>>> file.
>>>>>>>>> Somethinng like this:
>>>>>>>>>
>>>>>>>>> SecondLastColumnValue;;<EOF>
>>>>>>>>>
>>>>>>>>> In that case, readField fails because it tries to do a
>>>>>>>>> readQuotedField
>>>>>>>>> or readUnquotedField, both of which try to read beyond EOF.
>>>>>>>>>
>>>>>>>>> So I changed readField to this:
>>>>>>>>>
>>>>>>>>> readField
>>>>>>>>>
>>>>>>>>>      ^self atEnd "In case the very last field of a file is
>>>>>>>>> empty, like
>>>>>>>>> '45;56;;'"
>>>>>>>>>          ifTrue: ['']
>>>>>>>>>          ifFalse: [self peekQuote ifTrue: [self readQuotedField]
>>>>>>>>> ifFalse: [self readUnquotedField]]
>>>>>>>>>
>>>>>>>>> and all seems fine so far.
>>>>>>>>>
>>>>>>>>> Side note: My original file has a trailing CrLf but if I upload
>>>>>>>>> it via
>>>>>>>>> a browser to a Seaside Server, the Browser cuts off the trailing
>>>>>>>>> CrLf
>>>>>>>>> (I can see this in the Browser's Network debugging tools - both
>>>>>>>>> in IE
>>>>>>>>> and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>>>>>>
>>>>>>>>> Joachim
>>>>>>>>>
>>>>>>>>>
>>>>>>>> --
>>>>>>>> -----------------------------------------------------------------------
>>>>>>>>
>>>>>>>>
>>>>>>>> Objektfabrik Joachim Tuchel mailto:[hidden email]
>>>>>>>> Fliederweg 1 http://www.objektfabrik.de
>>>>>>>> D-71640 Ludwigsburg
>>>>>>>> http://joachimtuchel.wordpress.com
>>>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>>>
>>>>>>>>
>>>>>> --
>>>>>> -----------------------------------------------------------------------
>>>>>>
>>>>>>
>>>>>> Objektfabrik Joachim Tuchel mailto:[hidden email]
>>>>>> Fliederweg 1 http://www.objektfabrik.de
>>>>>> D-71640 Ludwigsburg
>>>>>> http://joachimtuchel.wordpress.com
>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>
>>>>>>
>>>>>
>>>>
>>>> --
>>>> -----------------------------------------------------------------------
>>>>
>>>> Objektfabrik Joachim Tuchel mailto:[hidden email]
>>>> Fliederweg 1 http://www.objektfabrik.de
>>>> D-71640 Ludwigsburg http://joachimtuchel.wordpress.com
>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>
>>>>
>>>>
>>>
>>
>>
>
>
>


--
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          mailto:[hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Sven Van Caekenberghe-2
In reply to this post by jtuchel
But before you ask for help on the Pharo ML you should run the NeoCSV code on Pharo to validate that your assumption hold there. If not, it is a porting error/problem.

And unit tests are *VERY IMPORTANT* to maintain our sanity across versions and/or platforms.

Again, what you report as a potential problem isn't one, I think. Please test on Pharo itself.

> On 01 Jul 2015, at 21:07, [hidden email] wrote:
>
> I gave up on porting tests. VAST doesn't support creating Arrays with curly braces, and they are used all over the place in Pharo code. I wuld use them if I had them, so this is not an accuse or anything. It's just a huge load of work to keep tests up to date when porting newer versions, so I just ignore tests when porting. A sad truth.
>
> Joachim
>
> Am 01.07.15 um 16:14 schrieb H. Hirzel:
>> Joachim,
>>
>> which results do you get on VA Smalltalk for the tests?
>>
>> Do they all pass?
>>
>> --Hannes
>>
>> On 7/1/15, [hidden email] <[hidden email]> wrote:
>>> I am not complaining. It just makes porting harder.
>>>
>>> What made the thing especially strange was that I only had that problem
>>> with uploaded files because the Browser removes the trailing CrLf (or
>>> better, doesn't add another one in the multipart form data). So reading
>>> from a file all works like a charm, but not when you process the same,
>>> uploaded file on the server side...
>>>
>>> Joachim
>>>
>>> Am 01.07.15 um 15:12 schrieb Sven Van Caekenberghe:
>>>> Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when #atEnd.
>>>>
>>>> It is the way it is (I am personally for stricter semantics), but that
>>>> fact is certainly used in code all allround the place.
>>>>
>>>>> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>>>>>
>>>>> Hi Sven,
>>>>>
>>>>> I didn't test on Pharo. But I remember seeing differences in the way
>>>>> Pharo and VAST react to reads beyond the end of a Stream.
>>>>> Not sure, but this could have been in NeoCSV context two or three years
>>>>> ago.
>>>>>
>>>>> So it is very likely I was bitten by platform differences.
>>>>>
>>>>> Thanks for answering.
>>>>>
>>>>> Joachim
>>>>>
>>>>> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>>>>>> Hi Joachim,
>>>>>>
>>>>>> First, thanks for the feedback.
>>>>>>
>>>>>> Second, since you are on a different platform, that might be a factor.
>>>>>>
>>>>>> Did you test your problem on Pharo itself ?
>>>>>>
>>>>>> Because there are already unit tests specifically for the case you
>>>>>> describe:
>>>>>>
>>>>>> #testEmptyLastFieldUnquoted
>>>>>> #testEmptyLastFieldQuoted
>>>>>>
>>>>>> These obviously pass for Pharo, do they pass for you ?
>>>>>>
>>>>>> Maybe your problem case is slightly different though ?
>>>>>>
>>>>>> Sven
>>>>>>
>>>>>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I've tried porting SvenVanCaekenberghe.20 and see the same problems in
>>>>>>> this version. IN addition to the fix already mentioned, I also had to
>>>>>>> change readSeparator:
>>>>>>>
>>>>>>> readSeparator
>>>>>>>
>>>>>>>     ^self atEnd ifFalse: [self peekFor: separator]
>>>>>>>
>>>>>>> As far as I can tell by now, this fixes the problem at hand. Any ideas
>>>>>>> if this is a safe fix?
>>>>>>>
>>>>>>> Joachim
>>>>>>>
>>>>>>>
>>>>>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>>>>>> Hi there,
>>>>>>>>
>>>>>>>> I am on VA Smalltalk and therefor using an older version of NeoCSV
>>>>>>>> (SvenVanCaekenberghe.14). I found a bug in this old version that is
>>>>>>>> somewhat special.
>>>>>>>>
>>>>>>>> It seems NeoCSV cannot handle the situation where the very last field
>>>>>>>> is just empty AND if there is no trailing CRLF at the end of the file.
>>>>>>>> Somethinng like this:
>>>>>>>>
>>>>>>>> SecondLastColumnValue;;<EOF>
>>>>>>>>
>>>>>>>> In that case, readField fails because it tries to do a readQuotedField
>>>>>>>> or readUnquotedField, both of which try to read beyond EOF.
>>>>>>>>
>>>>>>>> So I changed readField to this:
>>>>>>>>
>>>>>>>> readField
>>>>>>>>
>>>>>>>>     ^self atEnd "In case the very last field of a file is empty, like
>>>>>>>> '45;56;;'"
>>>>>>>>         ifTrue: ['']
>>>>>>>>         ifFalse: [self peekQuote ifTrue: [self readQuotedField]
>>>>>>>> ifFalse: [self readUnquotedField]]
>>>>>>>>
>>>>>>>> and all seems fine so far.
>>>>>>>>
>>>>>>>> Side note: My original file has a trailing CrLf but if I upload it via
>>>>>>>> a browser to a Seaside Server, the Browser cuts off the trailing CrLf
>>>>>>>> (I can see this in the Browser's Network debugging tools - both in IE
>>>>>>>> and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>>>>>
>>>>>>>> Joachim
>>>>>>>>
>>>>>>>>
>>>>>>> --
>>>>>>> -----------------------------------------------------------------------
>>>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>>>> D-71640 Ludwigsburg
>>>>>>> http://joachimtuchel.wordpress.com
>>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>>
>>>>>>>
>>>>> --
>>>>> -----------------------------------------------------------------------
>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>
>>>>>
>>>>
>>>
>>> --
>>> -----------------------------------------------------------------------
>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>> Fliederweg 1                         http://www.objektfabrik.de
>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>
>>>
>>>
>>
>
>
> --
> -----------------------------------------------------------------------
> Objektfabrik Joachim Tuchel          mailto:[hidden email]
> Fliederweg 1                         http://www.objektfabrik.de
> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>
>


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Thierry Goubier
In reply to this post by jtuchel
Le 01/07/2015 21:16, [hidden email] a écrit :
> You mean RewriteRules? I was planning to do that, for this exact reason.
> But you know how things go when there is a long list of priorities.
> I know it saves a lot of time and makes porting so much more safe, but
> it needs an up-front investment in time...

Yes, rewrite rules. Mark Rizun GUI attempts are a very interesting
direction to handle that...

Oh, by the way, Mark, if you're listening... I did what you were trying
to do by changing AST nodes positions in Pharo, but on a SmaCC based AST
for another language. I have to admit that you were right to try to do
it that way, but that the Pharo/RB AST infrastructure wasn't suitable
whereas SmaCC infrastructure is.

Thierry

Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

jtuchel
In reply to this post by Sven Van Caekenberghe-2
Sven,

you are, of course, right!
I should have checked in Pharo first. But since my version is old
anyways, I would have to try with at least two versions in order to make
sure if the bug is fixed or not. As long as keeping ports current is a
lot of work due to (increasing) differences between Phar and most other
dialects, this is hard work ;-) I should invest in it anyways, however,
since I am using your code from Pharo and you didn't ask me to use it...

So, to address this Array construction issue (as an example of others).
This cannot be handled with Grease or the like, since what is in my way
is the Smalltalk compiler (I am trying to concince Instantiations to
support {} for literal Array construction, but until I'm successful...).

Given somebody wrote RewriteRules to replace {} with "(Array new: x)
addAll:", this should probably be used on non-Pharo-platforms as part of
a port, so that the "original" code is still the Metacello version it
was on Pharo.

My plan for a port:
1. download .mcz in its "original" version and don't modify it
2. import and modify on target platform
3. test and publish on target platform's public repository

What do you think would be a good process to porting when it involves
such steps?

The problem with Step 2 is: I am not sure if RewriteRules can work in
code that's not compiled. (Well, I am mostly sure this won't work, just
asking for confirmation) You cannot compile methods with Arrays
constructed with curly braces on VAST.

So another possible process would be:
1. use Pharo to modify downloaded code in order to make it importable on
target platform
2. publish this code somewhere (not official Pharo repositories, maybe
StHub)
3. Download the modified version and import into target dialect
4. Make final adjustments to make code and tests run
5. Publish to target platform's public repository

Any thoughts or ideas? People interested in joining efforts?

And, now that we're on a new topic anyways, misusing this thread (mea
culpa):

I still wonder how we could make it easier to contribute back. Every
time I find a bug in some code from Pharo, there are several things to
consider:
1. Is there a newer version on Pharo that doesn't have this bug?
     a) yes: port newer version
     b) no: I'm in trouble - see below

in case of 1b), there are two options now:
1. I fire up Pharo and fix the bug in the original code, then commit the
changes to the official repository/ies, and then port the fixed version
2. I contact the ML or the author/maintainer of the code and give them a
bug report and hopefully a fix, which may or may not be compatible with
Pharo. Then I can either port the fixed version or just keep my fixed
version (if the fix is the same, this is not a problem from the code
perspective, but maybe from a version numbering standpoint).

I am interested in ideas here and would like to hear from people
interested in discussing these issues. Maybe ESUG would be a good
opportunity to start discussing. Maybe we can start a new initiative for
this problem area...

Joachim



Am 01.07.15 um 21:27 schrieb Sven Van Caekenberghe:

> But before you ask for help on the Pharo ML you should run the NeoCSV code on Pharo to validate that your assumption hold there. If not, it is a porting error/problem.
>
> And unit tests are *VERY IMPORTANT* to maintain our sanity across versions and/or platforms.
>
> Again, what you report as a potential problem isn't one, I think. Please test on Pharo itself.
>
>> On 01 Jul 2015, at 21:07, [hidden email] wrote:
>>
>> I gave up on porting tests. VAST doesn't support creating Arrays with curly braces, and they are used all over the place in Pharo code. I wuld use them if I had them, so this is not an accuse or anything. It's just a huge load of work to keep tests up to date when porting newer versions, so I just ignore tests when porting. A sad truth.
>>
>> Joachim
>>
>> Am 01.07.15 um 16:14 schrieb H. Hirzel:
>>> Joachim,
>>>
>>> which results do you get on VA Smalltalk for the tests?
>>>
>>> Do they all pass?
>>>
>>> --Hannes
>>>
>>> On 7/1/15, [hidden email] <[hidden email]> wrote:
>>>> I am not complaining. It just makes porting harder.
>>>>
>>>> What made the thing especially strange was that I only had that problem
>>>> with uploaded files because the Browser removes the trailing CrLf (or
>>>> better, doesn't add another one in the multipart form data). So reading
>>>> from a file all works like a charm, but not when you process the same,
>>>> uploaded file on the server side...
>>>>
>>>> Joachim
>>>>
>>>> Am 01.07.15 um 15:12 schrieb Sven Van Caekenberghe:
>>>>> Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when #atEnd.
>>>>>
>>>>> It is the way it is (I am personally for stricter semantics), but that
>>>>> fact is certainly used in code all allround the place.
>>>>>
>>>>>> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>>>>>>
>>>>>> Hi Sven,
>>>>>>
>>>>>> I didn't test on Pharo. But I remember seeing differences in the way
>>>>>> Pharo and VAST react to reads beyond the end of a Stream.
>>>>>> Not sure, but this could have been in NeoCSV context two or three years
>>>>>> ago.
>>>>>>
>>>>>> So it is very likely I was bitten by platform differences.
>>>>>>
>>>>>> Thanks for answering.
>>>>>>
>>>>>> Joachim
>>>>>>
>>>>>> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>>>>>>> Hi Joachim,
>>>>>>>
>>>>>>> First, thanks for the feedback.
>>>>>>>
>>>>>>> Second, since you are on a different platform, that might be a factor.
>>>>>>>
>>>>>>> Did you test your problem on Pharo itself ?
>>>>>>>
>>>>>>> Because there are already unit tests specifically for the case you
>>>>>>> describe:
>>>>>>>
>>>>>>> #testEmptyLastFieldUnquoted
>>>>>>> #testEmptyLastFieldQuoted
>>>>>>>
>>>>>>> These obviously pass for Pharo, do they pass for you ?
>>>>>>>
>>>>>>> Maybe your problem case is slightly different though ?
>>>>>>>
>>>>>>> Sven
>>>>>>>
>>>>>>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I've tried porting SvenVanCaekenberghe.20 and see the same problems in
>>>>>>>> this version. IN addition to the fix already mentioned, I also had to
>>>>>>>> change readSeparator:
>>>>>>>>
>>>>>>>> readSeparator
>>>>>>>>
>>>>>>>>      ^self atEnd ifFalse: [self peekFor: separator]
>>>>>>>>
>>>>>>>> As far as I can tell by now, this fixes the problem at hand. Any ideas
>>>>>>>> if this is a safe fix?
>>>>>>>>
>>>>>>>> Joachim
>>>>>>>>
>>>>>>>>
>>>>>>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>>>>>>> Hi there,
>>>>>>>>>
>>>>>>>>> I am on VA Smalltalk and therefor using an older version of NeoCSV
>>>>>>>>> (SvenVanCaekenberghe.14). I found a bug in this old version that is
>>>>>>>>> somewhat special.
>>>>>>>>>
>>>>>>>>> It seems NeoCSV cannot handle the situation where the very last field
>>>>>>>>> is just empty AND if there is no trailing CRLF at the end of the file.
>>>>>>>>> Somethinng like this:
>>>>>>>>>
>>>>>>>>> SecondLastColumnValue;;<EOF>
>>>>>>>>>
>>>>>>>>> In that case, readField fails because it tries to do a readQuotedField
>>>>>>>>> or readUnquotedField, both of which try to read beyond EOF.
>>>>>>>>>
>>>>>>>>> So I changed readField to this:
>>>>>>>>>
>>>>>>>>> readField
>>>>>>>>>
>>>>>>>>>      ^self atEnd "In case the very last field of a file is empty, like
>>>>>>>>> '45;56;;'"
>>>>>>>>>          ifTrue: ['']
>>>>>>>>>          ifFalse: [self peekQuote ifTrue: [self readQuotedField]
>>>>>>>>> ifFalse: [self readUnquotedField]]
>>>>>>>>>
>>>>>>>>> and all seems fine so far.
>>>>>>>>>
>>>>>>>>> Side note: My original file has a trailing CrLf but if I upload it via
>>>>>>>>> a browser to a Seaside Server, the Browser cuts off the trailing CrLf
>>>>>>>>> (I can see this in the Browser's Network debugging tools - both in IE
>>>>>>>>> and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>>>>>>
>>>>>>>>> Joachim
>>>>>>>>>
>>>>>>>>>
>>>>>>>> --
>>>>>>>> -----------------------------------------------------------------------
>>>>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>>>>> D-71640 Ludwigsburg
>>>>>>>> http://joachimtuchel.wordpress.com
>>>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>>>
>>>>>>>>
>>>>>> --
>>>>>> -----------------------------------------------------------------------
>>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>
>>>>>>
>>>> --
>>>> -----------------------------------------------------------------------
>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>
>>>>
>>>>
>>
>> --
>> -----------------------------------------------------------------------
>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>> Fliederweg 1                         http://www.objektfabrik.de
>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>
>>
>
>


--
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          mailto:[hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Ben Coman
On Thu, Jul 2, 2015 at 3:48 PM, [hidden email]
<[hidden email]> wrote:
> Sven,
>
> you are, of course, right!
> I should have checked in Pharo first. But since my version is old anyways, I
> would have to try with at least two versions in order to make sure if the
> bug is fixed or not. As long as keeping ports current is a lot of work due
> to (increasing) differences between Pharo and most other dialects, this is
> hard work ;-) I should invest in it anyways, however, since I am using your
> code from Pharo and you didn't ask me to use it...

This reminds me of Joel Spolsky's article on "barriers to entry." He
makes an interesting point with his "PayMyBills" example that a hidden
barrier to entry is not how hard it is to switch in, but how hard it
is to switch out [or port].

cheers -ben

Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Hannes Hirzel
In reply to this post by jtuchel
Hello Joachim

In May I did a port of NeoCSV to Cuis Smalltalk.

I followed a process like this

1. download .mcz in its "original" version and unpack it to have the
st file only. Keep
    the Core and the Tests st file in the target repository for reference.
2. Make a copy of the two files
3. Modify the st files with search/replace operations for obvious and
safe changes
4. Import and further modify on target platform
5. Run tests and resolve remaining issues
6. Publish on target platform's public repository
7. Document the process so that it can be redone from scratch much
faster by myself or somebody else in a semi-automatic way.


Remarks:
1) https://github.com/hhzl/Cuis-NeoCSV/blob/master/Neo-CSV-Core-orig.st
 is 1011 LOC
    https://github.com/hhzl/Cuis-NeoCSV/blob/master/Neo-CSV-Tests-orig.st
is 805 LOC

It took me two attempts to arrive at the goal

    https://github.com/hhzl/Cuis-NeoCSV

In the first attempt nearly all tests failed. But this was because of
some minor differences which caused most of the problems. After having
them fixed the number of tests which passed raised quickly.

In the end it needed a combination of systematic changes and  some
'compatibility layer' on Cuis which is necessary for the port. The
package used for that contains much more than needed, I did not take
the time yet to figure out which methods actually are needed.  I think
only very few methods are needed.

I think the Neo-CSV package is very useful to do workspace based
applications for an explorative analysis of complex data. CSV data is
a representation of one of the most common file formats (Excel data).
And we need to be able to do this in various Smalltalk distributions.

The Neo-CSV package is well written and not all that complex. Porting
can be done in some hours. Sven was very helpful and added another
test for me to illustrate a particular use, actually my main use case.

Summary:

a) So the porting experience was good.
b) Porting the tests is a necessity to prove that the port has been done.
c) I think you can do the same thing quickly for VA Smalltalk if you
actually take the plunge.


Way forward:

It might be worthwile to consider to maintain a github repository with
steps 1,2 and 3 from above. FileOut format for ease of use. Using
another repo will distinguish the effort to be a modified _copy_ of
http://smalltalkhub.com/#!/~SvenVanCaekenberghe/Neo/

Actually Sven already has taken precautions to make the code
porting-friendly and he might want to further facilitate this.

I am considering a port to Amber Smalltalk, but interestingly though
Amber is said to be Pharo compatible there are some problems which I
have to resolve first.

HTH

Hannes

On 7/2/15, [hidden email] <[hidden email]> wrote:

> Sven,
>
> you are, of course, right!
> I should have checked in Pharo first. But since my version is old
> anyways, I would have to try with at least two versions in order to make
> sure if the bug is fixed or not. As long as keeping ports current is a
> lot of work due to (increasing) differences between Phar and most other
> dialects, this is hard work ;-) I should invest in it anyways, however,
> since I am using your code from Pharo and you didn't ask me to use it...
>
> So, to address this Array construction issue (as an example of others).
> This cannot be handled with Grease or the like, since what is in my way
> is the Smalltalk compiler (I am trying to concince Instantiations to
> support {} for literal Array construction, but until I'm successful...).
>
> Given somebody wrote RewriteRules to replace {} with "(Array new: x)
> addAll:", this should probably be used on non-Pharo-platforms as part of
> a port, so that the "original" code is still the Metacello version it
> was on Pharo.
>
> My plan for a port:
> 1. download .mcz in its "original" version and don't modify it
> 2. import and modify on target platform
> 3. test and publish on target platform's public repository
>
> What do you think would be a good process to porting when it involves
> such steps?
>
> The problem with Step 2 is: I am not sure if RewriteRules can work in
> code that's not compiled. (Well, I am mostly sure this won't work, just
> asking for confirmation) You cannot compile methods with Arrays
> constructed with curly braces on VAST.
>
> So another possible process would be:
> 1. use Pharo to modify downloaded code in order to make it importable on
> target platform
> 2. publish this code somewhere (not official Pharo repositories, maybe
> StHub)
> 3. Download the modified version and import into target dialect
> 4. Make final adjustments to make code and tests run
> 5. Publish to target platform's public repository
>
> Any thoughts or ideas? People interested in joining efforts?
>
> And, now that we're on a new topic anyways, misusing this thread (mea
> culpa):
>
> I still wonder how we could make it easier to contribute back. Every
> time I find a bug in some code from Pharo, there are several things to
> consider:
> 1. Is there a newer version on Pharo that doesn't have this bug?
>      a) yes: port newer version
>      b) no: I'm in trouble - see below
>
> in case of 1b), there are two options now:
> 1. I fire up Pharo and fix the bug in the original code, then commit the
> changes to the official repository/ies, and then port the fixed version
> 2. I contact the ML or the author/maintainer of the code and give them a
> bug report and hopefully a fix, which may or may not be compatible with
> Pharo. Then I can either port the fixed version or just keep my fixed
> version (if the fix is the same, this is not a problem from the code
> perspective, but maybe from a version numbering standpoint).
>
> I am interested in ideas here and would like to hear from people
> interested in discussing these issues. Maybe ESUG would be a good
> opportunity to start discussing. Maybe we can start a new initiative for
> this problem area...
>
> Joachim
>
>
>
> Am 01.07.15 um 21:27 schrieb Sven Van Caekenberghe:
>> But before you ask for help on the Pharo ML you should run the NeoCSV code
>> on Pharo to validate that your assumption hold there. If not, it is a
>> porting error/problem.
>>
>> And unit tests are *VERY IMPORTANT* to maintain our sanity across versions
>> and/or platforms.
>>
>> Again, what you report as a potential problem isn't one, I think. Please
>> test on Pharo itself.
>>
>>> On 01 Jul 2015, at 21:07, [hidden email] wrote:
>>>
>>> I gave up on porting tests. VAST doesn't support creating Arrays with
>>> curly braces, and they are used all over the place in Pharo code. I wuld
>>> use them if I had them, so this is not an accuse or anything. It's just a
>>> huge load of work to keep tests up to date when porting newer versions,
>>> so I just ignore tests when porting. A sad truth.
>>>
>>> Joachim
>>>
>>> Am 01.07.15 um 16:14 schrieb H. Hirzel:
>>>> Joachim,
>>>>
>>>> which results do you get on VA Smalltalk for the tests?
>>>>
>>>> Do they all pass?
>>>>
>>>> --Hannes
>>>>
>>>> On 7/1/15, [hidden email] <[hidden email]> wrote:
>>>>> I am not complaining. It just makes porting harder.
>>>>>
>>>>> What made the thing especially strange was that I only had that
>>>>> problem
>>>>> with uploaded files because the Browser removes the trailing CrLf (or
>>>>> better, doesn't add another one in the multipart form data). So
>>>>> reading
>>>>> from a file all works like a charm, but not when you process the same,
>>>>> uploaded file on the server side...
>>>>>
>>>>> Joachim
>>>>>
>>>>> Am 01.07.15 um 15:12 schrieb Sven Van Caekenberghe:
>>>>>> Yes, on Pharo, #next (and #peek or #peekFor:) all return nil when
>>>>>> #atEnd.
>>>>>>
>>>>>> It is the way it is (I am personally for stricter semantics), but
>>>>>> that
>>>>>> fact is certainly used in code all allround the place.
>>>>>>
>>>>>>> On 01 Jul 2015, at 15:06, [hidden email] wrote:
>>>>>>>
>>>>>>> Hi Sven,
>>>>>>>
>>>>>>> I didn't test on Pharo. But I remember seeing differences in the way
>>>>>>> Pharo and VAST react to reads beyond the end of a Stream.
>>>>>>> Not sure, but this could have been in NeoCSV context two or three
>>>>>>> years
>>>>>>> ago.
>>>>>>>
>>>>>>> So it is very likely I was bitten by platform differences.
>>>>>>>
>>>>>>> Thanks for answering.
>>>>>>>
>>>>>>> Joachim
>>>>>>>
>>>>>>> Am 01.07.15 um 14:05 schrieb Sven Van Caekenberghe:
>>>>>>>> Hi Joachim,
>>>>>>>>
>>>>>>>> First, thanks for the feedback.
>>>>>>>>
>>>>>>>> Second, since you are on a different platform, that might be a
>>>>>>>> factor.
>>>>>>>>
>>>>>>>> Did you test your problem on Pharo itself ?
>>>>>>>>
>>>>>>>> Because there are already unit tests specifically for the case you
>>>>>>>> describe:
>>>>>>>>
>>>>>>>> #testEmptyLastFieldUnquoted
>>>>>>>> #testEmptyLastFieldQuoted
>>>>>>>>
>>>>>>>> These obviously pass for Pharo, do they pass for you ?
>>>>>>>>
>>>>>>>> Maybe your problem case is slightly different though ?
>>>>>>>>
>>>>>>>> Sven
>>>>>>>>
>>>>>>>>> On 01 Jul 2015, at 13:40, [hidden email] wrote:
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I've tried porting SvenVanCaekenberghe.20 and see the same problems
>>>>>>>>> in
>>>>>>>>> this version. IN addition to the fix already mentioned, I also had
>>>>>>>>> to
>>>>>>>>> change readSeparator:
>>>>>>>>>
>>>>>>>>> readSeparator
>>>>>>>>>
>>>>>>>>>      ^self atEnd ifFalse: [self peekFor: separator]
>>>>>>>>>
>>>>>>>>> As far as I can tell by now, this fixes the problem at hand. Any
>>>>>>>>> ideas
>>>>>>>>> if this is a safe fix?
>>>>>>>>>
>>>>>>>>> Joachim
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Am 01.07.15 um 12:35 schrieb [hidden email]:
>>>>>>>>>> Hi there,
>>>>>>>>>>
>>>>>>>>>> I am on VA Smalltalk and therefor using an older version of
>>>>>>>>>> NeoCSV
>>>>>>>>>> (SvenVanCaekenberghe.14). I found a bug in this old version that
>>>>>>>>>> is
>>>>>>>>>> somewhat special.
>>>>>>>>>>
>>>>>>>>>> It seems NeoCSV cannot handle the situation where the very last
>>>>>>>>>> field
>>>>>>>>>> is just empty AND if there is no trailing CRLF at the end of the
>>>>>>>>>> file.
>>>>>>>>>> Somethinng like this:
>>>>>>>>>>
>>>>>>>>>> SecondLastColumnValue;;<EOF>
>>>>>>>>>>
>>>>>>>>>> In that case, readField fails because it tries to do a
>>>>>>>>>> readQuotedField
>>>>>>>>>> or readUnquotedField, both of which try to read beyond EOF.
>>>>>>>>>>
>>>>>>>>>> So I changed readField to this:
>>>>>>>>>>
>>>>>>>>>> readField
>>>>>>>>>>
>>>>>>>>>>      ^self atEnd "In case the very last field of a file is empty,
>>>>>>>>>> like
>>>>>>>>>> '45;56;;'"
>>>>>>>>>>          ifTrue: ['']
>>>>>>>>>>          ifFalse: [self peekQuote ifTrue: [self readQuotedField]
>>>>>>>>>> ifFalse: [self readUnquotedField]]
>>>>>>>>>>
>>>>>>>>>> and all seems fine so far.
>>>>>>>>>>
>>>>>>>>>> Side note: My original file has a trailing CrLf but if I upload it
>>>>>>>>>> via
>>>>>>>>>> a browser to a Seaside Server, the Browser cuts off the trailing
>>>>>>>>>> CrLf
>>>>>>>>>> (I can see this in the Browser's Network debugging tools - both in
>>>>>>>>>> IE
>>>>>>>>>> and FF) - so it seems NeoCSV has to be ready for this situation.
>>>>>>>>>>
>>>>>>>>>> Joachim
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> -----------------------------------------------------------------------
>>>>>>>>> Objektfabrik Joachim Tuchel
>>>>>>>>> mailto:[hidden email]
>>>>>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>>>>>> D-71640 Ludwigsburg
>>>>>>>>> http://joachimtuchel.wordpress.com
>>>>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>>>>
>>>>>>>>>
>>>>>>> --
>>>>>>> -----------------------------------------------------------------------
>>>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>>>> D-71640 Ludwigsburg
>>>>>>> http://joachimtuchel.wordpress.com
>>>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>>>
>>>>>>>
>>>>> --
>>>>> -----------------------------------------------------------------------
>>>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>>>> Fliederweg 1                         http://www.objektfabrik.de
>>>>> D-71640 Ludwigsburg
>>>>> http://joachimtuchel.wordpress.com
>>>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>>>
>>>>>
>>>>>
>>>
>>> --
>>> -----------------------------------------------------------------------
>>> Objektfabrik Joachim Tuchel          mailto:[hidden email]
>>> Fliederweg 1                         http://www.objektfabrik.de
>>> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
>>> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>>>
>>>
>>
>>
>
>
> --
> -----------------------------------------------------------------------
> Objektfabrik Joachim Tuchel          mailto:[hidden email]
> Fliederweg 1                         http://www.objektfabrik.de
> D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
> Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Sean P. DeNigris
Administrator
Hannes Hirzel wrote
though Amber is said to be Pharo compatible there are some problems which I
have to resolve first.
Pharo is the reference implementation, but is a subset. I guess technically that's "compatible", but "compatible" is vague enough to be easily misleading...
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

stepharo
In reply to this post by jtuchel


Le 1/7/15 12:35, [hidden email] a écrit :
> Hi there,
>
> I am on VA Smalltalk
For your next successful project you should jump in Ph.... :)


> and therefor using an older version of NeoCSV
> (SvenVanCaekenberghe.14). I found a bug in this old version that is
> somewhat special.
>
> It seems NeoCSV cannot handle the situation where the very last field
> is just empty AND if there is no trailing CRLF at the end of the file.
> Somethinng like this:
>
> SecondLastColumnValue;;<EOF>
>
> In that case, readField fails because it tries to do a readQuotedField
> or readUnquotedField, both of which try to read beyond EOF.
>
> So I changed readField to this:
>
> readField
>
>     ^self atEnd "In case the very last field of a file is empty, like
> '45;56;;'"
>         ifTrue: ['']
>         ifFalse: [self peekQuote ifTrue: [self readQuotedField]
> ifFalse: [self readUnquotedField]]
>
> and all seems fine so far.
>
> Side note: My original file has a trailing CrLf but if I upload it via
> a browser to a Seaside Server, the Browser cuts off the trailing CrLf
> (I can see this in the Browser's Network debugging tools - both in IE
> and FF) - so it seems NeoCSV has to be ready for this situation.
>
> Joachim
>
>


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

stepharo
In reply to this post by Thierry Goubier


Le 1/7/15 21:10, Thierry Goubier a écrit :

> Le 01/07/2015 21:07, [hidden email] a écrit :
>> I gave up on porting tests. VAST doesn't support creating Arrays with
>> curly braces, and they are used all over the place in Pharo code. I wuld
>> use them if I had them, so this is not an accuse or anything. It's just
>> a huge load of work to keep tests up to date when porting newer
>> versions, so I just ignore tests when porting. A sad truth.
>
> Maybe we should write a refactoring to change those curly braces into
> an equivalent Array based syntax?
>
> I'll have some significant porting from Pharo to do one of these days,
> so maybe I'll setup something for that.
Why from Pharo? What is the problem?


Stef

Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

stepharo
In reply to this post by Thierry Goubier
Hi thierry

mark will arriv at Lille saturday for a month. He is working with
Camille on a new tree pattern matching algorithm
because we lost energy on RB matcher.

Stef

Le 1/7/15 21:29, Thierry Goubier a écrit :

> Le 01/07/2015 21:16, [hidden email] a écrit :
>> You mean RewriteRules? I was planning to do that, for this exact reason.
>> But you know how things go when there is a long list of priorities.
>> I know it saves a lot of time and makes porting so much more safe, but
>> it needs an up-front investment in time...
>
> Yes, rewrite rules. Mark Rizun GUI attempts are a very interesting
> direction to handle that...
>
> Oh, by the way, Mark, if you're listening... I did what you were
> trying to do by changing AST nodes positions in Pharo, but on a SmaCC
> based AST for another language. I have to admit that you were right to
> try to do it that way, but that the Pharo/RB AST infrastructure wasn't
> suitable whereas SmaCC infrastructure is.
>
> Thierry
>
>


Reply | Threaded
Open this post in threaded view
|

Re: NeoCSVReader and an empty field at the very end of a file

Thierry Goubier
Hi Stef,

Le 02/07/2015 23:18, stepharo a écrit :
> Hi thierry
>
> mark will arriv at Lille saturday for a month. He is working with
> Camille on a new tree pattern matching algorithm

A generic algorithm or one tied to the RB AST?

> because we lost energy on RB matcher.

What was your analysis?

Thanks,

Thierry

>
> Stef
>
> Le 1/7/15 21:29, Thierry Goubier a écrit :
>> Le 01/07/2015 21:16, [hidden email] a écrit :
>>> You mean RewriteRules? I was planning to do that, for this exact reason.
>>> But you know how things go when there is a long list of priorities.
>>> I know it saves a lot of time and makes porting so much more safe, but
>>> it needs an up-front investment in time...
>>
>> Yes, rewrite rules. Mark Rizun GUI attempts are a very interesting
>> direction to handle that...
>>
>> Oh, by the way, Mark, if you're listening... I did what you were
>> trying to do by changing AST nodes positions in Pharo, but on a SmaCC
>> based AST for another language. I have to admit that you were right to
>> try to do it that way, but that the Pharo/RB AST infrastructure wasn't
>> suitable whereas SmaCC infrastructure is.
>>
>> Thierry
>>
>>
>
>
>


12