Skip to content
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

Add colorama.just_fix_windows_console() #352

Merged
merged 9 commits into from
Oct 17, 2022

Conversation

njsmith
Copy link
Collaborator

@njsmith njsmith commented Oct 17, 2022

As discussed in #139

Re: the bugs you collected:

just_fix_windows_console() is a no-op if run repeatedly, or if run after someone else has called init.

just_fix_windows_console only touches streams that are pointing to a Windows console. So I guess in theory it would still interfere with your random binary if you try dumping it on your terminal, but (a) no-one expects that to do anything useful, (b) it interprets the random binary the same way a Unix console would, and people seem to live with that. So I'm not worried about it.

I've tested this manually (running demos/demo01.py) on Linux/Win7/Win10.

The big remaining issue is that I'm not sure how to test this, because it touches global state, and init also touches global state, and I'm not sure how to make all the tests run in a single process.

@njsmith
Copy link
Collaborator Author

njsmith commented Oct 17, 2022

The big remaining issue is that I'm not sure how to test this, because it touches global state, and init also touches global state, and I'm not sure how to make all the tests run in a single process.

Never mind, fixed this and wrote a test.

@njsmith njsmith requested a review from tartley October 17, 2022 04:07
@tartley
Copy link
Owner

tartley commented Oct 17, 2022

This looks amazing, works for me, thank you @njsmith !

@tartley tartley merged commit 60e1f54 into tartley:master Oct 17, 2022
@LqdBcnAtWork
Copy link
Contributor

I'm unsure what's up with 0.4.6, but Cursor appears to be broken on win10. Sort of.

Cursor.UP seems to do nothing.
Cursor.POS appears to work.

Also, is there a better way to get cursor position than win32.GetConsoleScreenBufferInfo().dwCursorPosition?

@njsmith njsmith deleted the just-fix-windows-console branch October 17, 2022 21:45
@njsmith
Copy link
Collaborator Author

njsmith commented Oct 17, 2022

@LqdBcnAtWork Microsoft says it should work, and it seems to work for me:
image
Do you have a test case you can share?

@LqdBcnAtWork
Copy link
Contributor

I have determined the issue I was having. Cursor is not effected. colorama.win32.GetConsoleScreenBufferInfo().dwCursorPosition however is.

The following code has shown the issue and is repeatable for me:

from time import sleep
from colorama import Cursor
import colorama

colorama.just_fix_windows_console()

space = 10

print(1)
print("\n"*space,Cursor.UP(space),end="\r",sep="") #run with end="" and end="\r"
sleep(1) # take note of where the cursor is while 'sleeping'
pos = colorama.win32.GetConsoleScreenBufferInfo().dwCursorPosition
print(2)
print("pos:",pos.Y)

By having end="" the Y position is calculated while not respecting the movement done by Cursor.UP, however having something print after the reposition fixes the issue.

@njsmith
Copy link
Collaborator Author

njsmith commented Oct 18, 2022 via email

@LqdBcnAtWork
Copy link
Contributor

I discovered another quirk with colorama.win32.GetConsoleScreenBufferInfo(). While this is probably a bad place to write it, I'm unsure of where I should.

Anyway, in windows default command prompt, running str(colorama.win32.GetConsoleScreenBufferInfo()) yields something like this: '(9001,120,56,0,7,27,0,56,119,63,120)'. Those first two numbers are the terminal height and width. However calling the same function in something like the fancy new Terminal app for win10, the result is very different '(52,209,51,0,7,0,0,51,208,52,209)'. It gives the actual space for text, no matter how much you can scroll back by. Now this is an issue because the CMD reports the cursor position from it's zero, while the Terminal reports the cursor position within the screen, but both translate the ANSI calls to be relative to the screen.

So, in my case I have a function that'll print n lines, move back up, n lines, then save the cursor position for printing/reprinting a line. This is so I can specify the amount of lines I may or may not print (but not track) before scrolling would happen. That way I can return to a line and update it's contents without worrying about having to know exactly how many lines have been printed. But due to inconsistent execution between dev and production I need to be able to handle things like the CMD and the Terminal.

Fortunately colorama.win32.GetConsoleScreenBufferInfo().srWindow.Top reports correctly for both, so I just get the Y value for the cursor, and subtract that Top value from it, and add 1. That way I always get an accurate value for the cursor's line.

Windows console, always causing issues because it's different.

@tartley
Copy link
Owner

tartley commented Oct 19, 2022

Thank you for sharing the info @LqdBcnAtWork, it's good to have it posted somewhere publicly. If I was better at curating our 'issues', I'd suggest it should go there. In fact, why don't I transfer it there... #357

I created both your reported quirks as issues. Thank you! I closed them, because I don't think we ought to be making any changes to Colorama in response (but am happy to hear disagreements), and then at least anyone who hits the same quirks can find them by searching our issues and possibly find workarounds, etc.

@tartley
Copy link
Owner

tartley commented Oct 19, 2022

We have a release candidate on PyPI containing this PR. Thank you for your contributions! https://pypi.org/project/colorama/0.4.6rc1/

If nobody spots any problems with it, I'll push the final 0.4.6 tomorrow.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Nov 9, 2022
0.4.6 Current release
* tartley/colorama#139 Add alternative to 'init()',
  called 'just_fix_windows_console'. This fixes many longstanding problems
  with 'init', such as working incorrectly on modern Windows terminals, and
  wonkiness when init gets called multiple times. The intention is that it
  just makes all Windows terminals treat ANSI the same way as other terminals
  do. Many thanks the njsmith for fixing our messes.
* tartley/colorama#352 Support Windows 10's ANSI/VT
  console. This didn't exist when Colorama was created, and avoiding us
  causing havok there is long overdue. Thanks to segeviner for the initial
  approach, and to njsmith for getting it merged.
* tartley/colorama#338 Internal overhaul of package
  metadata declaration, which abolishes our use of the now heavily
  discouraged setuptools (and hence setup.py, setup.cfg and MANIFEST.in), in
  favor of hatchling (and hence pyproject.toml), generously contributed by
  ofek (author of hatchling). This includes dropping support Python3.5 and
  3.6, which are EOL, and were already dropped from setuptools, so this
  should not affect our users.
* tartley/colorama#353 Attention to detail award to
  LqdBcnAtWork for a spelling fix in demo06
njsmith added a commit to njsmith/tqdm that referenced this pull request Nov 21, 2022
tqdm uses colorama to enable proper ANSI support on Windows.
Traditionally, it's done this by calling `colorama.init`. But this API
is pretty awkward: it tries to do too much (e.g. guessing whether you
have ANSI support and if not then installing sys.stdout/stderr filters
that remove all ANSI codes from output -- even if the system actually
does support ANSI!), and it isn't idempotent, so if multiple libraries
all call `colorama.init` then you can get brokenness (in particular, it
can trigger the situation where colorama thinks ANSI is unavailable,
when it actually is!).

To solve this, colorama 0.4.6 added a new API,
`colorama.just_fix_windows_console`:
tartley/colorama#352

The advantage of this is that it does what it says: it just fixes the
Windows console, without any strange side-effects, and it's safe to call
multiple times.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants