spec icon indicating copy to clipboard operation
spec copied to clipboard

Make CarbonCopy a better replacement for sys.stdout

Open frol opened this issue 9 years ago • 6 comments

sys.stdout has encoding attribute.

This PR will be useful for my next PR to Invoke.

frol avatar Sep 15 '15 20:09 frol

Thanks :) couple quick thoughts w/o actually testing anything yet:

  • Do we need to change getvalue() as well? It was the other spot that hardcoded 'utf-8' encoding stuff.
  • Will this break anything that used to rely on the implicit utf-8 encoding? Feels like the naïve case would end up being encoded as latin-1 now instead of utf-8. (This is hopefully ignorant paranoia on my part, but presumably I chose utf-8 earlier for a reason, instead of latin-1.)

bitprophet avatar Sep 15 '15 23:09 bitprophet

I have just fixed getvalue.

I guess it shouldn't break anything if cc'ing streams have encoding attribute.

frol avatar Sep 16 '15 06:09 frol

@bitprophet Is there anything to improve in this PR?

frol avatar Sep 17 '15 19:09 frol

Nope, thanks. I just have too many projects (with too many users each :D) and too little time. Not everything's gonna be quick :(

Assuming this is required for one of the encoding tests in Invoke to work correctly, I'll doubtless merge this as a dependency of the other, whenever it's gotten to.

bitprophet avatar Sep 18 '15 23:09 bitprophet

@bitprophet It used to be needed in my Invoke patch, but then I changed the logic around encoding, so it is not required at the moment. Nevertheless, it is good improvement anyway.

frol avatar Sep 19 '15 03:09 frol

Gotcha, good to know, thanks (& I agree, more encoding control is usually better :))

bitprophet avatar Sep 22 '15 20:09 bitprophet