search

hynek / characteristic

Please use attrs instead!
https://attrs.readthedocs.io/
MIT License
84 stars 19 forks source link

Truncate long attribute values by default by using reprlib.repr. #21

Closed Julian closed 9 years ago

coveralls commented 9 years ago

Coverage Status

Coverage remained the same when pulling e011eed7b5f66714eff53c7b6d0de709b8bcd67e on Julian:master into a8339ee0f9e1cd017afccdef770df593afc975a6 on hynek:master.

hynek commented 9 years ago

Thanks!

I feel like making that default is rather backward incompatible (maybe someone uses reprs in their logs?) other than that, that option should also be exposed in @attributes, no?

Julian commented 9 years ago

Yeah I mean, it is backwards incompatible, but personally I consider reprs that trigger this without truncation to basically be buggy, they're completely unreadable which is their raison d'être. If it's just a question of what to call it in the release notes I don't care :D, but I think this is the better default.

For exposing in attributes: I think the whole point of composable things is that they're composable :P, so someone who wants a different thing for this will pass apply_with_repr=False and just use with_repr. But again, no real preference if you think this is going to be trafficked enough to warrant being passed through all the way from the top.

Julian commented 9 years ago

Oh sorry, I missed that you said "maybe someone uses reprs in their logs" -- yeah, I guess that'd be a case where someone would expect this. I still think this is the right default, but given that it wasn't already, are you thinking this would need deprecation (not sure how you could do that for this sanely) or that the default should stay as is?