eliben / pss

pss is a power-tool for searching inside source code files.
Other
327 stars 46 forks source link

Coloring doesn't work in Windows with certain command-line arguments #5

Closed benhoyt closed 11 years ago

benhoyt commented 11 years ago

For example, coloring works as expected with no command-line arguments:

pss foo

But with certain command line arguments pss doesn't do any coloring:

pss --context=1 foo

However, some command line arguments do still color correctly, like pss -a foo.

I'm using pss on Python 2.7.5, Windows 7 64-bit.

benhoyt commented 11 years ago

Oddly, pss -C1 foo colors correctly, but pss -C 1 foo with the space doesn't color.

eliben commented 11 years ago

Hmm, that's indeed odd. It's non-trivial for me to come up with a Windows box for reproducing this, unfortunately.

benhoyt commented 11 years ago

Okay, I'll see if I can narrow this down.

benhoyt commented 11 years ago

Hmmm, this is actually a bug in colorama, which doesn't work properly in 64-bit Windows. The reason is that colorama's win32.py doesn't use ctypes correctly, specifically it doesn't specify the argtypes for Win32 functions. HANDLE on 64-bit Windows is a 64-bit value, so you need to specify these, otherwise ctypes assumes HANDLE is 32 bit. This is actually a fairly common (and easy-to-overlook) problem with ctypes on Windows. For example, here's another time I've run into this.

This is also why it works sometimes and not others -- it depends what's in memory on the stack. (I'm somewhat surprised I didn't see exceptions or core dumps, though.)

I've patched it locally by adding argtypes/restype for all the Win32 functions and the fix works.

I'll open a bug upstream on the colorama project site.

benhoyt commented 11 years ago

See https://code.google.com/p/colorama/issues/detail?id=43

eliben commented 11 years ago

Great, thanks for researching. You can send a pull request for pss and we'll fix it locally for now.

benhoyt commented 11 years ago

Good idea. Pull request created: https://github.com/eliben/pss/pull/7

eliben commented 11 years ago

Merged, thanks.