Enable/Disable ANSI Sequences on Windows.

[guilt]

Fixes #86

[jarun]

Will port the changes from @zmwangx for googler as discussed in jarun/googler#275. Closing this.

[jarun]

@guilt I have completed porting the changes from googler to ddgr at commit 5016e02. Can you please confirm it works as expected?

[guilt]

Nit: It's a bit wierd when you check for isatty() in two places now (get_colorize) but looks good otherwise. Shouldn't we just use 'colorize' only, assuming it verifies isatty() if 'auto'?

Other than that, it should work as expected.

[jarun]

Shouldn't we just use 'colorize' only, assuming it verifies isatty() if 'auto'

And what happens when user sets opts.colorize = 'always'?

[guilt]

It's set to True, which is what the user wants, don't they? And if it's not a tty, then they probably are saving to a text file which is going to have colored text in it, when viewed with less.

So, technically, the always option is sort of a way to shoot the user in the foot. This can also happen in non-Windows BTW, because this always option is not dependent on Windows.

Anyway, I tried it, and the escape sequences to appear in the text file:

python ddgr --colorize=always ddgr > output.txt

and I'm able to view the colored output.txt using more or less on Linux. So we could remove that check.

[jarun]

And if it's not a tty, then they probably are saving to a text file which is going to have colored text in it, when viewed with less.

I know. But it doesn't look like a very regular use case. From my experience, users of googler/ddgr who deal in text use the json format or plain text (-C). I haven't come across a scenario where one saves color codes to text files to view them in less/more later when (s)he can fetch the results from the web anytime again.

Anyway, given the fact that there is an auto option, I think we can always say - use auto instead of always (which does what it means). I will make the change now.

[jarun]

Wait, why do we need to call set_win_console_mode() when opts.colorize = 'always'?

Does calling the API make any difference when you redirect output?

[zmwangx]

It’s not only pointless to set console mode when stout is not a tty, it’s impossible to set it because guess what, it’s not a console.

[jarun]

Right. The API set_win_console_mode() has nothing to do with redirection to text file.

[guilt]

Well.... what it means is that if colorize is True, there's an error and stdout is not a tty but stderr is, that error will appear pretty ;)

But then colorize=always will not do what it should do, does it? I mean, It's really a nit but I wanted to keep it simple.

[jarun]

I've decided to leave it in its current state and can't spend any more time on trivia.

[zmwangx]

Once again, errors are not colored. Only the prompt is. Also, removing the condition doesn’t color stderr at all, since it will error out on stout.

Coming up with weird fictional use cases doesn’t help with anything. googler is for humans, not fuzzers.

I wrote every step of the logic and most loc in the current iteration of googler, so trust me, I have thought of everything.

[guilt]

It's a simple usecase, ddgr > x.txt , when you are not connected to the Internet or DDG/Google has a problem, you'd emit an unintelligle error to stderr, while stdout may not get anything. That is not a made up usecase.

[zmwangx]

$ https_proxy=255.255.255.255 googler 'not connected' > /tmp/blah
[ERROR] Failed to connect to proxy server 255.255.255.255: Remote end closed connection without response.

No idea why that's in any way "unintelligle [sic]", and why color/ANSI escape sequence has anything to do with it. I'll say it one last time: ANSI escape sequences are not emitted in any error message.

[guilt]

Looks like I was looking at an older version of code that fed sequences directly to printerr. ddgr looks fine now. This is like a recipe that was used in other similar CLI utilities as well. You could have mentioned that these were reviewed/removed.

Also, hold your horses: there still seems to be some of that code in googler which I think should be fixed, if you want to rid your old code of this:

https://github.com/jarun/googler/blob/fe9de616a90d88e08f14b16aff0e1cfbd006c6e2/googler#L2103