httpie / cli

🥧 HTTPie CLI — modern, user-friendly command-line HTTP client for the API era. JSON support, colors, sessions, downloads, plugins & more.
https://httpie.io
BSD 3-Clause "New" or "Revised" License
33.22k stars 3.67k forks source link

env.colors may be incorrectly detected #244

Closed evanpurkhiser closed 9 years ago

evanpurkhiser commented 10 years ago

When a terminal supports 256 colors but does not have the string 256colors in it's TERM name then httpie will fallback to the 88 color formatter.

For example, I'm using termite which sets the TERM environment variable to xterm-termite. Looking at the termcap we can see that it clearly supports 256 colors.

It looks like the most portable way to do this would be to use curses (though I'm not sure if this would be considered 'heavy weight')

>>> from curses import setupterm, tigetnum
>>> setupterm()
>>> tigetnum('colors')
256
sigmavirus24 commented 10 years ago

The biggest issue with curses is that it doesn't port well to Windows. Linux and OSX (and other *nix's) sure but not Windows from my experience. This could be different on newer Pythons, but on 2.6 (and I think 2.7) it's not really "portable"

evanpurkhiser commented 10 years ago

Shouldn't be a problem since it looks like windows gets defaulted to 256 colors anyway

sigmavirus24 commented 10 years ago

Good point. I forgot about that. =D

evanpurkhiser commented 10 years ago

I couldn't get the development environment setup, but here's what a proposed patch might look like


diff --git a/httpie/context.py b/httpie/context.py
index 1376347..fa0b599 100644
--- a/httpie/context.py
+++ b/httpie/context.py
@@ -2,9 +2,12 @@ import os
 import sys

 from requests.compat import is_windows
+from curses import setupterm, tgetnum

 from httpie.config import DEFAULT_CONFIG_DIR, Config

+setupterm()
+

 class Environment(object):
     """
@@ -18,7 +21,7 @@ class Environment(object):
     """
     is_windows = is_windows
     config_dir = DEFAULT_CONFIG_DIR
-    colors = 256 if '256color' in os.environ.get('TERM', '') else 88
+    colors = tgetnum('colors') or 88
     stdin = sys.stdin
     stdin_isatty = stdin.isatty()
     stdin_encoding = None

Though I suspect it might have to check if it's on windows before doing any curses stuff? I've never used python on windows before, so I'm not sure =P

Also can't find good documentation on tgetnum so I'm not sure what the error cases are.

jkbrzt commented 10 years ago

The challenge here is to make it work on OSX/Windows/Linux on Python 2.6+. I'll be happy to accept a patch that solves that.

>>> from curses import setupterm, tgetnum
Traceback (most recent call last):
  File "<ipython-input-14-0c0cf9b459cd>", line 1, in <module>
    from curses import setupterm, tgetnum
ImportError: cannot import name 'tgetnum'
>>> sys.version
'3.4.1 (default, May 19 2014, 13:10:29) \n[GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)]'
evanpurkhiser commented 10 years ago

@jakubroztocil you made a typo in your intepreter it's tigetnum not tgetnum

Edit: My bad, I see I made that same mistake in my example patch!

On 3.4.1:

Python 3.4.1 (default, May 19 2014, 17:23:49) 
[GCC 4.9.0 20140507 (prerelease)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from curses import setupterm, tigetnum
>>> setupterm()
>>> tigetnum('colors')
256
>>> import sys
>>> sys.version
'3.4.1 (default, May 19 2014, 17:23:49) \n[GCC 4.9.0 20140507 (prerelease)]'

On 2.6

Python 2.6.9 (unknown, Jul 26 2014, 16:17:22) 
[GCC 4.9.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from curses import setupterm, tigetnum
>>> setupterm()
>>> tigetnum('colors')
256
>>> import sys
>>> sys.version
'2.6.9 (unknown, Jul 26 2014, 16:17:22) \n[GCC 4.9.1]'

You're right though, windows doesn't include the curses module. But do we care about windows? Like I had pointed out earlier, windows is hard coded to 256 colors.

evanpurkhiser commented 10 years ago

I didn't test it because the pip development setup for this project I think is a little more tricky than running make on Arch Linux, but I think this patch should work. Let me know if you'd like me to open a PR!

evanpurkhiser commented 9 years ago

:+1: Thanks for fixing this!