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

Enable/Disable ANSI Sequences on Windows. #275

Closed
wants to merge 1 commit into from
Closed

Enable/Disable ANSI Sequences on Windows. #275

wants to merge 1 commit into from

Conversation

guilt
Copy link

@guilt guilt commented Apr 9, 2019

Similar to jarun/ddgr#87

@jarun
Copy link
Owner

jarun commented Apr 9, 2019

@zmwangx @dertuxmalwieder can you guys please review this PR?

@dertuxmalwieder
Copy link

Setting use_colors up to three times looks inefficient. It might work though. Is it designed to work with both cmd and ConEmu? (I remember having patched such for buku.)

Sent with GitHawk

@jarun
Copy link
Owner

jarun commented Apr 9, 2019

@guilt please take a look at the relevant Buku code.

@dertuxmalwieder
Copy link

Discussed here: jarun/buku#217

@guilt
Copy link
Author

guilt commented Apr 9, 2019

This is only designed to work with standard cmd.

There's plenty of Windows applications which are non Command mode / CLI applications and they shouldn't be affected by the change.

The default option, to disable color is the safest if one cannot enable the console mode. This is still IMO better than the original.

@jarun
Copy link
Owner

jarun commented Apr 9, 2019

@guilt does the patch mentioned above work for you?

@guilt
Copy link
Author

guilt commented Apr 9, 2019

The patch above should work. But yes, what it does is disable ANSI in standard Command Prompt and only enable it in ConEmu.

This one does not do ConEmu but selectively enables ANSI sequences on Command Prompt (which was historically the main problem child, warranting the creation of ConEmu/what not).

This one is also not required for WSL, where things work.

If there's a detection for ConEmu, I'd want someone to look into support for popular emulated environments on Windows, such as Mintty as well.

Here's a comprehensive list: https://alternativeto.net/software/conemu/?platform=windows

@guilt
Copy link
Author

guilt commented Apr 9, 2019

Anyway, my patch works for me. I'm removing my fork and keep this PR as reference. Can't spend more time on it. Sorry.

@zmwangx
Copy link
Collaborator

zmwangx commented Apr 9, 2019

Seems okay functionally, but code isn't very understandable (quite a few magic numbers) and I'd be cautious to include code I don't understand. It's definitely editing code at the wrong place though — the action for --nocolor apparently has to be store_false; otherwise, if your use_colors happen to be false, --nocolor would actually enable color. So the PR needs a medium rewrite anyway.

Since you don't want to spend more time on this, I guess I'll rewrite it later while cross referencing colorama's win32 implementation. A brief look tells me that you might have only modified stdout (where the magic number -11 comes from) but not stderr.

@zmwangx
Copy link
Collaborator

zmwangx commented Apr 9, 2019

Update: please ignore the "wrong place" part of the previous comment. I only glanced at the code and somehow had the wrong impression.

@jarun
Copy link
Owner

jarun commented Apr 9, 2019

@guilt no problem!

@zmwangx is it fine to close this PR then?

@jarun jarun closed this Apr 10, 2019
zmwangx added a commit to zmwangx/googler that referenced this pull request Apr 10, 2019
VT100 control sequences are supported in cmd and PowerShell starting from
Windows 10 Anniversary Update, but only if the
ENABLE_VIRTUAL_TERMINAL_PROCESSING flag is set on the screen buffer handle
using SetConsoleMode.

Setting this flag does not seem to negatively affect third party terminal
emulators with native ANSI support such as ConEmu, so hopefully there's no
regression.

References:
https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences
https://docs.microsoft.com/en-us/windows/console/setconsolemode

Credits:
jarun#275
tartley/colorama#139
zmwangx added a commit to zmwangx/googler that referenced this pull request Apr 10, 2019
VT100 control sequences are supported in cmd and PowerShell starting from
Windows 10 Anniversary Update, but only if the
ENABLE_VIRTUAL_TERMINAL_PROCESSING flag is set on the screen buffer handle
using SetConsoleMode.

Setting this flag does not seem to negatively affect third party terminal
emulators with native ANSI support such as ConEmu, so hopefully there's no
regression.

References:
https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences
https://docs.microsoft.com/en-us/windows/console/setconsolemode

Credits:
jarun#275
tartley/colorama#139
@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.

4 participants