VT100 escape sequences appearing on Windows 10 Console - #3897
Merged
Merged
Conversation
Closed
Contributor
|
Will be this a part of 2.0.6? Seems a reasonable user experience bug fix. |
AltGr
approved these changes
Jan 27, 2020
AltGr
left a comment
Member
There was a problem hiding this comment.
Looks good to me. Closing stdin might cause problems with some commands on Linux, but I think here it will be OK; and anyway we will have time to check those.
Member
Author
|
This isn’t ready to go - the CI failures are relevant |
Member
Author
|
I don't intend merging 890c297 (the stdin change) in 2.1. |
Subprocesses which gain access to the Console may reset it while running in the background.
Member
Author
|
Good to go with CI! |
Member
Author
|
�[32mThank you!�[0m 😊 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During
opam init, things proceed nicely until a second or so after the repository tarball is downloaded when the "Processing" message suddenly appears as:The problem is that processes invoked by
opamgainstdinand therefore the Console and clear the Console mode (in particular, Cygwin processes seem to do this). They're running in the background, so it doesn't matter whether they behave nicely and restore the console mode on termination, because the message may well be displayed while they're running.This PR offers two "fixes":
OpamConsolemode is altered to force-set the mode on every print (this is ugly, but not much more ugly than what was there before)allow_stdinis declaredfalseon Windows, and a couple of places whereallow_stdinshould clearly befalse(cygpathinvocation and the download command) are setEither of these on their own is sufficient to fix the Windows problem.