-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Enable tty, vt sequences on Windows #36598
Conversation
I just verified that this PR also solves the flickering cursor problem described in #36584. I verified this for the old school Windows console, the new Windows Terminal and the VS Code built-in terminal: all three had the flickering without this PR, with this PR it is gone in all three. |
Yes this PR fixes that issue for me, as well! As it stands, this PR is a stop-gap solution, until libuv has proper pseudo-tty support. Given that we don't have a time frame on when and if such a feature will come, I think something similar to what is proposed in this PR is needed in the meantime. Granted it is a stop-gap solution on already problematic scenario, being that we are forced to use a shared console between child processes, which can have different implicit assumptions on the console state and lead to corruption. |
We should probably try this on the oldest officially supported Windows version, just to be sure. This suggests that Windows 10 is the min version for Julia. There are of course very many different Windows 10 versions, and many of the older releases are no longer supported by MS. Looking at this table and assuming that versions that are still supported for enterprise and edu customers are also what Julia officially supports, is 1709 the oldest version that we need to test? Or should we go with 1803 right away, because any build that includes this PR here would come out after 10/13 in any case? |
I updated this PR and I actually think it should now be compatible with any Windows version. All that the new code does it reset the terminal mode back to the default mode, in case a shell application modifies it. |
For reference here's the example that demonstrates the current way echo_cmd = `C:\\Program\ Files\\Git\\usr\\bin\\echo.exe hello`
@async begin
sleep(10)
run(echo_cmd)
end
run(`wsl`)
Without This PR actually improves this pathological test case, because on master executing the above corrupts the console permanently, but with this PR resetting to the default states returns the REPL back to useful state. |
So on my end I'm happy with the current state of this RP. @vtjnash Would love to get your review on this if you have the time 😃. |
going to merge in a few days if no objections |
stdlib/REPL/src/LineEdit.jl
Outdated
ccall(:GetConsoleMode, stdcall, Int32, (Int32, Ptr{Nothing}), hOutput, dwMode) | ||
dwMode[] | ||
end | ||
const default_console_mode = _console_mode() |
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.
should not be a build-time constant
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.
Good point! I changed to be a reference that get's set the first time the REPL is printed at runtime and then is cached perpetually.
#ifdef _OS_WINDOWS_ | ||
SetConsoleCP(1252); // ANSI Latin1; Western European (Windows) | ||
#endif | ||
|
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.
this feels slightly risky and unrelated? (reverts eac525c, but libuv is now unicode-aware, so perhaps that's not as bad now)
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.
We need to remove this or set the console to SetConsoleCP(CP_UTF8)
or else the sequences don't get interpreted correctly
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.
CP_UTF8 (65001) is not well supported. That's why we set it to 1252 (It even can break dir
: nodejs/node-v0.x-archive#4246 (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.
Is that comment relevant? It looks like part of the issue was with using a raster front
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.
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.
Looks like things probably got fixed internally in libuv?
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.
Yes, libuv Is fixed now to bypass this. The issue would be with other shared libraries that assume an ascii-compatible stream.
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.
Good to hear. Is that issue more theoretical than practically relevant (?), I don't know what we could do about this pathological case without regressing the whole system to be dumber because of some library is forcing ascii.
Ok to merge soon? |
Unless there are objections or feedback, then yes, please do merge. |
Thanks @vtjnash for the review! and also @davidanthoff for working to test this PR and his collaboration on trying to make this a reality. |
…n Windows (JuliaLang#36598) On Windows, when launching external processes, we cannot control what assumption they make on the console mode. We thus forcibly reset the console mode at the start of the prompt to ensure they do not leave the console mode in a corrupt state.
Enable tty, vt sequences on Windows.
Ultimately, we can't control what external processes assume on the console mode when launched within Julia. With the shared console mode that Julia utilizes, this is problematic and the terminal can be left in a corrupted state. As a result, it's safest to reset the mode before each prompt to ensure we enter back into the original console state.
In particular, this fixes problems like that in #26894 (comment).
Currently Julia has disabled vt support, as this can help enhance compatibility. Note, however, we can corrupt the terminal state even under the current status quo, which uniformly disables vt support, and this can be demonstrated through examples. Disabling tty support, has been an annoyance on Windows and has lead to a lot of comments on how the Windows Julia terminal experience is crippled and that this is unfixable since Microsoft has not improved the terminal experience, leading to discussion on other terminal emulators such as mintty. In fact, I think we can support a quality terminal experience on Windows if we carefully enable features that support an improved REPL experience.
This PR in tandem with JuliaLang/libuv#11 enables tty support on Windows.
Here's an example of Julia with these patches using the Windows Terminal:
For those that want to try out this PR built with the updated libuv fork that enables tty support on Windows, here's a x86-64 windows installer you can play around with: https://1drv.ms/u/s!AvuMkGVjog3NuaQswzrzMyFP8XlVNw?e=qbv749
closes #31491