bpo-37827: IDLE shell handling of \r and \b control chars

[taleinat]

This is a first working version, looking for some user testing and feedback!

This allows most progress bars to just work in IDLE, rather than flooding the shell window and sometimes slowing it down to a crawl. More generally, this makes IDLE's shell behave more similarly to a plain Python shell in a terminal.

Note that some progress bar libraries, e.g. tqdm, now "just work"!

In some cases some integration might be required; for example, TensorFlow does several checks on the sys.stdout file-like object to decide whether to use these control characters. Hacking TensorFlow to override that check makes it work as intended in the IDLE shell with this change.

Note that this also currently includes a subtle change which attempts to avoid clobbering the user input when output is written after a command has finished executing, e.g. by a separate thread. But that's not the main addition here.

https://bugs.python.org/issue37827

[taleinat]

@aeros167, do give this a spin if you have some time.

[aeros]

Thanks for the PR @taleinat!

Tested on...
OS: Arch Linux 5.2.8

For testing the \r control character, I used the example @terryjreedy provided in his comment for bpo-37762, but added \r at the end of the string.

CPython master:

The output then continues to flood the shell. I had to use ctrl-c to kill it.

PR branch:

In the PR branch, the numbers rapidly incremented in place within a reasonable period of time (much faster than master, as expected), and it did not flood the shell output.

The PR branch provides far better handling of the \r character for the purpose of text-based progress bars.

[aeros]

After testing the \r character, I went on to test the \b character.

CPython master:

PR branch:

The unknown character symbol was removed, but it's still flooding the output. To ensure I wasn't misusing the \b character, I tried moving it in several different positions within the string, but the issue with the shell output flooding still occurred.

[aeros]

backspace characters (\b) move the cursor back one character.

Based on the documentation added in the latest round of commits, it looks like I was incorrectly using the \b control character. I'll update my tests accordingly.

[aeros]

Had to do a bit of experimenting with the intial example test from Terry to properly test \b after reading the recently added documentation. So far, the results are significant improved compared to my previous test, however, there are still some issues.

In lower numbers, the control character \b functions as expected, the number is continuously overwritten:

However, once around 39,000 is reached, the shell attempts to condense the output into fewer total characters, even though the cursor position did not move beyond the end of the last digit of the most recent number:

Clicking upon one of the "Squeezed text" boxes reveals the following:

It looks like this is happening due to '\b' adding invisible characters after the end of each number. To fix this, it might be required to modify the code responsible for condensing the output, to prevent the condensation from occurring from the invisible characters.

However, this might be a bit of an edge case. In most situations, I don't think users would require the usage of that many \b characters. For a simple percentage display, it still works correctly:

As a result of it providing a significant improvement for most use cases, I approve of this PR. If the bug can be fixed with minimal time investment, it could be done in the same PR. Otherwise, the edge case bug could be fixed separately.

[taleinat]

@aeros167

backspaces = '\b' * (i//10 + 1)

That's not the correct calculation, resulting in a large number of \b characters, eventually triggering Squeezer's auto-squeeze. Try this instead:

backspaces = len(str(i))

Though it should be a very rare edge-case, I'll need to consider the interaction between this and Squeezer.

P.S. If you do want to calculate the correct number of digits, calculate the base-10 logarithm of the number, round down to the nearest integer, and add one. This won't work correctly for zero or negative numbers through... using len(str(i)) as suggested above is simple, correct and fast.

[aeros]

That's not the correct calculation, resulting in a large number of \b characters, eventually triggering Squeezer's auto-squeeze

@taleinat Ah good catch. I was initially going to use len(str(i)), but then I thought it might potentially slow the performance down as it started rapidly outputting any larger numbers. It looks like that works perfectly fine:

Though it should be a very rare edge-case, I'll need to consider the interaction between this and Squeezer.

In that case, I'm glad that I used the incorrect calculation! It probably doesn't need to be addressed within this PR, but it's good to know that exists (as an edge case) rather than it coming up as a surprise later down the road. It probably also would affect \r as well.

P.S. If you do want to calculate the correct number of digits, calculate the base-10 logarithm of the number, round down to the nearest integer, and add one.

Thanks for the tip. Honestly, I have no idea why I thought using i//10 would work, I think I had a bit of a "brain fart" there. (:

[taleinat]

Sadly, I'm closing this due to @terryjreedy's clear disagreement with the desirability of the feature as it is implemented here. Further details on the b.p.o issue (bpo-37827).