search

davecgh / go-spew

Implements a deep pretty printer for Go data structures to aid in debugging
ISC License
6.05k stars 364 forks source link

Disable unsafe for GopherJS compiler. #51

Closed flimzy closed 8 years ago

flimzy commented 8 years ago

This simple commit makes go-spew work with the GopherJS compiler.

dmitshur commented 8 years ago

As a general observation (outside the scope of this PR), @davecgh, you may want to consider using safe build tag in place of disableunsafe. It's shorter, simpler, and mirrors unsafe nicely. It's the pattern people like Brad Fitzpatrick arrived at and used in a few high profile Go packages already.

Also, relevant to this PR, see this commit for some precedent:

https://github.com/camlistore/go4/commit/afaa0dd6bd8e33e9fb57caef777e8939a519b8ec#diff-c93dc9247ab9fb2e82713bca8e504d33R5

Notice:

// +build js appengine safe
// +build !js,!appengine,!safe

@flimzy, I've left some inline comments about fixing documentation, but otherwise this LGTM.

davecgh commented 8 years ago

Thanks for the PR. Looks good.

OK

@shurcooL: That seems very reasonable to me. As you mentioned it's shorter too. I don't really like breaking backwards compat like that, but now that the project was tagged and glide has become much more ubiquitous, that's not as big of an issue.

flimzy commented 8 years ago

@davecgh: You can always support both the safe and disableunsafe build tags for a time (or even indefinitely) if backward compatibility of this feature is a big concern.

davecgh commented 8 years ago

@flimzy. I merged in the safe tag yesterday and that is exactly what I ended up doing.