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

Make ByteBuffer.description useful and pretty #2863

Open
weissi opened this issue Sep 3, 2024 · 7 comments
Open

Make ByteBuffer.description useful and pretty #2863

weissi opened this issue Sep 3, 2024 · 7 comments
Labels
good first issue Good for newcomers size/S Small task. (A couple of hours of work.)

Comments

@weissi
Copy link
Member

weissi commented Sep 3, 2024

Right now, ByteBuffer's description is ugly and not very useful (doesn't print the bytes). With #2856 being merged, I'm proposing to make ByteBuffer's description the following:

[ hex bytes, maximum 64 or something ](length bytes)

so hello world would be

[68656c6c6f20776f726c64](11 bytes)

and 256 0 bytes would be

[00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000...00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000](256 bytes)

the empty string would be [](0 bytes)

@FranzBusch FranzBusch added good first issue Good for newcomers size/S Small task. (A couple of hours of work.) labels Sep 3, 2024
@FranzBusch
Copy link
Member

Onboard with this idea. The current description is not really helpful in most cases and I always do a hexDump manually while debugging. We should also change the debugDescription of ByteBuffer while we are it.

@weissi
Copy link
Member Author

weissi commented Sep 3, 2024

Onboard with this idea. The current description is not really helpful in most cases and I always do a hexDump manually while debugging. We should also change the debugDescription of ByteBuffer while we are it.

Normally I'd say that we shouldn't change debugDescription (because that actually has the bytes and when debugging a ByteBuffer the other stuff is interesting too). However, if you do Array<ByteBuffer>.description then that will invoke the element's debugDescription!?... So yeah, I guess we should just make .description == .debugDescription

@FranzBusch
Copy link
Member

Yes CustromStringConvertible and CustomDebugStringConvertible are a bit messy. The latter is used by collection types to print their contents. So you should always implement both. At least that's my latest understanding @dnadoba dove into this a bit a while ago.

@supersonicbyte
Copy link
Contributor

I can pick this one up also 👋

@supersonicbyte
Copy link
Contributor

@weissi since @FranzBusch mentioned here that () are preferred, should the output of ByteBuffer's description be wrapped with () or should we stick with []? IMO, [] feels more natural for ByteBuffer.

@dnadoba
Copy link
Member

dnadoba commented Sep 3, 2024

Normally I'd say that we shouldn't change debugDescription (because that actually has the bytes and when debugging a ByteBuffer the other stuff is interesting too). However, if you do Array.description then that will invoke the element's debugDescription!?... So yeah, I guess we should just make .description == .debugDescription

Good! Then please approve: #2495

supersonicbyte added a commit to supersonicbyte/swift-nio that referenced this issue Sep 3, 2024
Motivation:

Resolving the following issue: apple#2863

Modifications:

Rewrote `description` and `debugDescription` on `ByteBuffer` to make it more
useful.

Result:

A more userful `description` and `debugDescription`.

After your change, what will change.
@supersonicbyte
Copy link
Contributor

Here it is

FranzBusch added a commit that referenced this issue Sep 4, 2024
### Motivation:

Resolving the following issue:
#2863

### Modifications:

Rewrote `description` and `debugDescription` on `ByteBuffer` to make it
more useful. Also fixed a bug in the in `hexDumpCompact`.

### Result:

A more useful `description` and `debugDescription`.

---------

Co-authored-by: Franz Busch <f.busch@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers size/S Small task. (A couple of hours of work.)
Projects
None yet
Development

No branches or pull requests

4 participants