Squeak 3.9: using SqNumberParser in Number>>readFrom:

> Hi,
>
> I have found the discussion from last year:
>
>   Fun with Number readFrom: What should we do ?
>   http://lists.squeakfoundation.org/pipermail/squeak-dev/2006-April/103137.html
>
> Basically, the problem was that Number>>readFrom: will produce 0 where
> it should raise an error (e.g. when the input is letters only). There
> is a fix at
>
>   http://bugs.squeak.org/view.php?id=3512
>
> i.e. a new class SqNumberParser does the right thing. The bug was
> closed, but the problem still remains: while SqNumberParser is in the
> Squeak 3.9 image,  Number>>readFrom is still the old method.
>
> Do you have any pointers to the next step, i.e. changing the code in
> classes Number, Integer (and wherever else)?
>
> I have encountered this problem when using Magritte: when I use a
> MANumberDescription for a field, there still is no check if the user
> enters a number. Validation fails because the from field value is
> interpreted using Number>>readFrom, so it produces 0 for inputs like
> "xyz".

> > > > But with Magritte, I think you can replace the way it translates the
> > > > string to a number. What I would start with if I were you: implement
> > > > #visitNumberDescription: in MAStringReader. Look at
> > > > MAStringReader>>visitElementDescription: to see the current default
> > > > implementation.
> > >
> > > Indeed, please post a fix to Magritte repository.
> >
> > But then Magritte won't be compatible with 3.8 version anymore.
>
> Breaking compatibility with 3.8 would be a pity. I think it would be
> easy to build something that works on both versions.
>
> Lukas
>
> --
> Lukas Renggli
> http://www.lukas-renggli.ch
>
>

> > I have just uploaded the fix as Magritte-Model-MatthiasBerth.250 to
> > http://mc.lukas-renggli.ch/magritte.
> >
> > I started from an older version of Magritte-Model (lr.249), and this
> > is the first time I used Monticello to write something to a
> > repository. So could someone check and maybe merge it?
>
> The package you published is empty.
>
> > I did not know where to put a unit test. In the description tests it
> > all looks so generic, and I could not find a test for MAStringReader.
>
> The tests are in the class MAElementDescription in the category
> #testing-strings. They are indeed very generic, but the tests can be
> configured in subclasses with different parameters.
>
> > How about packaging such that it is clear that one needs
> > SqNumberParser? I don't know enough about monticello, but it should be
> > possible. Is that what you mean?
>
> I would prefer a change that does not introduce external dependencies.
> One that works in 3.8 and 3.9 out of the box.
>
> I published Magritte-Model-lr.250 that should work on all Squeak
> versions. I also added a new test Magritte-Tets-lr.107 that detects
> this issue. Silently ignoring invalid input is now also fixed for
> Colors and Time.
>
> Cheers,
> Lukas
>
> --
> Lukas Renggli
> http://www.lukas-renggli.ch
>
>

> Thanks a lot. I don't know what went wrong with my saving to the
> repository, sorry.
>
> It looks like visitNumberDescription: will raise an error on negative numbers:
>
>   self contents allSatisfy: [ :each | '0123456789.' includes: each ]
>
> maybe something like this? :
>
>   (self contents allButFirst allSatisfy: [ :each | '0123456789.'
> includes: each ] )
>      and:['-0123456789.' includes self contents first]

> > Thanks a lot. I don't know what went wrong with my saving to the
> > repository, sorry.
> >
> > It looks like visitNumberDescription: will raise an error on negative numbers:
> >
> >   self contents allSatisfy: [ :each | '0123456789.' includes: each ]
> >
> > maybe something like this? :
> >
> >   (self contents allButFirst allSatisfy: [ :each | '0123456789.'
> > includes: each ] )
> >      and:['-0123456789.' includes self contents first]
>
> Indeed, good catch. I will fix this as soon as possible, unless you
> commit your change ;-)
>
> Cheers,
> Lukas
>
> --
> Lukas Renggli
> http://www.lukas-renggli.ch
>
>

> > Thanks a lot. I don't know what went wrong with my saving to the
> > repository, sorry.
> >
> > It looks like visitNumberDescription: will raise an error on negative numbers:
> >
> >   self contents allSatisfy: [ :each | '0123456789.' includes: each ]
> >
> > maybe something like this? :
> >
> >   (self contents allButFirst allSatisfy: [ :each | '0123456789.'
> > includes: each ] )
> >      and:['-0123456789.' includes self contents first]
>
> Indeed, good catch. I will fix this as soon as possible, unless you
> commit your change ;-)
>
> Cheers,
> Lukas
>
> --
> Lukas Renggli
> http://www.lukas-renggli.ch
>
>

> Looks like empty strings are also not rejected:
>
>   MAStringReader>>read: aStream description: aDescription
>         ^ aStream atEnd
>                 ifFalse: [super read: aStream description: aDescription]
>
> will not use the super method because a stream on an empty string is
> already "atEnd". Would it make sense to remove that check and hand
> over to super anyway?
>
> Matthias
>
> PS: all those subtleties of parsing numbers... SqNumberParser was
> designed for that :-)
>
>
> On 3/25/07, Lukas Renggli <[hidden email]> wrote:
> > > Thanks a lot. I don't know what went wrong with my saving to the
> > > repository, sorry.
> > >
> > > It looks like visitNumberDescription: will raise an error on negative numbers:
> > >
> > >   self contents allSatisfy: [ :each | '0123456789.' includes: each ]
> > >
> > > maybe something like this? :
> > >
> > >   (self contents allButFirst allSatisfy: [ :each | '0123456789.'
> > > includes: each ] )
> > >      and:['-0123456789.' includes self contents first]
> >
> > Indeed, good catch. I will fix this as soon as possible, unless you
> > commit your change ;-)
> >
> > Cheers,
> > Lukas
> >
> > --
> > Lukas Renggli
> > http://www.lukas-renggli.ch
> >
> >
>

> > Thanks a lot. I don't know what went wrong with my saving to the
> > repository, sorry.
> >
> > It looks like visitNumberDescription: will raise an error on negative numbers:
> >
> >   self contents allSatisfy: [ :each | '0123456789.' includes: each ]
> >
> > maybe something like this? :
> >
> >   (self contents allButFirst allSatisfy: [ :each | '0123456789.'
> > includes: each ] )
> >      and:['-0123456789.' includes self contents first]
>
> Indeed, good catch. I will fix this as soon as possible, unless you
> commit your change ;-)
>
> Cheers,
> Lukas
>
> --
> Lukas Renggli
> http://www.lukas-renggli.ch
>
>

> > OK, I have committed to the repository. No SqNumberParser needed, so
> > it should be compatible to Squeak 3.8. Midway through it, I realized
> > that the input checking would be best handled by a regular expression.
> > I still use the normal collection protocol (occurrenceOf:, indexOf:
> > etc).
>
> Sounds fine, unfortunately the packages you posted are empty again.
>
> Btw, you can see the contents of your packages before publishing by
> clicking on 'Browse' in the Monticello browser.
>
> > Would it be OK to use regular expressions or is that another
> > dependency that people would not like to have?
>
> I would try to avoid any external dependencies that are not necessary
> at all cost.
>
> Lukas
>
> --
> Lukas Renggli
> http://www.lukas-renggli.ch
>
>

> > I can see my change inside there, in new/source.st. When I browse the
> > package in my image, I see my changes, too. Maybe it's another
> > problem?
>
> I can't see the changes and can't merge them. When I try to merge or
> load your versions it tries to remove the whole package, as it looks
> empty.
>
> You seem to save your packages as diffs (see the setting in the
> context menu of the repository), please store them as 'full versions'.
> Then it probably works.
>
> Lukas