On Monday, April 11, 2011, Chris Clark <chris.cl...@ingres.com> wrote:
> Christian Hammond wrote:
>> I think we need a better way to factor this all out, though. Otherwise
>> you'll have to maintain a custom patch for every upgrade.
>> Can you tell me where you placed the first bit of code (encoding
>> We probably can reuse the repository's Encoding field and insert the
>> lines into perforce.py to respect it, in much the same way that you
>> did. So instead of unicode(something, 'cp949'), it'd be
>> unicode(something, repository.encoding). Right now, that field only
>> appears to affect the patches.
>> And in your case, what does p4.charset show?
> This is a good example of why we need support for named encoding in
> postreview, doing a look up is certainly the right thing to do; by
> checking the source code control system (where possible) and the
> ReviewBoard repository encoding. However we also need an override/force
> In the first instance a command line parameter would be useful.
> RE where the decode() call (rather than unicode) takes places, I'd
> suggest around line 3877. Either before or after:
> if options.output_diff_only:
> # The comma here isn't a typo, but rather suppresses the extra newline
> print diff,
> The print would probably need a repr() around it if done before.
> Line 3854 also needs a change to open options.diff_filename in binary
> ('rb') mode.
> Example code:
> diff_encoding = 'utf-8'
> diff_encoding = 'cp949' # ... from command line, config file, lookup in
> RB repo, etc.
> diff = diff.decode(diff_encoding)
I'm fine with this, though it sounded like a large part of the
problems were dealing with purely server-side data, such as the change
description (which post-review doesn't provide on Perforce) and file
fetching. In both cases, post-review wouldn't necessarily be involved
in the process.
If more data took the repository.encoding flag into affect, we would
be largely there. The problem then is having a large repository shared
with people from around the world, all using different encodings. This
is actually a more general problem (customization per-path) that I'm
looking to solve in hopefully 1.6.
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com
Want to help the Review Board project? Donate today at
Happy user? Let us know at http://www.reviewboard.org/users/
To unsubscribe from this group, send email to
For more options, visit this group at