Magritte-Model changes .455, .456

I'm not sure the replacement of

visitNumberDescription: aDescription
     | contents |
     contents := self contents.
     contents isEmpty
         ifTrue: [ MAReadError signal ].
     (contents occurrencesOf: $-) > 1
         ifTrue: [ MAReadError signal ].
     (contents indexOf: $-) > 1
         ifTrue: [ MAReadError signal ].
     (contents occurrencesOf: $.) > 1
         ifTrue: [ MAReadError signal ].
     (contents allSatisfy: [ :each | '+-0123456789.eE' includes: each ])
         ifFalse: [ MAReadError signal ].
     super visitNumberDescription: aDescription

by
visitNumberDescription: aDescription
     | isContentsValid |
     isContentsValid := NumberParser isNumber: self contents.
     isContentsValid ifFalse: [ MAReadError signal ].
     super visitNumberDescription: aDescription

works cross-platform.

There are two 455s, and 456 misses changes from both.
Merged in 457

Stephan


_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Hmm, I missed some more changes.

I'm not sure I'd want the MABlockDescription in Magritte-Model.
Perhaps a separate package Magritte-UnsafeExtensions?

"protocol: visiting-description"
MAStringReader>>visitBlockDescription: aDescription
     | block |
     block := Compiler evaluate: self contents.
     self object: block

458 provides a better merge, without th eblock description.
Still not compatible with Pharo20 build (NumberParser)

Stephan
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Stephan Eggermont wrote

I'm not sure I'd want the MABlockDescription in Magritte-Model.
Perhaps a separate package Magritte-UnsafeExtensions?

Thanks for the review! Perhaps Magritte-ModelUnsafe? But it seems to me that if you're allowing users to type code blocks in a form, you have to know that there are security concerns e.g. [ FileLocator home... "delete all files" ]

Stephan Eggermont wrote

I'm not sure the replacement... works cross-platform

Thanks for this review too :) I'm never quite sure which platforms we expect to be supported, so I refactor and brace for complaints! When I was making the change, I also considered a Magritte extension on Number or String (which could more easily be made cross-platform), but couldn't think of a good message name - #representsNumber?

ModelUnsafe sounds good to me. That makes the decision to
live dangerously explicit...

Stephan

On 17-04-15 13:49, Sean P. DeNigris wrote:
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Stephan Eggermont wrote

I'm not sure the replacement of

visitNumberDescription: aDescription
...
     (contents allSatisfy: [ :each | '+-0123456789.eE' includes: each ])

Another reason to rely on the platform's number parsing and not reinvent our own logic. Pharo has ScaledDecimals, which use $s in the literal form and printString, which is not reflected in our tests. To further complicate, the error generated was silently swallowed as reported in another thread a moment ago.