Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
theseion opened this issue Oct 29, 2023 · 9 comments · Fixed by #1385
Closed

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

theseion opened this issue Oct 29, 2023 · 9 comments · Fixed by #1385
Labels

Comments

@theseion
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

@theseion hm... I think we should do that yes. I had the same issue in GemStone (see 95d0f2e) 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
Copy link
Member

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

@timrowledge
Copy link

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
Copy link

timrowledge commented Oct 30, 2023

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
Copy link
Member Author

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
Copy link

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

@theseion
Copy link
Member Author

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

@timrowledge
Copy link

timrowledge commented Oct 31, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants