Author gward
Recipients barry, durin42, gward, ncoghlan, pitrou, r.david.murray, terry.reedy
Date 2013-03-24.21:04:20
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1364159060.56.0.284752875405.issue17445@psf.upfronthosting.co.za>
In-reply-to
Content
> I recommend the following: replace the simple test in the attached bytes_diff.py with 
> Greg's unittest-based tests and adjust the __name__ == '__main__' incantation 
> accordingly.

Latest patch, following Terry's suggestion: http://hg.gerg.ca/cpython/rev/6718d54cf9eb

Pro:
- does not touch unified_diff() or context_diff(), just adds a new function
- no question that this is a new feature, so no debate about what branch to commit on
- forces caller to know exactly what they are dealing with, strings or bytes

Con:
- not a bug fix, so 3.3 users won't get it ... but they can just copy the 
  implementation of diff_bytes() (or, as Terry suggests, it could live on PyPI)
- has explicit isinstance() checks (for a good reason: see the comments)
- also has more Pythonic "raise TypeError if s.decode raised AttributeError",
  which is friendlier to duck typing but inconsistent with the isinstance() checks
  used elsewhere in the patch

Overall I'm fairly happy with this. Not too thrilled with the explicit type checks; suggestions welcome.

Regarding Terry's suggestion of putting diff_bytes() on PyPI: meh. If the only project that needs this is Mercurial, that would be pointless. Mercurial doesn't have PyPI dependencies, and that is unlikely to change. This might be one of those rare cases where copying the code is easier than depending on it.
History
Date User Action Args
2013-03-24 21:04:20gwardsetrecipients: + gward, barry, terry.reedy, ncoghlan, pitrou, durin42, r.david.murray
2013-03-24 21:04:20gwardsetmessageid: <1364159060.56.0.284752875405.issue17445@psf.upfronthosting.co.za>
2013-03-24 21:04:20gwardlinkissue17445 messages
2013-03-24 21:04:20gwardcreate