Skip to content
This repository has been archived by the owner on Mar 5, 2022. It is now read-only.

win32: also ENABLE_VIRTUAL_TERMINAL_PROCESSING on stderr #280

Merged
merged 1 commit into from Apr 11, 2019

Conversation

zmwangx
Copy link
Collaborator

@zmwangx zmwangx commented Apr 11, 2019

Follow-up on #277.

As discussed here[1], it's not really necessary when stdout and stderr are the
same console, but it doesn't hurt either.

Note that we did not test sys.stderr.isatty() in the code. This has been
considered, and deemed not a problem, since stdout is handled first (what
really matters) and we have a catch-all pass for any possible exception.

[1] #277 (comment)

Follow-up on jarun#277.

As discussed here[1], it's not really necessary when stdout and stderr are the
same console, but it doesn't hurt either.

Note that we did not test sys.stderr.isatty() in the code. This has been
considered, and deemed not a problem, since stdout is handled first (what
really matters) and we have a catch-all pass for any possible exception.

[1] jarun#277 (comment)
@zmwangx
Copy link
Collaborator Author

zmwangx commented Apr 11, 2019

Nice number by the way.

Screen Shot 2019-04-11 at 1 08 55 PM

@guilt
Copy link

guilt commented Apr 11, 2019

We're not considering the case where stdout goes to file and stderr does not :) But yeah, that's not a general usecase, so we should be good.

In general, we should be getting the same handle for STDOUT and STDERR, so we're good.

@zmwangx
Copy link
Collaborator Author

zmwangx commented Apr 11, 2019

stdout goes to file => no color by default.

stdout goes to file, color enabled by user, user still wants to use interactive prompt for whatever reason (errors are not colored) => user is insane.

@jarun jarun merged commit 493ec9d into jarun:master Apr 11, 2019
@@ -3247,17 +3247,19 @@ def main():
# https://docs.microsoft.com/en-us/windows/console/setconsolemode
if platform.release() == '10':
STD_OUTPUT_HANDLE = -11
STD_ERROR_HANDLE = -12
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put this complete block (from line 3245 to 3266) within a separate API?

@jarun
Copy link
Owner

jarun commented Apr 11, 2019

Oops, I think I've already merged it. But anyway, can we do the API separation?

jarun added a commit to jarun/ddgr that referenced this pull request Apr 11, 2019
@zmwangx zmwangx deleted the win32-set-mode-on-stderr branch April 12, 2019 15:55
@lock lock bot locked and limited conversation to collaborators Nov 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants