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

Print NSData's hash instead of byte description in error messages #263

Merged
merged 1 commit into from
Mar 13, 2016
Merged

Print NSData's hash instead of byte description in error messages #263

merged 1 commit into from
Mar 13, 2016

Conversation

inket
Copy link
Contributor

@inket inket commented Mar 9, 2016

closes #259

Printing the byte description of a 1MB NSData is long enough to keep printing for a while, and to freeze Xcode because of the length of the resulting message.

Print "NSData" instead as it's a decent representation of identity/equality (although not perfect).

@jeffh
Copy link
Member

jeffh commented Mar 10, 2016

Hey @inket,

Looks good to me. Although I would like to have a test to make sure this doesn't regress at a later period before merging this in.

@inket
Copy link
Contributor Author

inket commented Mar 10, 2016

Alright 👍

@briancroom
Copy link
Member

Do you think it would be useful to also include the data's byte count in the output?

@inket
Copy link
Contributor Author

inket commented Mar 10, 2016

@briancroom

Thought about it but couldn't think of an appropriate format to represent it: NSData<hash=123456789,length=38849> or NSData(hash:123456789,length:38849) or other variations? I'm open to suggestions otherwise I will just use the first one.

It is indeed useful to show the byte count as it would make it easy to tell whether an NSData is empty or not (and other things) at a glance.

@briancroom
Copy link
Member

I like the first one, myself!

@inket
Copy link
Contributor Author

inket commented Mar 11, 2016

After implementing tests, it seems that the Linux test fails because NSData's hash() causes a segmentation fault.

Using the REPL on Mac:

Welcome to Apple Swift version 3.0-dev (LLVM b361b0fc05, Clang 11493b0f62, Swift 24a0c3de75). Type :help for assistance.
  1> import Foundation
  2> "foobar".dataUsingEncoding(NSUTF8StringEncoding)?.hash
$R0: Int? = 114710658

on Linux:

Welcome to Swift version 3.0-dev (LLVM b361b0fc05, Clang 11493b0f62, Swift 24a0c3de75). Type :help for assistance.
  1> import Foundation
  2> "foobar".dataUsingEncoding(NSUTF8StringEncoding)?.hash
Execution interrupted. Enter code to recover and continue.
Enter LLDB commands to investigate (type :help for assistance.)

I fixed that by using #if os(Linux) and falling back to NSData<length=*> for now. But if that's not ideal, we can use the NSData<length=*> for all OS ? Please advise.

@briancroom
Copy link
Member

Oh shoot, that's unfortunate. There are a number of places where we have introduced conditionals for Linux behavior already, and it seems reasonable to me to do so here as well. Please do leave a comment indicating the reason for it so that it is easier to try again in a few weeks/months and see if the issue has been fixed.

It'd be great if you could log an issue in bugs.swift.org as well!

Printing the byte description of a 1MB NSData is long enough to keep printing for a while, and to freeze Xcode because of the length of the resulting message.

Print "NSData<hash=*,length=*>" instead as it's a decent representation of identity/equality.
@inket
Copy link
Contributor Author

inket commented Mar 11, 2016

Thanks! This PR is good for a last review now 😁

Will report the bug on the Swift bug tracker later.

jeffh added a commit that referenced this pull request Mar 13, 2016
Print NSData's hash instead of byte description in error messages
@jeffh jeffh merged commit 8b01403 into Quick:master Mar 13, 2016
@jeffh
Copy link
Member

jeffh commented Mar 13, 2016

LGTM, thanks @inket!

Megal pushed a commit to Megal/Nimble that referenced this pull request Jul 31, 2019
Print NSData's hash instead of byte description in error messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Result of stringifying a big NSData is too long
3 participants