SeasideSt / Seaside

The framework for developing sophisticated web applications in Smalltalk.
MIT License
519 stars 71 forks source link

Squeak does not have an implementation of Integer>>#asByteArray #1383

Closed theseion closed 1 year ago

theseion commented 1 year ago

WAAbstractFileLibrary>>#entityTagFor: sends #asByteArray to a LargePositiveInteger, which causes and MNU in Squeak. This causes errors when accessing jQuery files (no styling visible on pages).

I've come up with a quick fix version but it lacks VM support as in Pharo:

asByteArray

    | stream |
    stream := ByteArray new writeStream.
    ((self numberOfDigitsInBase: 2) / 8) ceiling to: 1 by: -1 do: [:digitIndex |
        stream nextPut: (self digitAt: digitIndex)].
    ^ stream contents
theseion commented 1 year ago

We could actually just copy the implementation from Pharo, which uses primitive 62 to compute the number of bytes in large integers.

I suppose I would implement #greaseAsByteArray on Integer and subclasses, as required, where the Pharo version would simply dispatch to Integer>>#asByteArray, right @jbrichau?

jbrichau commented 1 year ago

@theseion hm... I think we should do that yes. I had the same issue in GemStone (see https://github.com/SeasideSt/Seaside/commit/95d0f2e3f20b29736a5f25b403024dd4b7f8f6f7) but now I do not understand entirely because Integer>>asByteArray still does not exist in GemStone. I guess there is some other package being loaded that adds it... so there is an issue here for both Squeak and GemStone currently, which makes Grease the best place to tackle the difference.

jbrichau commented 1 year ago

Well... not the same issue but I should have the same issue actually after that commit... which is strange.

timrowledge commented 1 year ago

There may be more to this than initially meets the eye. Squeak really should have #asByteAray for integers since it is sent in Magnitude>>#putOn: which gets used in writing values to assorted streams. Some of which can be set to binary (file streams, for example) andwhich would fail with any sort of number.

timrowledge commented 1 year ago

It turns out that #asByteArray is really rather trivial for LargePositiveInteger - myLargePositiveInteger as: ByteArray will do it. Hacking that trivially into #entityTagFor: to test makes all the nice formatting come back.

theseion commented 1 year ago

Yes, it's weird that that method isn't there. I looked back as far as I could (5.2 only, unfortunately) and it's not there. There's a PR in the works that adds Grease support for all platforms (GemStone has the same issue).

timrowledge commented 1 year ago

I've added a suitable #asByteArray for squeak 6.1 trunk.

theseion commented 1 year ago

That's great, thanks. Did you use the implementation from Pharo (which used for Seaside)?

timrowledge commented 1 year ago

No idea on that - I just used the simplest possible Squeak mechanism. It's a bit crazy really since a large integer is just a byte array!

On 2023-10-30, at 11:55 PM, Max Leske @.***> wrote:

That's great, thanks. Did you use the implementation from Pharo (which used for Seaside)?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

tim

tim Rowledge; @.***; http://www.rowledge.org/tim After a number of decimal places, nobody gives a damn.