New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed #31216 -- Added support for colorama terminal colors on Windows. #12387
Fixed #31216 -- Added support for colorama terminal colors on Windows. #12387
Conversation
Ticket on TRAC -> https://code.djangoproject.com/ticket/31216 |
colorama
terminal colors on Windows.
colorama
terminal colors on Windows.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @MinchinWeb. Welcome aboard! ⛵️
So this works, which is great.
ANSICON
is documented in docs/refs/django-admin.txt
, and the link there still functions. (Also https://github.com/adoxa/ansicon — which has slightly more info about how you would install it.)
An update saying pip install colorama
, would be fine. (I haven't about the exact phrasing: it seems to me ANSICON is a lot harder to get going with: there are no instructions... — but I don't see we need to remove it for folks who have it up and running.)
If you think a mention in the install docs is merited, happy to review that too.
@carltongibson Thank you so much for the prompt comment! I will look into adding the documentation... |
@carltongibson I've added documentation of this in four place:
The docs build for me locally, and look okay. Let me know your thoughts! |
Question: How would I propose colorama being automatically installed (on Windows) with Django? |
Hi @MinchinWeb.
What exactly would you propose? (We've tended to be skeptical about requiring dependencies in the past...) |
# setup.cfg
install_requires =
# existing
asgiref >= 3.2
pytz
sqlparse >= 0.2.2
# new
colorama; platform_system=="Windows" I'd leave the colorama check in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @MinchinWeb. Thanks for this.
So per comment inline, can we detect the new Windows Terminal? That should be our first goal.
Then I think the note in django-admin.txt
update, plus OK, an extra mention in windows.txt
is enough for colorama.
Ref install, @pope1ni has #10663 to use extras_requires
. We could possibly include colorama there. (It's not a hard requirement so we won't add it to install_requires
.) If though we can get detecting the new Windows Terminal working, I'd be inclined to just leave it as a documented manual install (since we want to head to where the puck is going to be, so to speak...)
I think this is a big win! 🥇
Maybe?? You can check for the environmental variable Newer versions of PowerShell also support ANSI color codes, but I'm not sure which version it was added in, and detecting the that requires delving into the WinAPI. |
This comment has been minimized.
This comment has been minimized.
Hi @MinchinWeb. Super thanks. I'll play some more with this but, I'm leaning towards thinking we should opt-in to the future here, perhaps just documenting an off-switch (if you're in an older environment). Let's discuss it and resolve when you're back. |
Hi @carltongibson ! I'm back! I rebased the pull request onto the current master. It seems like a 3.1 branch has been created, so I updated the documentation to indicate this feature will be included in 3.2. I like your suggestion to "opt-in to the future here", and so I've made installing I also updated the "use color?" test like you'd suggested. Let me know if you need me to do anything further. I'm excited to get this included in Django! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @MinchinWeb.
Thanks for this. A few inline comments. Main one being, can we drop the automatic installation, as colorama is not required per se, and then adjust docs accordingly.
django/core/management/color.py
Outdated
try: | ||
import colorama | ||
except ImportError: | ||
colorama = None | ||
else: | ||
colorama.init() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we try moving this to a function, has_colorama()
say?
I know it's not much but the top-level import is a bit 🤔 — I think it might just be clearer inside a function.
(maybe that looks horrible once we try it... — maybe then a comment would do...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we move this to a function, we still need to have a global level variable to test to see if colorama is active (and thus we can turn on color, see line 24).
Also, I think moving this to a function would make it less clear what is going on. Also, this code really should only be called once (so it doesn't need to be in a function for DRY reasons). Possible replacement code:
colorama = has_colorama()
def has_colorama():
try:
import colorama
except ImportError:
colorama = None
else:
colorama.init()
return colorama
docs/howto/windows.txt
Outdated
Colored Terminal Output | ||
======================= | ||
|
||
.. versionadded: 3.2 | ||
|
||
A quality-of-life feature is to output colored (rather than monochrome) output | ||
on the terminal. This should work both on CMD and PowerShell. If for some | ||
reason this needs to be disabled, set the environmental variable | ||
:envvar:`DJANGO_COLORS` to ``nocolor``. | ||
|
||
See :ref:`syntax-coloring` for more information on color settings. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an instruction to pip install colorama
here.
docs/ref/django-admin.txt
Outdated
.. versionchanged:: 3.2 | ||
|
||
Added enabled-by-default support for colored terminal output on Windows via | ||
colorama_. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can drop these in favour of the install instructions (which link here.)
Hi @carltongibson , Earlier, you'd said:
I also worry about discoverablity: i.e. the only people that will know about this are those that do a deep, deep dive into the documentation or they find it on a "Django tips and tricks" blog post. If we do make If you still want it optional, let me know and I can make the changes needed. |
Hi @MinchinWeb. Thanks. Yes. I was leaning that way. But after discussion with @felixxm we decided that including I guess leaving the old instructions don't hurt. I'd put the Thanks for your efforts here! |
Ok, just wanted to make sure something wasn't lost in the time between updates. I'll make the changes and push them here shortly! |
I've made the changes you requested, and even figured out a reasonable way to make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @MinchinWeb.
Thanks again for this. I was thinking I could just tweak the docs changes, and pull this in today but, I think we’re still not on the best approach here.
I think in our most recent comments we were speaking at cross-purposes with reference to what was meant by _ opt-in to the future_ — I think I wasn’t clear. Sorry.
In the suggested changes you have:
Use a newer version of PowerShell or Windows Terminal … and "fake" the installation of
ANSICON
… The easiest way to do this may be to…
This goes back to our discussion earlier:
So per comment inline, can we detect the new Windows Terminal? That should be our first goal.
Maybe?? You can check for the environmental variable
WT_SESSION
, but that is also inherited by any child process you start from Windows Terminal (like if you start VS Code from the command line). See microsoft/terminal#1040Newer versions of PowerShell also support ANSI color codes, but I'm not sure which version it was added in, and detecting the that requires delving into the WinAPI.
Coming back after the long gap, I didn’t realise at first that you’d let this approach drop. Sorry again about that but, this should definitely be the first choice approach in my opinion — _ The easiest way_ here is ”Just use a modern terminal” and if we can detect that we should.
Newer versions of PowerShell also support ANSI color codes…
Do they? I’m on the latest Windows 10 build and, whilst Powershell has colors, it doesn’t look to support Django’s color output:
(This adjusting the supports_color()
function to return True
.)
Maybe we can work out a specific test for Powershell, or correct the output to be compatible, but I’d be inclined to rule that out of scope for this issue.
You can check for the environmental variable
WT_SESSION
, but that is also inherited by any child process you start from Windows Terminal (like if you start VS Code from the command line).
OK, but both the new Windows Terminal and the terminal in VS Code (which we can detect with TERM_PROGRAM
, I think) support colours no problem.
We should try and determine if we’re running in these environments and enable colours if so.
Only then should we fallback to installing colorama
, or the ANSICON
(or faking that with the envvar if needed).
… perhaps just documenting an off-switch
Users can set (still) DJANGO_COLORS=nocolors
to disable if their environment isn’t compatible.
Does that sound reasonable to you?
If so, a few tests for Windows adjusting the environment for Windows Terminal & VS Code to make sure we have the right logic there would be enough I think. (If we could factor that so that someone could add other logic later, like the Powershell case say, that would be good.)
Also, I wondered if we could wrap the colorama
import in a function, something like:
diff --git a/django/core/management/color.py b/django/core/management/color.py
index 0942c44d64..c6d0290a8f 100644
--- a/django/core/management/color.py
+++ b/django/core/management/color.py
@@ -14,14 +14,16 @@ def supports_color():
Return True if the running system's terminal supports color,
and False otherwise.
"""
- try:
- import colorama
- except ImportError:
- colorama = None
- else:
- colorama.init()
-
- supported_platform = sys.platform != 'win32' or 'ANSICON' in os.environ or colorama is not None
+ def has_colorama():
+ try:
+ import colorama
+ except ImportError:
+ return False
+ else:
+ colorama.init()
+ return True
+
+ supported_platform = sys.platform != 'win32' or 'ANSICON' in os.environ or has_colorama()
# isatty is not always implemented, #6223.
is_a_tty = hasattr(sys.stdout, 'isatty') and sys.stdout.isatty()
If we can get that all running right, then a re-jiggle of the docs changes and we’re there.
Do you still have the energy and enthusiasm to push this over the line? It’s a nice addition. I’m happy to pick it up if you’re running out of steam.
Thanks again for your input! Sorry if it’s not quite as quick as you might hope, but if we can make the right decisions here we don’t have to revisit it immediately again — and not needing the extra dependency seems by far the smoothest user-facing route.
Hi @carltongibson ! I'm not quite out of energy yet on this, so I'll keep at it. But feel free to polish it off if need to hit a deadline. I've adjusted the check, basically expanding
It turns out my PowerShell terminal is like yours in that that it's not processing the color codes. I thought I'd "fixed" this some time ago, but maybe I'd just moved to Windows Terminal... As for VS Code's terminal, I can't find it stated definitively either way whether it supports the VT codes/ANSI colors but it's working locally, so I'll go with it. Not done here, but it seems you could enable ANSI colors using I haven't updated the documentation quite yet; but if the code works with you, I'll do that next. |
Running this in powershell shows that it doesn't support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @MinchinWeb. This looks super. I think we should also check for VS Code (after the Windows Terminal check) it’s very popular, and supports colors so...
Great stuff! 👍
Hi @MinchinWeb I read the code before the comment... - it doesn’t look like VS code is checked for, but you said you’d include it — I’ll look closer on Tuesday but, if it’s not there can we add it? Thanks. |
...well then let me just the commit to support VS Code. It was sitting here on my computer, but didn't make it to GitHub earlier for some reason. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @MinchinWeb.
Thanks again for this. I've rebased and squashed and adjusted the docs. (Please git reset
to this point if you make any changes.)
I'm just going to go over each version manually now.
Then a last squish down, because I know @felixxm has comments, and I think we're good to go. 🥇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MinchinWeb Thanks for this patch 👍 I left small comments.
django/core/management/color.py
Outdated
@@ -14,11 +14,57 @@ def supports_color(): | |||
Return True if the running system's terminal supports color, | |||
and False otherwise. | |||
""" | |||
supported_platform = sys.platform != 'win32' or 'ANSICON' in os.environ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chop blank line, and also all blank lines in the supported_platform()
hook.
Right, this works as expected as far as I can see. @MinchinWeb are you OK to reset to 49374b1 and address @felixxm's comments? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels rather verbose and unwieldy. Perhaps something like the following:
try:
import colorama
except ImportError:
HAS_COLORAMA = False
else:
HAS_COLORAMA = True
colorama.init()
...
def supports_color():
"""
Return True if the running terminal supports color, otherwise False.
"""
def enabled_in_windows_registry():
# Check the Windows Registry to see if VT code handling has been
# enabled by default; see https://superuser.com/a/1300251/447564
try:
import winreg
except ImportError:
return False
key = winreg.OpenKey(winreg.HKEY_CURRENT_USER, 'Console')
try:
value, _ = winreg.QueryValueEx(key, 'VirtualTerminalLevel')
except FileNotFoundError:
return False
return value == 1
# isatty() is not always implemented, #6223.
return hasattr(sys.stdout, 'isatty') and sys.stdout.isatty() and (
sys.platform != 'win32' or
HAS_COLORAMA or
'ANSICON' in os.environ or
'WT_SESSION' in os.environ or # Windows Terminal
os.environ.get('TERM_PROGRAM') == 'vscode' or # MVSC Terminal
enabled_in_windows_registry()
)
As mentioned below, I'd prefer that we switch out that inner function checking the registry for one that checks whether the terminal attached to sys.stdout
supports VT codes.
django/core/management/color.py
Outdated
@@ -14,11 +14,57 @@ def supports_color(): | |||
Return True if the running system's terminal supports color, | |||
and False otherwise. | |||
""" | |||
supported_platform = sys.platform != 'win32' or 'ANSICON' in os.environ | |||
|
|||
def supported_platform(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this inner function? We could just shortcut out early if not a TTY.
django/core/management/color.py
Outdated
# check the Windows Registry to see if VT code handling has been | ||
# enabled by default; see https://superuser.com/a/1300251/447564 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that it makes any sense checking the registry. There is no guarantee that you are using a terminal that uses this. So it could just result in a blanket "on" if this is set. It would be better to take the ctypes
approach mentioned to check whether currently enabled for the actual terminal in use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the point is that this defaults to off
, so our chance of a false positive is going to be pretty low no? (In which case folks can just set nocolors
...) (But, yes, maybe this is a better way...)
The ctypes approach is from Davids #12387 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setting defaults to "off" and so if it's on, you've most likely done it deliberately at some point. I.e. it seems like a deliberate attempt by the user to get color output.
It's possible that a terminal emulation might not pay attention to this setting, but that is only an issue if it also does not process VT colours. I'm not aware of any any that would fall into this (although I also haven't don't an exhaustive survey either...). I.e. This seems more hypothetical than something that we will run into with any frequency.
The ctypes
approach is probably better, however, it is beyond my (current) Python ability. My fear there is that even if I did get it working, I wouldn't be able to fix it (debug it) when if breaks...
We're going round in a circle on the We should probably go back to the original (or Nick's variant). If we're going to have it at the top level, then we may as well init it there, to keep it in one place. Sorry for the back-and-forth on that @MinchinWeb. We should try and get this in one way or the other, as it's a big win for a lot of our users. |
@MinchinWeb just to say thanks for all your work on this. I'm super excited for this feature. |
I will get back to finish this up; life just keeps happening at the moment! |
Hi @MinchinWeb. No stress! We're nearly there, and there's a long time to the 3.2 feature freeze. Plenty of time. If at any stage you think you'd rather pass it on, just say, and I'm happy to pick it up. Thanks again for your input! |
Hi @MinchinWeb hope you are well, wow what a year it has been. We're a couple months out from the next feature freeze and I'm keen on seeing this get into 3.2. Are you still up for finishing this off? Anything you need help with? |
@carltongibson @smithdc1 I think this is ready to go. I've added a comment to the testing documentation. I've tried to address the comments here. Let me know if anything else need polishing. @smithdc1 Indeed, what a year! My year has been well, all things considered. Busy, very busy. We're happy and healthy, and last night's snowfall has left the world in beautiful white :) |
Modern setups on Windows support terminal colors. The colorama library may also be used, as an alternative to the ANSICON library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks @MinchinWeb, let's take this now.
Zen of Python:
Now is better than never
Maybe the ctypes approach is nicer overall, but this is a good win, seems robust enough in my testing, and is the option we have in hand now. If someone else wants to come forward later with a better patch then cool, but this works well enough. It's likely that's never needed.
Thanks for your effort @MinchinWeb. Welcome aboard! ⛵️
👍 I'd be able to handle the ctypes side of things, but I don't have access to a Windows environment so 🤷♂️. My main interest is that Windows support is advanced enough not to hold back using colours or unicode on other platforms. This is nonetheless a great improvement. Thanks @MinchinWeb. |
Thanks y'all 🎊 I pushed small edits. |
@carltongibson @pope1ni @felixxm Thank you all 🙏 for all your help in getting this across the finish line! It's excited to say I helped make Django just a little bit better :) Thank you again! |
Hi @MinchinWeb — Thank you for your work here. It's a great addition. Welcome aboard! ⛵️ |
If colorama is installed (as on Windows), support colored terminal output.
There is a very old ticket (https://code.djangoproject.com/ticket/13476) discussing how to support colorized terminal output on Windows. At the time, ANSICON was chosen as a solution over
colorama
. However, the link provided for ANSICON (http://adoxa.hostmyway.net/ansicon/) no longer works, and, personally, I struggled to install it previously (it involved messing around with DLL's and some fairly low level Windows functionality).colorama
, on the other hand, is a current Python package that provides color on Windows by converting the ANSI cods to Windows native color "codes"; on other platforms, it just passes through the ANSI codes through.colorama
is also very popular: for example, it is vendored bypip
(https://github.com/pypa/pip/tree/master/src/pip/_vendor).What this change does is test to see if
colorama
is available, and activate color (on Windows) if it is. It is not a hard dependency or even added to the "install_requires" ofsetup.py
. Personally, I would support addingcolorama
as a dependency on Windows, but I wasn't sure how much support there would be for that.No documentation changes have been added, although I think this would be wise to mention in the installation instructions for Windows. No tests have been added either, but I'm not really sure what to test directly. If someone could point me to where to make those changes, I would be happy to.